[PATCH v3 6/6] line-range-highlight: copy URL to clipboard on click
Andy Green
andy at warmcat.com
Mon Jun 25 02:46:06 CEST 2018
On June 25, 2018 12:03:56 AM GMT+08:00, John Keeping <john at keeping.me.uk> wrote:
>On Sun, Jun 24, 2018 at 11:06:45PM +0800, Andy Green wrote:
>> 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.
>
>And what about the piece of code that was cut into the clipboard just
>before the interruption?
The guy is clicking the link to get a highlight...
... which is a new feature introduced in my patches last week and not in mainline...
... so this wasn't the first time he clicked on line-number links in the Dark Timeline where this patch was accepted...
... so a little animation of a clipboard icon fading out would be enough to have taught him highlight = clipboard now.
From my POV the only way to make progress here is provide patches and align them to comment. There are lots of choices in how to implement stuff, but just talking about it before any code isn't very interesting for anyone (I did try it last week). Two people won't write something nontrivial the same way, I didn't spend much time in the project code, so all I can do it provide a working reasonable implementation so people can see light at the end of the tunnel already, and wait to hear what people objected to and refactor or adapt. This patch is no different.
I appreciate the time you have spent providing the feedback to make the alignment part possible.
>> >> 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...
>
>That's fine, it doesn't overwrite the clipboard with no notice (and
>right-click still works). My point isn't "we can't provide a new way
>to
>copy to the clipboard", it's "we can't overwrite the clipboard behind
>the user's back".
It becomes a bit more of an abstract concern if we inform the user that's what clicking on the line numbers does, with an animation.
Anyway this seems the middle way forward -->
>> 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.
>
>This sounds good. Copying the line range link is clearly useful and
>this way the user knows that it's an option and controls when it
>happens.
OK then I'll look at it today.
-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
More information about the CGit
mailing list