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

Add copy button to demo site URL #57

Merged
merged 2 commits into from
Apr 29, 2024
Merged

Add copy button to demo site URL #57

merged 2 commits into from
Apr 29, 2024

Conversation

derekblank
Copy link
Contributor

@derekblank derekblank commented Apr 29, 2024

Related to:

Proposed Changes

Adds copy button to easily copy demo site URLs, using the same behavior for copying text on the Settings tab.

Screenshot 2024-04-29 at 5 18 36 pm

Testing Instructions

  1. Add a site from the sidebar, and then add a demo site from the new site's Share tab.
  2. Once the demo site is created, click the copy icon next to the URL (screenshot above).
  3. Observe that the URL is copied to the clipboard.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@derekblank derekblank added the [Type] Enhancement Improvement upon an existing feature label Apr 29, 2024
@derekblank derekblank requested a review from a team April 29, 2024 07:23
@derekblank derekblank self-assigned this Apr 29, 2024
Copy link
Contributor

@katinthehatsite katinthehatsite left a comment

Choose a reason for hiding this comment

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

The changes work as expected on my end! Nice work 👍

@wojtekn
Copy link
Contributor

wojtekn commented Apr 29, 2024

@derekblank, thanks for that improvement. I used to need to open the demo site in a browser to copy the URL, but now I can do that easily without that step.

My only concern is that the helper icon appended to the link/text causes a different action than the preceding link/text, and this behavior is different from links in the Settings tabs. @matt-west as you shared those concerns in the past, what do you think?

@matt-west
Copy link
Contributor

Let's keep the behaviour consistent with URLs on the settings pane. So the URL should be the regular text color, and clicking it copies rather than launching the browser.

@derekblank
Copy link
Contributor Author

derekblank commented Apr 29, 2024

Let's keep the behaviour consistent with URLs on the settings pane. So the URL should be the regular text color, and clicking it copies rather than launching the browser.

@matt-west Updated. The Demo Site URL button now matches the behavior of the Settings tab, where clicking it copies the URL (and does not open the URL in a browser).

copy-text.mov

@matt-west
Copy link
Contributor

Thanks @derekblank!

Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

LGTM 🎊 !

@wojtekn wojtekn merged commit 768686e into trunk Apr 29, 2024
5 checks passed
@wojtekn wojtekn deleted the feat/copy-demo-url branch April 29, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Improvement upon an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants