-
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
Adjust the icons and text of the binding connected blocks #61560
Conversation
Size Change: +257 B (+0.01%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
{ ( toggleProps ) => ( | ||
<DropdownMenu | ||
className="block-editor-block-bindings-toolbar-indicator" | ||
label={ firstBlockTitle } |
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.
The aria-label
here doesn't match the visual label below. This is using the block title's name, but the visual is using the block's metadata.name
. Not sure about the accessibility concern here and how we should fix it. @richtabor Any idea?
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.
I don't think we need a visual "Override" label.
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @jarekmorawski. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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 don't think we should use the override/connection name in the toolbar. It's enough to surface the details in the popover. |
I also think the copy in the proposed issue is a bit clearer and informative (with lower case block name as well). |
@richtabor @jameskoster @jarekmorawski Is there a conclusion about the design direction we want to go in? Wanted to flag as well that the 6.6 beta is rapidly approaching (a week on Friday for Gutenberg), so might be good to think about it in terms of what's achievable within that timescale, and what we can iterate on for 6.7. |
Thanks for the ping. I just discussed with @artemiomorales what we might have in time for next Friday, and IMO our opportunity here is to ship block bindings with some of the base UI that we know we need, and then learn not only from beta feedback, but from what people do in the 6.6 phase, to build out any additionally needed affordances for 6.7. Especially some of the pieces that have been proposed for the block toolbar itself have received various bits of critique both across design and accessibility, which to me suggests that could be a piece to refine for the next release. Things like the use of purple, the dot indicator, and more. So what could a V1 look like? The block inspector is the most prominent piece, footprint wise, and the main interface for block connections: It's unclear whether folks who use block bindings would close the inspector, given it's a slightly more advanced than usual flow, but if they did, the pre-publish flow would surface if they had edited bound blocks, and give an option to uncheck that: It would benefit the flow to surface connections in more states. When a URL is connected, show it as such in the URL popover. When an image URL is connected, show that in the image dropdown, and so on. It's less clear how far we can get with those in time for beta 1, but a minimal version could be this: That mockup intentionally omits colors and other affordances added already in trunk. That's not necessarily a suggestion to pull those out, I'd keep those at least for beta 1, but then in that period, learn, and refine, and then decide whether to keep those pieces, or refine them in 6.7. What do you think? |
Is the I wonder if the Connections panel could include the 🔌 icon somewhere, to conceptually connect (pun intended) that panel with the locations in which the icon appears. Perhaps in the title, or per-row like color swatches and other similar UIs? In the save panel there's a 'Block bindings' item. Could this be 'Connections' to align with the panel (or vice versa)? |
Had the same thought. Also, the unblock icon. |
Yes, that could work. Feel free to tinker in the Figma. |
7542ac6
to
450b3f5
Compare
It's already a button that does nothing, with a tooltip of the block name. |
We don't need that button/application when editing the synced pattern entity directly. Results in a double-icon: For this initial iteration, let's have this modification apply only in the context of overriding, rather than editing the synced pattern itself. I'm ok with a fast-follow bug fix for this as well. |
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.
There are a lot of ideas on this, but the centered concept of using the block icon (purple) with a popover referencing details on what the block consists of is an improvement.
Let's follow-up with a fix for the doubled icon when editing a pattern in focus mode (#61560 (comment)), but this looks good to me.
I run into an issue when testing Block Bindings in the site editor - homepage template. I know that establishing a post meta connection isn’t the best choice there but regardless, we need to prevent this type of accidental errors: Screen.Recording.2024-06-03.at.12.54.17.movIt seems to be related to the changes introduced in this PR. |
It seems the issue is caused because at some point Object.values( block?.attributes?.metadata?.bindings || {} ) |
I started this pull request with the potential fix. Although I must say I don't have the full context about that code. |
…61560) Unlinked contributors: jarekmorawski. Co-authored-by: kevin940726 <kevin940726@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org>
…61560) Unlinked contributors: jarekmorawski. Co-authored-by: kevin940726 <kevin940726@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org>
What?
Close #61515 and #61547.
Adjust the icons and the text descriptions of the binding connected blocks (pattern overrides, connected post meta, etc.)
Why?
The generic connection icon is not that helpful and the block type names should be helpful.
How?
Adjust
<BlockBindingsToolbarIndicator>
.Testing Instructions
You can paste the markup here to create an empty post-meta connected paragraph.
Screenshots or screencast
A pattern overrides block
Multiple pattern overrides blocks
A post-meta connected block
Multiple post-meta connected blocks