New feature: copy permalink to clipboard

[Jan-Ka]

Preview link

This is a first step for implementing #23 . And as far as I can see all the functionality is there. One might call this: the minimum viable product :)

I put the "copy shareable link" button beside the existing "edit article" link and choose an appropriate UTF-8 symbol (however I think both edit link and copy share link should get dedicated svgs if I'm honest).

I would really like to use a modal dialog similar to the one github uses but I think this would add almost as much code as the rest of primerpedia itself. Css is not my strength, do you have any input @waldyrious ?

[waldyrious]

Nicely done. The choice of Unicode character was the appropriate one in my view, but I can see how a custom SVG could work better. I'd rather leave that for a future change, though -- I'm not entirely sold on abandoning the simplicity of a single pictorial character.

I don't think we need to replicate GitHub's behavior, but I do feel we need a notification that confirms that the link has been copied to the clipboard. Briefly fading in and out an absolutely-positioned banner at the top of the page (a common UI pattern these days) might be a nice, mobile-friendly way to do it. What do you think?

[Jan-Ka]

Yes, I would have liked to implement such a behaviour but I'm not up to snuff on css animations. Might be a good opportunity to try.

However I would suggest putting that in a new enhancement issue.

[waldyrious]

I would suggest putting that in a new enhancement issue.

Makes sense. Just to be clear, are you talking about the feedback banner I suggested, or the github-like copy dialog?

[waldyrious]

I've rebased the branch to fix the conflicts.

[Jan-Ka]

I think we have different things in mind for the success message. I thought that the maximum implementation would be a github style modal dialog, a lesser form of that would just be a animated notification popping up from the button when pressed. Something like the basic implementation of notify.js

When you say "feedback banner" do I understand correctly if I think of a side spanning header popping up for a short while after pressing? Like this example from css-tricks . I think I could do that :)

[waldyrious]

When you say "feedback banner" do I understand correctly if I think of a side spanning header popping up for a short while after pressing? Like this example from css-tricks.

Yes, exactly! The notify.js and github approaches seem overkill for this feature / project IMHO.

I think I could do that :)

Just to be clear, do you mean in this PR, or as a separate enhancement?

[Jan-Ka]

Ok, perfect. I think I can whip something up in no time. Should I add this to this PR or make a new one? Derp, just saw that you asked me the same. I would put this in a new PR as enhancement.

[Jan-Ka]

I do not have any particular documentation on the topic but the hash is used because it won't trigger a page reload ever (For that reason angular as example uses it to store the internal location). Please correct me if i'm wrong.

Additionally I use it because It indicates that this href is not left empty but deliberately points to the current page which adds to the expressiveness of the code. Not sure if this is still best practice but I did not see anything to the contrary in the last few years.

[waldyrious]

the hash is used because it won't trigger a page reload

Ah, right, I forgot that detail. An empty href indeed tells the browser to load the root page.

I don't have proper documentation at hand either, but IIRC the use of href="#" started out as more of a hack to workaround default browser behavior, than a deliberate choice with the primary aim to indicate a semantic meaning (although I'm sure that was seen as a nice side effect).

I believe that if you remove the href attribute altogether, the <a> element will work as a plain anchor —i.e. a non-clickable link target—, which is in line with its standard intended use. (Since it's not clickable, the cursor doesn't change to a hand, but that can be fixed with the CSS rule cursor: pointer.)

I'd prefer this approach than appending a # to the URL, but I'm open to hear arguments to the contrary.

[Jan-Ka]

Sounds fine to me.

[Jan-Ka]

I added the discussed changes. Also I did a rebase as per the primerpedia/CONTRIBUTING.md document.

Did I do it correctly, @waldyrious ?

[waldyrious]

I hope I didn't put you in too much trouble -- I expanded a bit on my motivation for the git workflow I mentioned in CONTRIBUTNG.md in this comment.

In that light, for this branch some of the changes would be folded into earlier commits, while others are justifiably kept separate. I'll rebase this branch to illustrate (and also to fix the conflicts). Please take a look and let me know if it makes sense to you.

[waldyrious]

Here's what I did on the rebase:

• I changed 177065b and a5fe979 to not include the inconsistent quotes in the first place, thus obviating the need for 76609be;
• I changed the commit message of a5fe979 to include a blank line between the title and the body of the commit message, as is recommended by git itself
• I merged commits 6552232 and 3947c1c, since the former would represent a visible regression without the latter. Compare this to changes like 650a0da, which introduces a functionality which only gets used in a later commit -- in this case af081e1 and abde6ef. In such cases, an inert new functionality is acceptable as a separate commit.

[waldyrious]

On a note more related to the contents: how is the modal dialog supposed to show up? I don't see anything when I click the button (but the URL does get copied to the clipboard). I'm using Firefox v56 on Linux. I tried with Chromium v62 in the same system, and the results were the same (no visual changes on the page when I click the button).

[Jan-Ka]

Thanks for documenting and explaining what you did while merging, greatly appreciated. I'm just hesitant with merge/base as I'm not yet comfortable enough experience wise, but this helps a lot.

About your Question: Currently the Modal Dialog does not show up (at least for the user) since we would require additional code to handle position etc. The container is set to visible until after execCommand("copy") since there can be problems with some older browsers which refuse to copy something that is not visible in the moment of execution. On (really) slow machines you might get a glimpse of it showing up at the bottom of the article.

The name is probably misleading since we currently don't show a modal dialog.

[waldyrious]

Understood. In that case please change the name of the element, or at least add a comment explaining its intended behavior.

[Jan-Ka]

Added the discussed changes @waldyrious ! Sorry for the delay.

[waldyrious]

Sorry for the delay.

No problem at all! This is a volunteer project -- it's meant to be done whenever we have the time and desire to work on it :)

I just merged the change. Thanks again for taking the time to implement one more nice feature :) Before merging, I cleaned up the branch lightly, essentially to integrate 38c3e30 into the previous commits, for the same reasons as explained above.