Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/uniquelink #37

Merged
merged 11 commits into from
May 26, 2020
Merged

Fix/uniquelink #37

merged 11 commits into from
May 26, 2020

Conversation

rooberrydev
Copy link
Collaborator

branched off #34 so merge that first.

reloading the uuid url now works, and loads comments as well except for one bug #36
fixed invite button.

the invite button now copies the invite link to the clipboard and shows a success popup.

Copy link
Collaborator

@mbanerjeepalmer mbanerjeepalmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Anonymous user' is maybe a bit boring - perhaps even threatening? Could be worth going back Wiki again on guiding principles. One was that random anonymous commenting is what makes things seem toxic. Maybe we set the user a random name?

Also maybe worth documenting the reason for the choice of the clipboard library over native browser APIs [1]. We're only copying to the clipboard and not doing much else, so the library could be surplus.

[1] https://commentative.herokuapp.com/09cdffed-f4b2-455f-a90b-e145f47bbc68 (original https://developers.google.com/web/updates/2018/03/clipboardapi)

@mbanerjeepalmer mbanerjeepalmer merged commit 756eb84 into master May 26, 2020
@rooberrydev
Copy link
Collaborator Author

Yep that's definitely going to change,just put that in for the sake of having something there(it was a pen emoji before) I'm still unsure what the best place to prompt the user for a name is.

@rooberrydev
Copy link
Collaborator Author

Clipboard library was for browser support tbh, async clipboard api is fairly new, and document.execCommand("copy") requires text to be selected. In our case we don't really have text, the button has an attribute which has the link as its value.

Saw a couple hacky solutions to temporarily append a textarea to the dom, copy text from it, then delete it.

Seemed unnecessary.
I'll check again if there's an easier way to implement this for out specific use case(button to copy the current url to clipboard)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants