-
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
Block bindings: UI for connecting bindings #62880
Conversation
Size Change: +3.24 kB (+0.18%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
1c7b5a9
to
0e267cf
Compare
0e267cf
to
f1f4b43
Compare
160ca47
to
7b5bccb
Compare
I'm worried about long custom field names which can be typical with many people who have large sites with many custom fields. How might those display and how flexible would the UI be for that? |
Oh also is the plan for this UI to eventually accept other registered custom sources where "Custom Fields" is an example of the heading of the source? If so, I think this would work well for other sources in general. |
We didn't think yet about how to group different sources, but the idea is to eventually accept other registered custom sources as you mention. Would this UI be enough for them (with a search field)? |
@jarekmorawski, @jasmussen, what do you think about the progress on the UI? It's looking close to what you shared in #39831 (comment). In particular: bindings_panel.mp4 |
I'm heading AFK for some summer AFK soon, so won't be able to follow this that closely. If Jarek has the bandwidth to follow this work, I trust him to have to have good instincts, I know he has WooCommerce experience too. One quick glance: If a menu opens from the block toolbar, it should have the dark border and not the shadow menu style, just like all other block toolbar dropdowns. I thought this was automatic (CC: @WordPress/gutenberg-components), but otherwise I think it might involve an That's a general thought too, start with as little as possible, get a feel for it, then add on top of this only when those things become necessary. |
Thanks, folks! I'll do a few more iterations sometime this week and keep you updated on the progress. |
This is true AFAIK, the only thing I'd add is that |
Flaky tests detected in 90da57a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10091266598
|
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. |
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.
Avoid custom CSS as much as possible. Tool Item component may have a helper attribute so we avoid adding plain HTML. Adding an option for items to remove their margins, without having to tweak borders. Pinging @WordPress/gutenberg-components for feedback about this.
I'd personally need to look more into it to understand fully the request. Pinging also @aaronrobertshaw who originally authored the component.
I believe the issue you are facing is not because there are no sources (by default
I am not sure if this is the expected behavior of ToolsPanel. It seems to be the case for other styles using it. For example, if I go to paragraph styles and I click on the three dots of Color or Typography, I add an option, and I move to a different block, it is not there anymore. I understand it like this: "You started creating a connection, you didn't finish it, so it is not created. It doesn't show until you create it again". Having said that, I don't have a strong opinion on what should be the behavior, but I believe it is not specific to this pull request.
It seems the keyboard navigation of the dropdown is not working as expected. Although I believe it is related to the DropDownMenu component we are using, because it doesn't seem to work in the docs examples either. Steps to reproduce: Go to the component docs -> Navigate to the "Open Menu" button with the keyboard -> Press "Enter" -> Try to navigate through the items. @ciampo Is this a known issue? Should I submit one, or do you have any clue how to solve it? EDIT: Maybe this is expected and users can just navigate with the arrows? 🤔 It seems to be the case in Ariakit Menu component.
Yes. The idea is to explore how to support other sources in follow-up pull requests. |
It seems the last remaining decision here is: Should we move the UI to the Advanced panel?This was suggested in this comment and in this WordPress Slack conversation. Basically, the suggestion comes from the fact that it seems an advanced feature. Apart from that, moving it to the advanced panel wouldn't change the default tab of blocks like paragraphs and headings. As explained as part of this comment, with the current implementation, when you create a new paragraph it shows the "Settings" tab instead of the "Styles" tab because we are adding the "Attributes" panel. Whatever we decide, the position of the panel can always be changed later. It seems moving it to the Advanced section would be "safer" and would give us time to decide how to handle the edge cases. On the other hand, I believe there is no precedent for using the How it looks outside the Advanced panel (current implementation) How it looks inside the Advanced panel by default How it looks inside the Advanced panel with custom CSS Any thoughts? |
I'm not sure it's a good idea. If we want to minimize the footprint of this section, we can remove the help text and rename it to My argument against putting it in The section is collapsed by default and contains features that are rarely needed for daily use. It's also quite consistent across different blocks so if people aren't used to looking there while editing bindings-enabled blocks, they may never discover the feature. Lastly, the Advanced section is always placed at the bottom of the settings panel, which may cause information hierarchy issues. Applied connections will affect the fields and data above which may feel odd and backfire at some point. For instance, a user wanting to insert the post title in the Image block's alt text may start looking for such a setting in the Settings section which would be placed high above Connections tucked away under Advanced. Going all the way down there only to be forced to scroll back up to see the result may throw people off. |
Can you please elaborate on what doesn't work as expected? In my testing, keyboard navigation works as expected. |
It looks like it would help to see the steps used on both ends to cross-check the expected behavior. |
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.
50f6f1e](50f6f1e) - with that it’s behind the experiment. Let’s land it and decide separately when to expose it to the users. In my opinion, it’s going to be really soon, even before the next Gutenberg RC1 planned for next week. However, if this is the confidence necessary then I’m happy with it. It could be that e2e tests will need some updating to account for the experiment UI.
@tyxla I was trying to navigate the menu items with "SHIFT" and it seems I need to use the arrows instead. As said in the edit, maybe this is expected because it is how Ariakit Menu component works.
I agree this could be improved. I suggested something similar in this comment: "Should we add a prompt or helper message when an attribute is not connected?".
That should already happen in this logic. When there are no custom fields to connect to, it will show only the panel when there are bindings created.
As it seems there are some concerns, I moved the UI under an experimental flag. I feel landing the prototype will help moving forward the follow-up discussions, fixes, and pull requests. |
Thanks for confirming, this is exactly how it's supposed to work - with arrows, and you can also use SPACE and ENTER for selection and navigating between levels (in the nested version). |
To add to what Marin said, that is how |
As it is now wrapped under an experimental flag, I went ahead and merged the pull request. I'll add the follow-ups to the tracking issue and we can discuss items separately. Thanks everyone for all the feedback and the great work 👏 |
Hey all 👋 Is is correct that the bindings UI only shows the core meta binding sources as options to select from? Custom sources registered via the PHP CC: @SantosGuillamot |
Yes, that's correct. The UI is specific for post meta at this moment. We explored the possibility of extending the UI to other sources, but we didn't consider the APIs stable enough to open them for 6.7. It will probably be one of the focus for 6.8, but we need to ensure whatever is built is solid enough to avoid potential issues in the future. |
Thanks for confirming :) That is another detail that I think will be really important to include in the Dev Note for the block bindings feature :) |
I started exploring the possibility of providing the APIs to extend the UI in this pull request. I plan to work on another one on top of it to provide a "default" UI for sources only registered in the server. |
What?
Closes #62944
This PR adds the capability of connecting attributes with custom fields via block bindings. Previously, the only way to do it was by updating the post content via the code editor.
The PR uses the
Tool Item
component to show and enable different attributes per block, andDropdown components
to select thepost meta
that will be attached.The block bindings code has been moved to the
editor
package, as it will fetch post meta data, that should not be a dependency in theblock-editor
library. While this feature is per block, it uses general information from the editor.There is a list of follow ups to this work, but I don't want the PR to get bigger and harder to review:
getters
andsetters
to anuseFunction
. It will need to use some hooks for accesing the block store, so moving to an utils folder is not enough.Button
,Heading
andImage
blocks.openDocumentSettingsSidebar
to only open the settings sidebar, and create anopenDocumentStylesSidebar
option. I had to update some tests that were assuming that, if there are no settings for paragraph, the style editor should open by default.How to test
Register post meta sources.
Use the UI to link them to paragraphs, buttons, images and headings.
Screen.Recording.2024-07-18.at.16.22.20.mov
Screenshot