On June 25, 2018 12:03:56 AM GMT+08:00, John Keeping <[email protected]> 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
><[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.
>
>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 <[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
