On Sat, Mar 25, 2017 at 2:21 AM, Denis Laxalde <de...@laxalde.org> wrote:
> # HG changeset patch > # User Denis Laxalde <denis.laxa...@logilab.fr> > # Date 1489594320 -3600 > # Wed Mar 15 17:12:00 2017 +0100 > # Node ID ec77aa4ff2993057b604bdffb449d2d179525a9f > # Parent 2f75006a7f31c97d29fd6dd9b72fa7cc9e7ab840 > # Available At https://hg.logilab.org/users/dlaxalde/hg > # hg pull https://hg.logilab.org/users/dlaxalde/hg -r > ec77aa4ff299 > # EXP-Topic linerange-log/hgweb-filelog > hgweb: expose a followlines UI in filerevision view (RFC) > > In filerevision view (/file/<rev>/<fname>) we add some event listeners on > mouse selection of <span> elements in the <pre class="sourcelines"> block. > Those listeners will capture the range of mouse-selected lines and, upon > mouse > release, a box inviting to follow the history of selected lines will show > up. > This action is advertised by a :after pseudo-element on file lines that > shows > up on hover and invite to "select a block of lines to follow its history". > > All JavaScript code lives in a new "linerangelog.js" file, sourced in > filerevision template (only in "paper" style for now). > > This is proposal implementation, comments welcome on any aspects. > This feature is frigging awesome!!! I can pretty much guarantee that some engineers at Mozilla will go completely crazy for this feature. As I'm playing with it locally, I found a few paper cuts. First, on the initial revision introducing lines (last revision as rendered in hgweb), the diff is often (always?) empty. I can also see empty diffs in the intermediate changesets. For example, run this on mercurial/exchange.py and ask it for the line range of the _pushdiscovery(pushop) function. For this function, I get 2 empty diffs (revisions 233623d58c9a and ddb56e7e1b92). Highlighting _pushdiscoverychangeset() yields only an empty initial diff. Second, the highlighting UI is a bit confusing. It took me a few seconds to realize I had to actually hold down the mouse button and highlight the lines. I don't think users are accustomed to do this on text in web pages unless they are copying text. It was definitely surprising to me that highlighting lines of text resulted in a special pop-up appearing. I'm no web design expert, but how about this design. As you mouseover lines, you see a graphic cursor somewhere on that line. There is likely a label next to it saying "click anywhere to follow from this line" or something like that. This is similar to the floating text label you have now. When you mouse click, that floating cursor is dropped and locked into place on that line. On subsequent mouseovers, another cursor+label appears. The lines between the initial cursor and the current mouse location are highlighted somehow (possibly adding a different background color). The label says "click anywhere to follow lines XX to YY" or something. When the user clicks to drop the 2nd cursor, either they're taken directly to the filelog view or we get an inline box with links (like the line number mouseovers on the annotate page). If the user accidentally clicks to drop a cursor, they can clear the cursor be clicking the cursor or an "x" next to it. Another minor nitpick with the highlighting UI is the "follow lines" box may not appear next to the mouse cursor. I think we want it to appear near the cursor so it is easier to find/use. The UX can obviously be improved as a follow-up. I don't want to detract from getting the core of the feature landed. This is shaping up to be pretty amazing! > > diff --git a/contrib/wix/templates.wxs b/contrib/wix/templates.wxs > --- a/contrib/wix/templates.wxs > +++ b/contrib/wix/templates.wxs > @@ -225,6 +225,7 @@ > <File Id="static.coal.file.png" Name="coal-file.png" /> > <File Id="static.coal.folder.png" Name="coal-folder.png" /> > <File Id="static.excanvas.js" Name="excanvas.js" /> > + <File Id="static.linerangelog.js" Name="linerangelog.js" /> > <File Id="static.mercurial.js" Name="mercurial.js" /> > <File Id="static.hgicon.png" Name="hgicon.png" /> > <File Id="static.hglogo.png" Name="hglogo.png" /> > diff --git a/mercurial/templates/paper/filerevision.tmpl > b/mercurial/templates/paper/filerevision.tmpl > --- a/mercurial/templates/paper/filerevision.tmpl > +++ b/mercurial/templates/paper/filerevision.tmpl > @@ -71,8 +71,11 @@ > <div class="overflow"> > <div class="sourcefirst linewraptoggle">line wrap: <a > class="linewraplink" href="javascript:toggleLinewrap()">on</a></div> > <div class="sourcefirst"> line source</div> > -<pre class="sourcelines stripes4 wrap bottomline">{text%fileline}</pre> > +<pre class="sourcelines stripes4 wrap bottomline" > data-logurl="{url|urlescape}log/{symrev}/{file|urlescape}" > >{text%fileline}</pre> > </div> > + > +<script type="text/javascript" src="{staticurl|urlescape} > linerangelog.js"></script> > + > </div> > </div> > > diff --git a/mercurial/templates/static/linerangelog.js > b/mercurial/templates/static/linerangelog.js > new file mode 100644 > --- /dev/null > +++ b/mercurial/templates/static/linerangelog.js > @@ -0,0 +1,83 @@ > +// Copyright 2017 Logilab SA <cont...@logilab.fr> > +// > +// This software may be used and distributed according to the terms > +// of the GNU General Public License, incorporated herein by reference. > + > +document.addEventListener('DOMContentLoaded', function() { > + // install mouse event listeners on <pre class="sourcelines"> > + var sourcelines = document.getElementsByClassName('sourcelines')[0]; > + if (typeof sourcelines === 'undefined') { > + return; > + } > + var targetUri = sourcelines.dataset.logurl; > + if (typeof targetUri === 'undefined') { > + return; > + } > + // add event listener to the whole <pre class="sourcelines"> block to > have > + // it available in all children <span> > + sourcelines.addEventListener('mousedown', lineSelectStart); > + > + //** event handler for "mousedown" */ > + function lineSelectStart(e) { > + var startElement = e.target; > + if (startElement.tagName !== 'SPAN') { > + // we may be on a <a> > + return; > + } > + // retarget "mouseup" to sourcelines element so that if this event > + // occurs outside the <pre> we don't miss it > + sourcelines.setCapture(); > + // drop any prior <div id="followlines"> > + var previousInvite = document.getElementById('followlines'); > + if (previousInvite !== null) { > + previousInvite.parentNode.removeChild(previousInvite); > + } > + > + var startId = parseInt(startElement.id.slice(1)); > + > + //** event handler for "mouseup" */ > + function lineSelectEnd(e) { > + // remove "mouseup" listener > + sourcelines.removeEventListener('mouseup', lineSelectEnd); > + > + var endElement = e.target; > + if (endElement.tagName !== 'SPAN') { > + // not on <span>, probably outside <pre > class="sourcelines">, > + // we cannot compute endId > + return; > + } > + > + // compute line range (startId, endId) and insert the > + // "followlines" element > + var endId = parseInt(endElement.id.slice(1)); > + if (endId == startId) { > + return; > + } > + var inviteElement = endElement; > + if (endId < startId) { > + var tmp = endId; > + endId = startId; > + startId = tmp; > + inviteElement = startElement; > + } > + var div = followlinesBox(startId, endId); > + inviteElement.appendChild(div); > + } > + > + sourcelines.addEventListener('mouseup', lineSelectEnd); > + > + } > + > + //** return a <div id="followlines"> with a link for followlines > action */ > + function followlinesBox(startId, endId) { > + var div = document.createElement('div'); > + div.id = 'followlines'; > + var a = document.createElement('a'); > + var url = targetUri + '?patch=&linerange=' + startId + ':' + > endId; > + a.setAttribute('href', url); > + a.textContent = 'follow lines ' + startId + ':' + endId; > + div.appendChild(a); > + return div; > + } > + > +}, false); > diff --git a/mercurial/templates/static/style-paper.css > b/mercurial/templates/static/style-paper.css > --- a/mercurial/templates/static/style-paper.css > +++ b/mercurial/templates/static/style-paper.css > @@ -276,10 +276,28 @@ td.annotate:hover div.annotate-info { di > float: left; > } > > +div.overflow pre.sourcelines > span:hover:after { > + content: "select a block of lines to follow its history"; > + display: inline; > + float: right; > + line-height: 1em; > + background-color: #FFFFFF; > + color: #001199; > +} > + > .sourcelines > span:target, tr:target td { > background-color: #bfdfff; > } > > +div#followlines { > + display: inline; > + position: absolute; > + background-color: #FFFFFF; > + border: 1px solid #999; > + color: #000000; > + padding: 2px; > +} > + > .sourcelines > a { > display: inline-block; > position: absolute; > diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t > --- a/tests/test-hgweb-commands.t > +++ b/tests/test-hgweb-commands.t > @@ -1343,9 +1343,12 @@ File-related > <div class="overflow"> > <div class="sourcefirst linewraptoggle">line wrap: <a > class="linewraplink" href="javascript:toggleLinewrap()">on</a></div> > <div class="sourcefirst"> line source</div> > - <pre class="sourcelines stripes4 wrap bottomline"> > + <pre class="sourcelines stripes4 wrap bottomline" > data-logurl="/log/1/foo"> > <span id="l1">foo</span><a href="#l1"></a></pre> > </div> > + > + <script type="text/javascript" src="/static/linerangelog.js"></script> > + > </div> > </div> > > @@ -1468,9 +1471,12 @@ File-related > <div class="overflow"> > <div class="sourcefirst linewraptoggle">line wrap: <a > class="linewraplink" href="javascript:toggleLinewrap()">on</a></div> > <div class="sourcefirst"> line source</div> > - <pre class="sourcelines stripes4 wrap bottomline"> > + <pre class="sourcelines stripes4 wrap bottomline" > data-logurl="/log/2/foo"> > <span id="l1">another</span><a href="#l1"></a></pre> > </div> > + > + <script type="text/javascript" src="/static/linerangelog.js"></script> > + > </div> > </div> > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
_______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel