Replace clipboard.js with native Clipboard API — second try
#37810
Conversation
ciampo
added
[Package] Components
ciampo
requested review from
mirka,
ellatrix,
noahtallen,
jorgefilipecosta and
ZebulanStanphill
ciampo
changed the title
clipboard.js with native Clipboard API — second try
|
Size Change: -2.65 kB (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
@jorgefilipecosta I tested again on my local machine and the "copy block" and "copy all content" buttons seem to work: Z00ySe.mp4Would you be able to check yourself, since you reported the issue when reverting the original PR ? |
|
@ciampo Where in the editor can I give this a test? I'd like to quickly check it over for accessibility to ensure focus management is no longer needed. I'd say you are probably correct given how Clipboard.js works, but just want to be safe.
Thanks! |
|
Hey @alexstine thank you for helping out!
The 2 main places that were reported broken when last attempting this refactor are the "Copy" and "Copy all content" buttons in the editor (you can see them being clicked in the video posted in the comment above). You can also refer to the "Screenshots" section in this PR's description, where I've added more examples of where in Gutenberg you can tests for these changes. Hope this helps, let me know if you need more guidance! |
|
@ciampo A list of steps to navigate to these buttons would help. I can't see the videos/images so doesn't really help out here. Can you try to give me some context on how to find one of these buttons? Thanks. |
|
Hey @alexstine , sorry if I assumed that a video would give you enough context. In order to navigate to the "Copy block" button:
In order to navigate to the "Copy all content" button:
On a separate note, while preparing the written instructions above, I noticed that the changes in this PR work when tested locally, but seem to error when tested on a |
|
@ciampo The Copy in both Options menus did not work for me. Nothing was written to the clipboard. I did find this error if it helps. |
Thank you for the feedback, @alexstine — it's very much appreciated. I will circle back once I have updates on the issue! |
Hi @ciampo the issue still persists on this PR. If I press Copy all content nothing happens. The domain may be relevant I'm not using localhost or 127.0.0.1 on my development machine I use a test domain so that may explain why the issue happens for me and on gutenberg.run. |
Thank you for the feedback and the potential insight, I'll take a look and update this PR once I can find a more robust solution! |
ciampo
force-pushed
the
refactor/use-native-clipboard-api-try-2
branch
from
22d5e41
to
1f268d4
Compare
|
I think I've found the potential issue — the native I thought of 2 potential approaches:
Any thoughts? |
|
@ciampo Possible to fallback to clipboard.js when HTTP or is the goal to completely remove? Seems like not a good idea to just break it for HTTP sites. We couldn't even test this properly on wp-env. Just my thoughts. Thanks. |
|
Hey @alexstine , thank you for sharing your thoughts — it's very appreciated!
For me, the goal of this PR was to remove If we were to keep
In what sense you weren't able to test properly? I was personally able to test this PR on my local machine by running
I'm not personally up to date on this, but it would be good to know how many WordPress websites still run on HTTP instead of HTTPS? I would hope that the percentage is small (and shrinking every day). Finally, the fact that the native APIs only work in a secure context should also make us wonder if, by currently allowing users to read/write to the clipboard while on a HTTP connection, we're not potentially exposing any sensitive information that a user may copy/paste. |
|
@ciampo Is there a way to connect via HTTPS on wp-env? That is what I mean by not testable. I can't see if it works or not over HTTP. I understand the reasoning for the PR. I just want to make sure this can get a fair test. Thanks. |
I think this is the best path forward, in my opinion. :) I think if we need to support copy in HTTP environments, we can't move forward with this PR. If we don't, then I think option 1 would be a better approach. (Disabling the button seems better than a notice, especially if this hook is used outside of gutenberg.)
I believe it's possible, but it's not well-documented or supported, unfortunately... I haven't done it myself so I can't give much advice. It's unfortunate that |
Not sure if it's possible to test locally over HTTPS, but testing locally is possible because
Yeah, I agree. To recap:
Not sure who would be in a well-informed to help making this choice — maybe @gziolo can help? |
Description
Closes #37700
This is the second attempt, after the original PR #37713 got reverted
Changes in this PR:
useCopyToClipboardand the (deprecated)useCopyOnClickhooks in@wordpress/composeso that they use the native Clipboard APIclipboarddependency from the project's list of dependencies (saving2.55 kB (23%)for the@wordpress/composepackage)How has this been tested?
useCopyToClipboardhook work as expected (same as production) — see screenshots below for a few examplesIn particular, check that the issues flagged by @jorgefilipecosta in #37779 can't be reproduced
Screenshots
"Copy" menu item when editing any block in the editor:
copy-clipboard-refactor.mp4
"Copy URL to clipboard" button when viewing an attachment details in the media library:
"Copy URL" button when editing an attachment in the File block:
"Copy" button when reviewing a post's info after publishing it:
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).