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

replace clipboard with native browser support #7392

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zoran995
Copy link
Collaborator

What this PR does

Fixes #6774

Replace clipboard.js with native browser clipboard support. Browsers support native clipboard events for quite some time, while clipboard.js still uses old deprecated method, which might be removed at any point from browsers. There is no need for

Test me

Try copying the share URL in the app, it should properly copy the URL and show success for 3 seconds. I wanted to write unit tests for this, but we are using react-test-renderer, which is deprecated and does not offer convenient ways to test this. We need to see how to get the testing environment to a more modern stack.

Checklist

  • There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist) - written above
  • I've updated relevant documentation in doc/.
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

@zoran995 zoran995 force-pushed the native-clipboard branch 2 times, most recently from 18bb888 to c71bb92 Compare February 13, 2025 10:53
@na9da
Copy link
Collaborator

na9da commented Mar 5, 2025

@zoran995 - thanks for the improvement.

One thing I realized when testing from our CI is that navigator.clipboard only works in a secure HTTPS context. I'm not sure this is a feature we currently want to break for users in (insecure) HTTP mode. So would it be possible to implement a fallback like the one in this stackoverflow answer? We could still remove clipboard.js dependency.

@zoran995
Copy link
Collaborator Author

zoran995 commented Mar 5, 2025

The main idea behind this was not to depend on deprecated APIs such as document.execCommand. Perhaps we could implement a fallback with a deprecation notice to give users time to migrate to https, but these days https should be standard (browser probably would never remove such old feature until it blocks them in implementing something modern)

@na9da
Copy link
Collaborator

na9da commented Mar 5, 2025

I sort of agree.. also it seems to work in localhost where we use this feature quite a bit during development.
I think if we go ahead with this, we should show the Copy button only when navigator.clipboard is available. Otherwise let the user manually copy it.

@zoran995
Copy link
Collaborator Author

zoran995 commented Mar 7, 2025

Hi @na9da, thank you for the suggestion. I have removed the copy button when the clipboard is unavailable, but would it be better to disable it instead of removing?
I have also added tests using @testing-library/react, we already talked about using it in future, and it will be needed for other components as part of the react upgrade

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.

Replace clipboard.js with native clipboard API
3 participants