-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve override indication for editable blocks in synced patterns #60599
Conversation
Size Change: +898 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
I'm into this, seems a big improvement over the dashed outlines. I can't test in more detail at the moment, but just a quick thumbs up for now. |
I like this, it's an improvement, but sometimes the overlay isn't super clear, typically when applied to images or when the background is darker. Would it be overkill to include a solid border too?
How come? Shouldn't they be the same? 🤔 |
I think a better empty placeholder for core/post-content within Site Editor pages would work much better than highlighting much of the page with purple (nor outlines, like it is now): The UX issue on the pages front is that it's not clear where on the "Add new page" flow you actually start adding content. Here's what we have in trunk; the outlines in trunk, or proposed background animations, don't solve for that: |
Yea, that's the one drawback. It feels a bit so with both. And with just a border it's intermixing with the hover/selected styles, making it feel like the block is selected temporarily. |
I think we can try it with varying degrees of opacity/visual effect, but if we're good with the concept I'd like to get this iteration in, to keep driving the related parts of #60286 forward. |
I tweaked it a bit, to work better with dark backgrounds. What do you think of this @jameskoster? CleanShot.2024-04-10.at.11.17.16.mp4 |
The dark background challenge is a bit of a misnomer as the outlines have the same problem. Can you use the inspector on that pattern and see if there's an instance of an |
I think the border helps a little, but yeah, definitely not perfect. From a quick look it seems You'll probably think it's too much, but a simpler alternative could be a stronger initial state in the animation. This is outline opacity .9 and bg opacity .3, which makes it super-obvious: indocator.mp4 |
It's not localized per block/pattern, only per useDarkThemeBodyClassName, based on the site's background color. |
Was coming here to call out the same issue, especially with a dark background. I wish we had a way to pick up on the darker background and change the highlight color that's used. I also noticed some oddness imagining being a user and trying to double click on a field I can't override: Screen.Recording.2024-04-10.at.4.48.01.PM.movInstead of just surfacing the blocks I could edit, some weird selection happened with this PR. |
The thing here is: before we didn't have anything, and I found that to be a better experience than this. The overlay is a step in the right direction because it isn't about adding blinking elements in the UI, it's about reducing them. IMO the next step wouldn't be to add borders, but to revert the original addition of these and explore other solutions, such as diming surrounding UI instead. |
Probably fine to remove the border if the animation begins with a more opaque overlay like in #60599 (comment). Spotlighting is worth a try too, but I'm curious how would that work? If you select a pattern, would non-overrideable blocks be dimmed, or would it be a temporary on-click thing? Obviously it would need to play nicely with Spotlight mode itself. |
I've undone the outline attempt. If we're good with the premise, let's get this merged to further explore related alternates. I do think as-is this is a clear step forward. Any objections? |
eb344ce
to
175517a
Compare
175517a
to
ea2c620
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Andrei Draganescu <[email protected]>
Co-authored-by: Andrei Draganescu <[email protected]>
What?
A piece of #60286.
Removes the previously implemented dotted border animation in lieu of a subtle background animation, to help indicate which blocks within the synced pattern are editable (have overrides). Too many outlines make it difficult to discern what is being communicated.
Testing Instructions
Screenshots or screencast
Before
CleanShot.2024-04-09.at.08.23.40.mp4
After
CleanShot.2024-04-09.at.08.13.15.mp4
Note
This effort scopes the effect only to synced patterns. Editable blocks within a template will not have this effect, or the previous outlines, as seen below:
CleanShot.2024-04-09.at.08.27.56.mp4
Related: #57901 and #58159