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

Content/Wide controls need 40px-compatible design #64842

Closed
mirka opened this issue Aug 27, 2024 · 10 comments · Fixed by #64891
Closed

Content/Wide controls need 40px-compatible design #64842

mirka opened this issue Aug 27, 2024 · 10 comments · Fixed by #64891
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Design Needs design efforts. [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@mirka
Copy link
Member

mirka commented Aug 27, 2024

What problem does this address?

The Content/Wide inputs used in Layout and Dimensions controls have a design that cannot be converted to the new 40px control sizes in a straightforward way.

Content/Wide inputs in the older 36px size

Even with tweaks to decrease the footprint of all the relevant elements (#64708, #64712), we still have some overflow even in one of the more optimistic cases ("px" unit in Chrome, which supports CSS field-sizing), if there is a visible scrollbar in the sidebar:

Content/Wide inputs in the newer 40px size

What is your proposed solution?

Some of the suggested solutions were:

  • Put the icon in the label suffix
  • Remove the icons
  • Break the controls into two rows

All which were not ideal for one reason or another. This is one of the last remaining controls with the older 36px size, so we need a new design as soon as possible.

@mirka mirka added [Type] Enhancement A suggestion for improvement. Needs Design Needs design efforts. [Package] Block editor /packages/block-editor labels Aug 27, 2024
@WordPress WordPress deleted a comment Aug 27, 2024
@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Aug 28, 2024
@afercia
Copy link
Contributor

afercia commented Aug 28, 2024

A few more points from the duplicate issue #64862

  • Those icons are confusing and break an established contention. They look clickable but they aren't. They should be removed as unnecessary and only ornamental.
  • The two ToolsPanelItems in this UI do have a label prop set that, as far as I can tell, doesn't do anything. I will create a separate issue for that.
  • Removing the icons would help with making these input use the __next40pxDefaultSize.

@afercia afercia added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Aug 28, 2024
@afercia afercia self-assigned this Aug 28, 2024
@MaggieCabrera
Copy link
Contributor

* Those icons are confusing and break an established contention. They look clickable but they aren't. They should be removed as unnecessary and only ornamental.

I wouldn't say they are ornamental or confusing. The label "Content" on its own is not enough to convey what it's referring to. I think the combination of icon/label here helps a lot with clarity. In particular, seeing one icon next to the other gives you the information as to which of the two inputs should hold the higher value.

@afercia
Copy link
Contributor

afercia commented Aug 28, 2024

Labeling and descriptions could be improved as well. I do agree the label 'Content' is not ideal. As a used, I'm confused by teh description as ell: Same applies to the label 'Wide'.
Actually, these are the Content size and Wide size. The UI should explain it better.

Set the width of the main content area.

Makes me think I can set ONE width. Instead, they may conditionally be two.

I disagree about the icons. As a used, those icons don't tell anything useful to me. They look clickable because they editor uses this pattern for clickable buttons. They aren't clickable though and that breaks an established design pattern.

@afercia
Copy link
Contributor

afercia commented Aug 28, 2024

When editing blocks, these widths are named differently and are clearer. Basically:

  • I find it arguabel that they are placed in a menu called 'Align'. These are widths, not alignments. It's also confusing because in the Site editory Styles these settings are under Layout > DImensions, there's no mention of the term 'align'.
  • The 'content' width is labeled 'None' (see screenshot below). For consistency and clarity it should be 'Content width'.
  • The 'Wide width' is cleaer enough (see screenshot below). It should be labeled 'Wide width' in the Site editor as ell.

Screenshot 2024-08-28 at 12 33 14

Worth also reminding that themes may support only one of the widths. In that case, the editor UI will show only one control, where the icon is even more unnecessary and confusing. Screenshot:

Screenshot 2024-08-28 at 12 22 57

I'd totally agree with this comment from @jameskoster :

#64520 (comment)

One drawback with this approach is that we now have decorative icons situated in a location where you often find interactive icon buttons. In other words those icons look clickable.

As such, I think the best option for now is to just remove these icons.

@afercia
Copy link
Contributor

afercia commented Aug 28, 2024

Additionally: in the 'DImension options' popover, these settings are named 'Content size' and 'Wide size', which I would say it's incorrect. These are widths of the block. They should use a consisntent name across post and site editor.

Screenshot 2024-08-28 at 13 54 28

@afercia
Copy link
Contributor

afercia commented Aug 28, 2024

It's worth reminding the same issue occurs for the layout settings of some blocks (e.g. the Group block). Screenshot:

Screenshot 2024-08-28 at 15 43 46

@jameskoster
Copy link
Contributor

I agree the labelling is inconsistent and confusing. It might be worth extracting that into it's own issue if you have a moment @afercia.

Somewhat related; the "Inner blocks use content width" toggle is confusing. It assumes some conceptual understanding of "Content width", which is likely to be lacking in most cases.

There is some value in the icon though, imo. It visually connects these settings to the associated toolbar options.


@mirka I don't particularly love either option, but here are a couple of ideas how we might solve the original issue.

Screenshot 2024-08-28 at 17 35 49

The help text refers to these inputs as a single control, perhaps they could be visually presented as such. Would this create the necessary space? Style overrides are a shame.

Alternatively we could split the inputs onto separate rows, but also display the labels inline so they do not consume so much vertical space:

Screenshot 2024-08-28 at 17 36 06

This would be a departure from existing layout conventions, but perhaps there are other controls that would benefit from this orientation...?

@mirka
Copy link
Member Author

mirka commented Aug 28, 2024

I also agree the icons are valuable. To mitigate the issue of it seeming clickable, I'm hoping we can codify this in our design guidelines for input-like controls that the prefix slot should be used for informational cues and never interactive things.

As for the layout suggestions in #64842 (comment), I'm concerned about how these decisions will scale. Unless we can present clear guidelines on when exactly these new patterns should be deployed, it's just going to lead to entropy.

So I'm wondering if we can solve this within the bounds of our already existing patterns. To be honest I'm not seeing how breaking the two controls into two rows (with the standard label placement) is going to make them seem unrelated. If anything, the icon ties them together. If we're really concerned about them being perceived as a group, maybe they should be in their own Panel section? Or is the primary concern here to maintain a small footprint?

@afercia afercia added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Aug 29, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Aug 29, 2024
@jameskoster
Copy link
Contributor

It would be nice to avoid separate rows, but I agree we should not break the constraints of the underlying system to achieve it; unless there are very clear wins in terms of reusability, IE they go on to become established conventions in their own right.

I could potentially see the 'combined' approach being applicable elsewhere, e.g. a phone number control with combined country code + number:

Screenshot 2024-08-29 at 12 33 00

But it's by no means a sure thing.

Two rows is not world-ending imo.

@afercia
Copy link
Contributor

afercia commented Aug 30, 2024

The help text refers to these inputs as a single control, perhaps they could be visually presented as such.

I think the help text should be conditional. Actually depending on theme support for contentSize and wideSize, the UI may render both input fields or only one of them.

As mentioned in #64906 (comment) the concept of 'Content width' has been used for ages in WordPress but that's because of the $content_width variable for theme developers. I'd agree it's not the best terminology to use for users.

Overall, I struggle with this UI because it doesn't explain well what these two inputs are about. I'd love to see the descriptions and label explain it clearly. Something along these lines, in prose:

When both are supported by the theme:

Here you can customize the width used for the Default (None) and Wide block alignments.
The default values are provided by the active theme.

When only one of them is supported:

Here you can customize the width used for the {alignment type} block alignments.
The default value is provided by the active theme.

For the similar inputs in the Group block settings panel a similar wording should be used, making clear that they are meant to override the global values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Design Needs design efforts. [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants