[PATCH 7 of 7 V5] hgweb: expose a followlines UI in filerevision view (RFC)
Gregory Szorc
gregory.szorc at gmail.com
Mon Mar 27 23:58:02 EDT 2017
On Mon, Mar 27, 2017 at 9:40 AM, Augie Fackler <raf at durin42.com> wrote:
> On Sat, Mar 25, 2017 at 12:34:47PM -0700, Gregory Szorc wrote:
> > On Sat, Mar 25, 2017 at 2:21 AM, Denis Laxalde <denis at laxalde.org>
> wrote:
> >
> > > # HG changeset patch
> > > # User Denis Laxalde <denis.laxalde at 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.
>
> I've queued patches 1-6, which lay all the groundwork for this. Greg,
> can I have you drive the review of this last patch and let me know
> when I should take a look at it?
>
OK.
I'm not sure how others feel about landing this in its current form. If
another reviewer is OK with it, I'll give it a thorough review. Otherwise,
I'll wait for UI enhancements.
Also, I'd encourage others to comment on the UI.
>
> Thanks!
>
> (Very excited for this cool feature)
>
> >
> > 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 <contact at 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 at mercurial-scm.org
> > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> > >
>
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170327/4caf9aad/attachment.html>
More information about the Mercurial-devel
mailing list