[PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click

[andy at warmcat.com (Andy Green)]

On June 24, 2018 9:39:35 PM GMT+08:00, John Keeping <john at keeping.me.uk> 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
><john at keeping.me.uk> 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 <andy at warmcat.com>
>> >> ---
>> >>  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
>> >> CGit at lists.zx2c4.com
>> >> https://lists.zx2c4.com/mailman/listinfo/cgit
>> _______________________________________________
>> CGit mailing list
>> CGit at lists.zx2c4.com
>> https://lists.zx2c4.com/mailman/listinfo/cgit