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

Font Size Picker: Remove Custom option from FontSizePickerSelect dropdown #69038

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

himanshupathak95
Copy link
Contributor

@himanshupathak95 himanshupathak95 commented Feb 5, 2025

What?

Closes #69029

Removes the 'Custom' option from the FontSizePicker's select dropdown while maintaining custom font size functionality through the settings icon button.

Why?

The current implementation includes a 'Custom' option in the font size dropdown when there are more than 5 font sizes. This creates two issues:

  • Semantically incorrect: A select/listbox control should contain values, not interactive controls that change the UI
  • Accessibility violation: When using keyboard navigation, reaching the 'Custom' option unexpectedly changes the entire UI context and loses focus
  • Redundant functionality: The same custom size feature is already available through the settings icon button

Testing Instructions

  • Use a theme with more than 5 font sizes (e.g., modify Twenty Twenty-Four's theme.json to add extra font sizes)
  • Open the block editor
  • Select a Paragraph block
  • Click on the Font size picker in the inspector panel and verify the "Custom" option is removed

Testing Instructions for Keyboard

  • Use a theme with more than 5 font sizes (e.g., modify Twenty Twenty-Four's theme.json to add extra font sizes)
  • Open the block editor
  • Use Tab to focus on the font size and select dropdown
  • Use arrow keys to navigate through options
  • Verify that no unexpected UI changes occur

Screencast

Before -

Screen.Recording.2025-02-05.at.10.45.04.mov

After -

Screen.Recording.2025-02-05.at.10.32.17.mov

@himanshupathak95
Copy link
Contributor Author

himanshupathak95 commented Feb 5, 2025

This PR currently just removes the "Custom" option from the dropdown for the reasons mentioned by @afercia in #69029 (comment)

This is a WIP and the following things can be implemented after discussion -

  • Opening of the dropdown and persistence while navigating options through keystrokes
  • Autofocus on the custom sizes control

This is a screencast of the current changes -

Before -

Screen.Recording.2025-02-05.at.10.45.04.mov

After -

Screen.Recording.2025-02-05.at.10.32.17.mov

After discussing and refining the approach, I will take care of the failing test cases.

@afercia
Copy link
Contributor

afercia commented Feb 5, 2025

@himanshupathak95 thanks for the PR.
I'd suggest to keep the changes into the scope of the issue and not proceed with the additional points.
When you have a chance, please add the What, Why, How, Testing Instructions to the PR description so it can be reviewed 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Font size picker: unexpected change of context when selecting Custom option from select via the keyboard
2 participants