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

Button: Audit 24px isSmall size #51708

Closed
mirka opened this issue Jun 20, 2023 · 10 comments · Fixed by #51842
Closed

Button: Audit 24px isSmall size #51708

mirka opened this issue Jun 20, 2023 · 10 comments · Fixed by #51842
Assignees
Labels
Needs Design Feedback Needs general design feedback. [Package] Components /packages/components

Comments

@mirka
Copy link
Member

mirka commented Jun 20, 2023

In light of #46734 and #51012 (comment), we need to evaluate which of the current 24px isSmall button usages are still legitimate.

This is not meant as a complete listing, but as an overview of the typical occurrences.

Types of 24px isSmall usage

Auxiliary toggle buttons in heading labels

Description
FontSizeControl Toggle button in FontSizeControl
ToolsPanel dropdown Dropdown button in a ToolsPanel pattern

Link/Unlink buttons in a box-like control

Link/Unlink buttons

Navigator back button

Navigator back button

With text labels

Description
Image block Image size toggles and reset button
Button block Width settings of Button block
Cover block 'Clear media' button in Cover block
Search block Width settings of Search block
@mirka mirka added Needs Design Feedback Needs general design feedback. [Package] Components /packages/components labels Jun 20, 2023
@mirka mirka self-assigned this Jun 20, 2023
@mirka
Copy link
Member Author

mirka commented Jun 20, 2023

@jasmussen I'm assuming that the "With text label" small buttons are all outdated and need to be updated with a newer style, but the other types of usages are all still legitimate and need to be preserved as a Button size variant. Is that correct?

@jasmussen
Copy link
Contributor

Thanks for following up. In the examples you've shared in this grid, the main valid usages are the icon buttons, which especially the link/unlink buttons and ellipsis menus should remain 24x24. I wonder how to best address that. For what it's worth, I don't see us having a 40x40px icon button, so those could potentially be 32x32 (large) and 24x24 (small). I'd appreciate a @jameskoster sanity check on this.

The remaining examples are all instances that have needed updating for quite a while. Clear media, for example, could benefit from being a full tertiary button with the text "Clear" or "Reset", and all those buttongroups could benefit from being segmented controls:

Screenshot 2023-06-21 at 09 09 37

@jameskoster
Copy link
Contributor

For what it's worth, I don't see us having a 40x40px icon button, so those could potentially be 32x32 (large) and 24x24 (small).

Putting our existing UIs aside for a moment and thinking about the components as part of a design system, I'm not sure how practical it is for icon buttons to have different size properties / names to regular buttons (and other inputs for that matter). Wouldn't a default 40px and small 32px for all buttons simplify things considerably?

I appreciate we don't have a specific need for 40px icon buttons currently, but it doesn't seem beyond the realms of possibility as component usage begins to expand outside of the full screen editor. [input] [button] is a fairly common layout:

Screenshot 2023-06-21 at 10 13 36

That leaves a question about what to do with the 24px buttons in the Inspector. I quickly fudged the numbers in dev tools and it doesn't look horrendous:

Trunk (24px) 32px
Screenshot 2023-06-21 at 10 18 02 Screenshot 2023-06-21 at 10 16 18

With a little massaging of our grid values it could be made to work?

@jasmussen
Copy link
Contributor

How would you fit all those "advanced" settings icons on the right?

For what it's worth, the small click area there is in part because they are advanced properties. That's not to say that there isn't a use case for a 40px icon button, just that I find it also valid to have a smaller button.

@jameskoster
Copy link
Contributor

I don't dislike the 24px button in isolation, I'm just a bit anxious about the complexity it adds to the overall system from a consumer perspective.

For what it's worth, the small click area there is in part because they are advanced properties

There are other buttons though, (ellipsis, linking/unlinking borders, choosing which side to apply padding), which can always benefit from a larger target.

To be clear I'm not suggesting the advanced icons would need to use the 40px size – they can use the 32px variant. If you manually change the values in dev tools it doesn't actually affect the overall appearance that much. It's easier to see with an active / focus state which my screengrabs above were missing:

Screenshot 2023-06-21 at 14 12 17

@jasmussen
Copy link
Contributor

I hear you, and I don't entirely disagree. That said, I like having as a tool the option for the smaller buttons as yet another tool in the toolbox for managing both compression and prominence of advanced features.

Is there any chance we can move forward with 40+32 for buttons, and 32+24 for icon for now, and then we can make exploring icon sizes a separate thing?

@ciampo
Copy link
Contributor

ciampo commented Jun 22, 2023

Just to make sure I'm understanding:

  • you are suggesting that Buttons expose a 40px (default size) and 32px (small size) variant
  • when the Button is 40px, icons rendered inside the button should be 32px. And when the Button is 32px, icons rendered inside the button should be 24px

Is that correct?

@jasmussen
Copy link
Contributor

Not precisely. From this sheet:

Inspector controls

I'm suggesting that the Button is 40px by default, but has a 32px option that is the same font size, just not as tall. You can see the 40px button in the modal, and the 32px button in the toolbar.

For icon buttons or toggles, they would default to 32px, but have a 24px option.

It isn't clear to me we will ever need a 40x40px icon button.

I would imagine a Button with an icon inside (I think we have a case of an Upload button that features an icon), I would expect the same heuristic as the button, i.e. 40px or 32px button. Icons should never be smaller than 24x24 with few exceptions, so this is mainly about the footprint of the Button and Icon Button components.

@jameskoster
Copy link
Contributor

I would imagine a Button with an icon inside (I think we have a case of an Upload button that features an icon), I would expect the same heuristic as the button, i.e. 40px or 32px button.

This is exactly the point I was making above. Buttons would be 40px or 32px. Whether any individual button includes a text label, an icon, or a combination thereof is inconsequential. This seems much simpler to me in terms of the design system, from both a consumer, and a maintenance perspective.

@mirka
Copy link
Member Author

mirka commented Jun 22, 2023

Given the urgency of having to retract the __next32pxSmallSize prop for WP 6.3, I'm going to start preparing a PR to introduce a size prop where:

  • small = 24px
  • compact = 32px
  • default = 40px when __next40pxDefaultSize = true, otherwise 36px

Is there any chance we can move forward with 40+32 for buttons, and 32+24 for icon for now

I thought about it, but from the consumer API perspective, it is way too complicated for the component sizing scheme to change automatically based on whether the button is an icon button or not. Especially when we are in the middle of a sizing scheme transition from 36px default to 40px.

The size prop system I'm proposing above (small/compact/default) will push the complexity out of the automatic component logic, and into the consumer's actual understanding of the design system. We will provide guidelines in the documentation of course, but the consumer will be responsible for choosing the correct size for their use case.

This will potentially increase UI inconsistency in the wider ecosystem because not everybody will follow the design system correctly. IMO that is the main downside over a simpler system like @jameskoster is suggesting.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jun 23, 2023
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Package] Components /packages/components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants