Skip to content
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: Add bindings panel in the block inspector controls #61404

Closed
SantosGuillamot opened this issue May 6, 2024 · 17 comments
Closed
Assignees
Labels
[Feature] Block bindings Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.

Comments

@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented May 6, 2024

This is part of a broader effort to improve the block bindings UI.

What?

Add a new panel in the block inspector controls to indicate the attributes that are connected in a block. It could look something like this:

Screenshot 2024-05-06 at 13 25 26

We can start without the add/remove buttons and add that later once it is properly supported.

Once/if this is implemented, we can revisit the existing indicators in the block toolbar, as they might not be needed anymore or they will need an update.

Why?

Right now, there aren't enough visual indicators of the block attributes connected. This would be one step towards improving that part.

Additionally, I believe the management of bindings should probably live under these inspector controls. In the future, when it is possible to create/remove bindings through the UI, we could potentially reuse or improve this panel. This would be the very first iteration.

How?

Ideally, we should try to reuse existing components like the ItemGroup.

@artemiomorales
Copy link
Contributor

@afercia In reference to your comment on improving Block Bindings accessibility, I wanted to bring your attention to this proposed design.

I was speaking with @jasmussen, and an idea was to remove the indicator in the block toolbar for now and instead just focus on having a Bindings panel in the Block Inspector as shown here.

I started working on a PR to get the ball rolling and have something to look at. Do you think something like this could be sufficient as an indicator for the Block Bindings, or have any suggestions?

I'm thinking we could potentially make these items tabbable, with screen readers announcing that these different bindings exist, even before including the buttons to add/remove the bindings.

@afercia
Copy link
Contributor

afercia commented May 15, 2024

@artemiomorales thanks for the ping.
I'm not sure the inspector would be a good option as the only place where a binding indicator is shown. The inspector can be closed and in that case there wouldn't be any indication of bindings.

As per the design of the new inspector panel:

binding indicator inspector

I'm not sure that would be ideal.
The UI doesn't really explain what a binding is and what that text is about.
As a user, I may not know what an src is, or what an alt is. That is too technical. Also, I may not know what a 'post meta' or a 'custom source' is.

To provide some more meaningful explanation, something that is understandable for all users, we'd need more text. More text needs more space. The inspector doesn't provide much space.

Also, to my understanding sooner or later the add/remove buttons should be added to start building the missing UI to manage bindings. That mkes me think even more the inspector isn't the right place for such a complex UI.

I would consider to use a modal dialog, which would provide way more space for a dedicated UI for the bindings.

I'd consider to add some sort of description in the inspecrtor block 'card' at the top (the most prominent part of the inspector) to inform users: 'hey, this block has some bound values'. Also, I'd consider to add a 'Manage bidnings' button that opens the modal dialog.

Just my personal opinion. I'm not a designer but I understand the Bindings API is a feature that is a bit technical to be well explained to users. As such, I think it would need its own dedicated UI to provide clear descriptions and instructions. To me, the block inspector feels just too narrow for such an UI.

@artemiomorales
Copy link
Contributor

As a user, I may not know what an src is, or what an alt is. That is too technical. Also, I may not know what a 'post meta' or a 'custom source' is.

One point to potentially help clarify this: @cbravobernal mentioned that having a header for the bindings panel to specify attributes and sources could be helpful.

block-bindings-panel-header

@cbravobernal is this what you had in mind? Pinging @jasmussen for thoughts. I don't think it's high priority for WordPress 6.6 unless we receive feedback otherwise, and we can always iterate, but thought I would mention here in any case.

@jasmussen
Copy link
Contributor

The ItemGroup isn't meant to be a table in that sense, so I don't think it makes semantic sense to have a table heading. However I do think there's an opportunity to clarify each item. On the other hand, each item is meant to open a popover (see colors, etc.) that is a new canvas to show as much text, help text, context, controls, as is necessary. To that end it seems good to re-evaluate what gets shown in the ItemGroup itself, vs. what can live in the popover instead. One path could be, "Attribute: src" or "src attribute", leaving source information to live inside the popover.

@jasmussen
Copy link
Contributor

Another option is to rename the panel to be called "Attributes", instead of "Bindings".

@cbravobernal
Copy link
Contributor

Another option is to rename the panel to be called "Attributes", instead of "Bindings".

Attributes and bindings are different things. While an attribute is an HTML attribute, a binding is a connection between an attribute and a source. So IMHO renaming the panel is not a good option.

The ItemGroup isn't meant to be a table in that sense, so I don't think it makes semantic sense to have a table heading.

That's a problem. I still think that is necessary to differentiate the attribute vs the source on that list, and a position is not the best way to do it. But right now, I cannot find a better way to do it to be honest 😭

I agree that we can iterate on it later, after receiving some feedback.

@youknowriad
Copy link
Contributor

I'm with @jasmussen here bindings is too technical, attributes is slightly better, it corresponds to the source of the block attributes (not the HTML attributes) and it's a more user friendly name. If you're not a developer, there's 0 change for you to understand "bindings".

@richtabor
Copy link
Member

+1 on "Attributes" rather than "Bindings". The panel references the attributes that are bound anyhow.

@SantosGuillamot
Copy link
Contributor Author

What about adding a short description, even if we change the title? Could that help? Is that something done in other parts of the UI?

Screenshot 2024-06-07 at 15 51 50

@jasmussen
Copy link
Contributor

Sure, can add text before or after the panel, if we have some good ideas for what to add.

@SantosGuillamot
Copy link
Contributor Author

Something like "Block attributes connected to dynamic data"? I didn't think too much about it, to be honest.

@afercia
Copy link
Contributor

afercia commented Jun 10, 2024

+1 on "Attributes" rather than "Bindings".

How about 'Connected attributes'?
A description after the list could further expand what connected attributes are.

Taking inspiration from the original PR that added the Bindings API where the terminology is very clear. It refers to 'attributes connected to different sources' whihc seems to me the most simple and understandable wording for this feature.

@cbravobernal
Copy link
Contributor

We end up using "Attributes" for the title and "Attributes connected to various sources." for the description.

@SantosGuillamot
Copy link
Contributor Author

A first iteration of the bindings panel was already added without using "Bindings" and with a help text to hopefully clarify the connections. Should we close this issue and follow up on potential improvements like adding the remove/add bindings in new issues/pull requests?

Screenshot 2024-06-27 at 13 27 39

@artemiomorales
Copy link
Contributor

A first iteration of the bindings panel was already added without using "Bindings" and with a help text to hopefully clarify the connections. Should we close this issue and follow up on potential improvements like adding the remove/add bindings in new issues/pull requests?

@SantosGuillamot I think closing this and opening a new issue with the updated design makes sense, that way folks following along don't need to read the whole history to keep up with the feature and we can keep moving things forward.

@cbravobernal
Copy link
Contributor

Let's close this one then!
Thanks @artemiomorales for the feedback, and @SantosGuillamot for handling the issue.

@artemiomorales
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block bindings Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

7 participants