On June 24, 2018 9:39:35 PM GMT+08:00, John Keeping <[email protected]> wrote:
>On Sun, Jun 24, 2018 at 08:00:08PM +0800, Andy Green wrote:
>> 
>> 
>> On June 24, 2018 7:42:33 PM GMT+08:00, John Keeping
><[email protected]> wrote:
>> >On Sun, Jun 24, 2018 at 10:44:54AM +0800, Andy Green wrote:
>> >> Since the only reason to click on the line number links
>> >> is to get the corresponding #URL to share, this patch
>> >> makes that process more convenient by copying the
>> >> highlit area, be it a single line or a range, to the
>> >> clipboard on each click of the line number links.
>> >
>> >As a user, I'd find this surprising and probably quite annoying.
>> >
>> >I strongly prefer that software not overwrite the clipboard contents
>> >without an explicit request to do so.  A quick survey suggests none
>of
>> >GitHub, Gitlab or repo.or.cz (GitWeb) behave in this way.
>> 
>> Is there another possible intention behind clicking on the line
>number
>> links I am missing?
>
>One I can think of (given your recent patches) is to highlight a line
>when pointing it out to someone else looking at the same display.  I
>have definitely used this with other web interfaces in the past.

Well, ok, so he can click on it to show his buddy.  He gets the URL in his 
clipboard.  But his buddy is there he doesn't need it.

>> If not, literally the only reason to click on them is because you
>> intend to copy the #URL to the clipboard.
>> 
>> Then that is "an explicit request", isn't it?
>
>In the X11 world there are separate selection buffers for selecting
>text
>and for Ctrl-C/V behaviour [1].  It might not be unreasonable to set
>PRIMARY to the URL, but setting CLIPBOARD is clearly wrong.
>
>My use of "explicit" above is consistent with the use in this spec,
>unless the user has selected Copy, or pressed Ctrl-C there is no
>expectation that the clipboard will be overwritten.
>
>Certainly for an X11 user overwriting CLIPBOARD when clicking a

The browser has no idea about that.  Cgit isn't an X11 beast, it feeds 
crossplatform browsers.

If the browser copies anything to the clipboard by js api, it's always the ^C 
^V one since the js stuff only has the concept of that one.  So in the case the 
user wants this behaviour, the surprising thing would be on some platforms it 
went in some other "clipboard" (this is not possible in js afaik).  The user 
would be nonplussed if it could and then he was sat in front of a windows 
browser.  So I think the observations about that lead nowhere.

>hyperlink fails the principle of least surprise, and I think that

Well, I used it and I liked it.  It felt pretty natural.  But an extra click 
won't kill me.

>applies to all users because this isn't how web pages work: on any
>other
>page, if I want to copy a link I right-click and copy the link.  I
>don't
>expect the clipboard to be overwritten just because I clicked a
>hyperlink.

On github it pops up a sticky popup with an ellipsis because you "clicked a 
hyperlink" in the same conditions.  (Clicking that brings a js menu with a copy 
entry).  Users can handle variation, at least if it's indicating to them what's 
happening and why, and is useful.  At least it doesn't seem to have held github 
back...

The link we want to copy is not the ...#nXXX link in the a href when there is a 
range up.  I never tried to intercept "Copy Link", maybe it could be done.  But 
at least the default menu won't do as it is in this case.

I think no point for the two-step menu like github since the only option we 
plan to show atm is Copy (the other useful option is switch to blame at that 
line).  How about a popup that appears on the left for a few secs with a copy 
type icon, or "Copy", in it, that fades away if not used?  It's one extra click 
but still one less than github.

-Andy

>[1]
>https://specifications.freedesktop.org/clipboards-spec/clipboards-0.1.txt
>
>> >> Signed-off-by: Andy Green <[email protected]>
>> >> ---
>> >>  cgit.js |   36 ++++++++++++++++++++++++++++++++----
>> >>  1 file changed, 32 insertions(+), 4 deletions(-)
>> >> 
>> >> diff --git a/cgit.js b/cgit.js
>> >> index 2cfad29..e2c3799 100644
>> >> --- a/cgit.js
>> >> +++ b/cgit.js
>> >> @@ -89,8 +89,29 @@ function cgit_line_range_highlight()
>> >>           e.scrollIntoView(true);
>> >>  }
>> >>  
>> >> +function cgit_copy_clipboard(value)
>> >> +{
>> >> + var inp = document.createElement("textarea");
>> >> + var e = document.getElementById("linenumber_td");
>> >> +
>> >> + inp.type = "text";
>> >> + inp.value = value;
>> >> + /* hidden style stops it working for clipboard */
>> >> + inp.setAttribute('readonly', '');
>> >> + inp.style.position = "absolute";
>> >> + inp.style.left = "-1000px";
>> >> +
>> >> + e.appendChild(inp);
>> >> +
>> >> + inp.select();
>> >> +
>> >> + document.execCommand("copy");
>> >> +
>> >> + inp.remove();
>> >> +}
>> >> +
>> >>  function cgit_line_range_click(e) {
>> >> - var t = e.target.id;
>> >> + var t = e.target.id, cp;
>> >>  
>> >>   cgit_line_range_override = null;
>> >>  
>> >> @@ -101,13 +122,13 @@ function cgit_line_range_click(e) {
>> >>    * is called, and override it there.
>> >>    */
>> >>  
>> >> - if (window.location.hash && window.location.hash.indexOf("-") <
>0)
>> >> + if (window.location.hash && window.location.hash.indexOf("-") <
>0)
>> >{
>> >>           if (parseInt(window.location.hash.substring(2)) <
>> >>               parseInt(t.substring(1))) /* forwards */
>> >> -                 cgit_line_range_override =
>> >> +                 cp = cgit_line_range_override =
>> >>                           window.location + '-' + t.substring(1);
>> >>           else /* backwards */
>> >> -                 cgit_line_range_override =
>> >> +                 cp = cgit_line_range_override =
>> >>                           window.location.href.substring(0,
>> >>                                   window.location.href.length -
>> >>                                   window.location.hash.length) +
>> >> @@ -115,6 +136,13 @@ function cgit_line_range_click(e) {
>> >>                                   window.location.href.substring(
>> >>                                           window.location.href.length -
>> >>                                           window.location.hash.length + 
>> >> 2);
>> >> + } else
>> >> +         cp = window.location.href.substring(0,
>> >> +                                       
>window.location.href.length
>> >-
>> >> +                                       
>window.location.hash.length)
>> >+
>> >> +                 '#n' + t.substring(1);
>> >> +
>> >> + cgit_copy_clipboard(cp);
>> >>  }
>> >>  
>> >>  /* line range highlight */
>> >> 
>> >> _______________________________________________
>> >> CGit mailing list
>> >> [email protected]
>> >> https://lists.zx2c4.com/mailman/listinfo/cgit
>> _______________________________________________
>> CGit mailing list
>> [email protected]
>> https://lists.zx2c4.com/mailman/listinfo/cgit
_______________________________________________
CGit mailing list
[email protected]
https://lists.zx2c4.com/mailman/listinfo/cgit

Reply via email to