Add paste styles to the block settings #45477
Conversation
|
|
|
Size Change: +960 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
kevin940726
force-pushed
the
add/copy-paste-styles
branch
from
f4ad923
to
9b203f0
Compare
|
Maybe I'm doing something wrong, but I've tried this with three different browsers and none of them ask me to give them clipboard access, so when I click the paste option I get the |
Oops, I guess it's because the API requires secure context (https) to work. I should update the error message in that situation. An exception is if you're browsing the page in |
|
I've given this a bit of a run. It's testing well so far nice work @kevin940726 👍 Not receiving the request for clipboard access initially caught me out as well.
While the ability to avoid a "Copy Styles" button is much cleaner. I wonder if that makes this more of a power user only feature? Is there a way we could assist the average user to discover this easier?
If we can't rely on attributes being style attributes if they don't have the
This sounds like a good starting point.
This matched my expectations. It would be a tricky one to give any kind of visual prompt or feedback around though. |
|
Here's what it looks like when pasting styles from a pattern. Kapture.2022-11-03.at.18.24.00.mp4 |
There was a problem hiding this comment.
It works really well for me, but I think it just needs some more thought around what attributes are copied. I left a comment about using __experimentalRole: 'style' (which was also mentioned in the PR description)? It would be good to get thoughts from other @WordPress/gutenberg-core contributors on this.
May also need design feedback on the PR as this implementation deviates from the proposal in the issue.
|
Took it for a quick spin, in this GIF I get a clipboard error message:
I do appreciate the idea of unifying "copy styles" into the "copy block" option for simplicity. It opens a few questions:
Item 3 might necessitate a dropdown menu flyout, so we can provide additional context which otherwise would take up too many slots in the menu perhaps. Here's what Figma does, which feels like a useful pattern to draw inspiration from:
CC: @javierarce in case you have input. |
|
@jasmussen The clipboard API requires secure context (https) or localhost, are you using a custom domain without https?
I figured a block should already contain the information about the style so the "and styles" feels a bit redundant to me. But of course that can go either way.
No, we can only read clipboard data (or request permission) on user interaction. It's a privacy issue on the web and I'm afraid there's nothing we can do about it. (We could simulate the clipboard API using JavaScript but that only works in the same tab, it might be confusing for data copied from other tabs/apps.)
I'm wondering if we should rename "Paste style" to "Paste style only", as I think a "block" should actually be "block content" + "block style". This might be less confusing let's say when we decide to add a "Paste content only" button as seen in many other apps. |
Ah, I'm using Local to test, so that's why it doesn't work. I wonder if it's better to hide or gray the option, if https isn't available? Probably not, but maybe we can simplify the snackbar to say something simpler, such as "Pasting styles requires a secure domain (HTTPS)" ?
Main reason I'm waffling on it is, there's a "Paste styles", but no "Copy styles" option. I'm wondering if it may still be simpler to have a separate and additional "Copy styles" just for this 🤔
👍 👍 — to me that mainly says that it would be all the more helpful to have the nested dropdown flyout menu.
Copying a block should definitely also paste the styles. But pasting styles should not necessarily paste the block. So in that light, maybe we should add separation and do it like this mockup after all, and keep "copy block" and "copy styles" separate after all. It may be a better starting point and then we can come back to it when we have nested dropdown menus. Curious what you think? |
Sure! Welcome to tweak the wording in this PR!
I don't think it's weird though, it's how Google Sheets does it too. If we convey clearly that "blocks" contain both "contents" and "styles" then I think it makes total sense to only have the "Copy block" action. If we add a "Copy style" action then it's gonna behave exactly the same as the "Copy block" action which is a trade-off we would need to take. Since clipboard on the web is linear, we also need to decide what happens when we paste the copied styles to other apps. In contrast, I think the single "Copy block" action is a lot more straightforward. The downside, however, is that it's probably not easily discoverable for users who are intentionally looking for the "Copy style" action, because they see the "Paste style" action. I think tweaking the wording to "Paste block style only" would help, but it'll be best if we can do some real user research on this. |
There was a problem hiding this comment.
Unfortunately it doesn't seem to be working as expected for me:
Kapture.2022-12-08.at.11.15.35.mp4
This is copying a heading with background and text color, and pasting styles to another heading.
The code looks pretty good though, I left some feedback about reorganising the hook functions, and possibly those new functions could also be used in those hooks files when there's a manual hasBlockSupports check (e.g. https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/hooks/align.js#L96). Updating that could also be a follow-up if you want to keep things atomic.
I realise there might still be some design iteration to go here, is there a clear direction for that?
I don't think there is! I guess we have both voiced our opinions on this, so we might just need a decision-maker to chime in and make the call. We can always experiment with other alternatives in future iterations. |
|
Testing again, and it works very well now since that last commit fixed the earlier issue. I guess this just needs some final direction on the design, since it seems this implementation is quite different to the original mockup. There do seem to be a lot of technical constraints which need to be taken into account. It might be worth summarising what those are @kevin940726. |
|
Agreed! We want to make a decision about the final design (of this PR) of the user interface. We currently have two primary options for displaying the actions:
I've summarized an overview of the pros and cons in a comment here: #45477 (comment).
I'm open to either decision! Another note is that if we choose the separated actions approach, do we want to only copy the style or should we also copy the block content when users click on "Copy style"? The latter will behave exactly the same as the "Copy block" action, while we can disallow anything from pasting to other apps with the former approach. |
I did a bit of research, and it seems like the clipboard I think it could be unusual to have 'Copy block' and 'Copy style' do the same thing (but maybe users would never notice?). It would probably still be preferable to have 'Copy style' followed by a 'Paste' output some weird json data. The alternative is to not use the clipboard API, which disallows pasting across tabs or browser instances, but it maybe it's an ok trade-off if it results in the rest of the experience being better. Are there any other trade-offs of not using the clipboard API? |
AFAIK, Chrome currently only supports three MIME types:
I think ultimately we would want to enable this feature from different tabs, like copying styles from wp.org/patterns for instance. Limiting this only inside the same browsing context would probably confuse the users. It's still an option to mimic the clipboard API in JS, but I'd say it's not worth it to do this rather than going for the "Single action" option in this case. |
With an eye towards potential flyouts in the future (#44418), of those two options I would go with the left one. Outside of clarity, what if I copy the wrong style? I would have to paste it first in order to be able to copy a new one, with the right side. |
I think there's a misunderstanding. You don't need to paste it first to copy a new one. You can just copy a new one and it will override the existing copied data in the clipboard. In short, what the right side (or this PR) does is to automatically extract the styles from any given copied block data, so that we don't have to add a separate action. This means if you already have some blocks copied, you can paste their styles to other blocks without having to copied their styles separately. This works across browsing-context or apps too, so if we ever want to allow copying styles from wp.org/patterns, we can already do that with this approach with no changes required. While for the left one, we would want to introduce a separate "Copy pattern styles" action there to enable the same functionality. |
Ah, right. I'm still leaning towards keeping these actions separate. It's not an incredibly strong opinion, but the feeling is that we're being too smart for the sake of saving some space, which ultimately I think we can address with a flyout in the future. For some additional takes, CC: @WordPress/gutenberg-design |
FWIW, the goal is never to save some space, and the part that's "smart" is quite essential for this feature and has to be implemented one way or another for the "Separated actions" approach anyway. The motivation is to avoid confusion when we want to allow pasting styles from other browsing-context in the future. It's the same idea as shown in Google Sheets' "Paste special" > "Format only" action.
They also only have a single "Copy" action and people seem to be pretty used to it too. As you can see, we might also add other pasting actions like "content only" in the future, where a flyout/nested dropdown would be valuable. |
kevin940726
force-pushed
the
add/copy-paste-styles
branch
from
90cb88b
to
991f685
Compare
|
Flaky tests detected in 991f685. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3871581606
|
|
Hi! Is there a possibility to add an exception to secure connection requirement if a local development / testing domain is used? For example: http://test.local/ |
|
@srcek The feature uses the browser's clipboard API, and that's the limiting factor, rather than anything controllable by Gutenberg. There's more info here - https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts. It seems like you may be able to workaround the issue by modifying your hostname to something like 'test.localhost' instead of 'test.local'. |
|
Thank you for clarifying. Changing the hostname to test.localhost solved the problem. |
|
Hello! Is there a way to disable this option for specific blocks? |
I don't think so. What's your use case? |
|
Because I am using 3rd party block called Stackable and the attributes are not being copied whenever I paste them into another Stackable block. It also has its own copy-paste styles feature; so I was looking for a way to disable the option to prevent others from using it whenever they edit Stackable blocks. |
|
I think the problem lies in the As for actually disabling the feature entirely, I think we'll need more time and cases to come up with a solution that doesn't confuse users. Please share more feedback if you have any! |
|
I have added an explicit reference to the browser permissions issue in the "More Options" end-user documentation. I adding it here in case someone looks up the exact messaeg I got in Chrome.
|
|
Safari also requires a confirmation; it looks a bit different — not sure if it should be documented though. |
What?
A partial solution to #44418. Allow pasting styles of the copied blocks to the selected blocks.
Why?
See #44418.
How?
I don't think we need a separate "Copy styles" action since that the copied blocks should already contain that information. Instead, we can only provide a separate "Paste styles" action to selectively paste/apply only the styles to the selected blocks.
Which attributes should be considered "styles attributes" is debatable. This PR mark all attributes as styles attributes except for the ones that have
__experimentalRoleofcontent. We can discuss it and further refine this logic though. One possible idea is to assign__experimentalRole: "style"to the attributes.pasteStyles()will recursively handle the inner blocks but it won't check for block names. That is, the common styles will be applied if the copied blocks are different from the target blocks. This decision is made because most blocks have many common attributes like padding/margin or text color. We can discuss it further and decide if we want to only allow pasting styles to the same block type, though.If there's only one (top-level) copied block, all the selected blocks will be applied to the same styles. Else if there are multiple copied blocks, only the minimum common blocks will be applied to the styles. This could be kind of confusing sometimes, but I think it matches what the users expect most of the time.
Worth noting that for security reasons, reading from the clipboard requires additional permissions. If permission is denied, we'll show a warning snackbar to notify the user. Different browsers have different ways to ask for permissions natively.
Testing Instructions
Screenshots or screencast
Kapture.2022-11-02.at.14.19.50.mp4