Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

feat(hyperlink modal): added message for successful copy of a link url #391

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nik72619c
Copy link
Contributor

Signed-off-by: Nikhil nikhilsharmarockstar21@gmail.com

Issue #379

Changes

  • Created a new styled-component for displaying the success message
  • added state to toggle the message on copying the icon for the same

Signed-off-by: Nikhil <nikhilsharmarockstar21@gmail.com>
@nik72619c nik72619c force-pushed the nik72619c-issue379-indicate-link-copied branch from dbe4241 to 3c55902 Compare May 1, 2020 21:18
@nik72619c
Copy link
Contributor Author

@DianaLease @irmerk @Michael-Grover please have a look :)

@Michael-Grover
Copy link

I'm not sure if this is entirely necessary. Google Docs doesn't do this but that's the only example I have had a chance to check. Do you know of any examples of a hyperlink copy confirmation in popular tools so we can see how they handle it?

If we choose to do this, I think the confirmation message should appear to the right of the icons so that the size of the modal and the position of the icons doesn't abruptly change
image

@nik72619c
Copy link
Contributor Author

thanks for the review @Michael-Grover . Making the changes !

@elit-altum
Copy link
Contributor

elit-altum commented May 4, 2020

@nik72619c If I copy a link once. The message will show. If I then change the link to some other value, and then try to copy it again in the same modal,the message still stays up and there would be no way for me to know that the new link was copied correctly or not.

Can we make linkCopied to false when the link value changes?
What do you say?

@jolanglinais
Copy link
Member

We're migrating this repository into web-components the next couple of days, so I'll be closing this soon and you should reopen there.

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

Successfully merging this pull request may close these issues.

4 participants