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

Make the X Close buttons target size larger #66277

Closed
2 tasks done
afercia opened this issue Oct 21, 2024 · 5 comments · Fixed by #66756
Closed
2 tasks done

Make the X Close buttons target size larger #66277

afercia opened this issue Oct 21, 2024 · 5 comments · Fixed by #66756
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Editor /packages/editor [Package] Interface /packages/interface [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Oct 21, 2024

Description

See conversation on #63243

Recent changes in #61331 and other PRs standardized the size of various X Close buttons to the small variant, which is 24 by 24 pixels.

While standardizing is good, unnecessarily reducing the target size isn't.

Whenever possible, given the constraints of the layout and containers where the buttons live, the target size should be as large as possible.

Additionally, the 24px size is inconsistent with other buttons that may be displayed close to the X button. A few examples where I used background colors to illustrate the inconsistency:

In the Styles panel header: 3 buttons use the compact size (32 pixels) and the X button uses the small size (24 pixels):

Image

Similarly, in the Plugins panel, one button is compact and the X button is small:

Image

A good point was made on #63243 (comment) that these buttons should be horizontally aligned with other controls in the UI so that the right padding of the containers should be adjusted. In the screenshot below, observe that besides the inconsistent size, the container right padding is inconsistent as well:

Image

Additionally: all the other X close buttons e.g. within modal dialogs should be at least compact (32 by 32 pixels).

It is worth reminding that the WCAG Target Size (Minimum) requires a size of at least 24 by 24 CSS pixels. That's the minimum though. There's no reason not to make these buttons bigger, when possible. The editor should aim to provide the best possible experience and not be limited to meet the minimum requirements.

Step-by-step reproduction instructions

  • Open the Styles panel.
  • Observe the size of the buttons in the panel header is inconsistent and the X close button is the only one that is small.
  • Install and activate a plugin that adds a plugins panel e.g. Yoast SEO.
  • Open the plugin panel and observe the 'Pin / Unpin' button is compact, while the X close button is small.
  • Open the main inserter and observe the X close button is small. Observe there is enough space to make it compact.
  • Same for the List View panel and other instances across the editor.
  • Open a modal dialog e.g. the 'Rename blockone and observe the X close button issmall. Observe there is enough space to make it compact`.
  • Same for other modal dialogs e.g. the Keyboard shortcuts one or the Preferences one.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Editor /packages/editor [Package] Interface /packages/interface [Type] Bug An existing feature does not function as intended Needs Design Feedback Needs general design feedback. labels Oct 21, 2024
@afercia
Copy link
Contributor Author

afercia commented Oct 21, 2024

Quickly looking at a few buttons instances in the codenase, it appears there's some room for some CSS clean-up. For example, the X close button of the tabs panel uses the small variant. That should be sufficient to render the button with the 24 pixels size but there's still some CSS overriding in place, which I assume it's a leftover and should be removed. See below, where the $icon-size variable is 24 pixels:

.components-panel__header.editor-sidebar__panel-tabs {
padding-left: 0;
padding-right: $grid-unit-15;
.components-button.has-icon {
padding: 0;
min-width: $icon-size;
height: $icon-size;

Image

@afercia afercia self-assigned this Oct 21, 2024
@t-hamano
Copy link
Contributor

This close button appears to have been changed to a small size by #61331.

https://github.com/WordPress/gutenberg/pull/61331/files#diff-c2b4b3fd0242f8ea3fab899322d819626b990004cfa592f949967217bed13e71R305

cc @WordPress/gutenberg-design

@jameskoster
Copy link
Contributor

I agree the buttons should be the same size, as should the spacing between them and the horizontal padding of their containers. This consistency would make alignment much simpler to achieve and in most cases the default meaning we can remove some ad hoc styling.

@afercia
Copy link
Contributor Author

afercia commented Nov 5, 2024

Aside: noticed that in the Widgets page, the List View close button is 40 pixels, which is too big and inconsistent. Will fix that too. Screenshot:

Image

@afercia
Copy link
Contributor Author

afercia commented Nov 5, 2024

Noting the modal dialog component X close button was changed in #65102 / #65131. Neither the issue nor the pull requests have any accessibility label of any considerations related to accessibility.

The modal X close button used to be 36 pixels. The new sizing scheme provides 3 size varaints: 40, 32, and 24 pixels. The button was changed to 24 pixels for no other apparent reasons than alignment. The 32 pixels variant would have been better as the modal provides enough space tor a bigger button. Making it 24 pixels makes the target size unnecessarily smaller and only for visual purposes.

I'll split the modal close button to a separate issue to not block this one.

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). Needs Design Feedback Needs general design feedback. [Package] Editor /packages/editor [Package] Interface /packages/interface [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.

3 participants