-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Font Size Picker: Remove Custom option from FontSizePickerSelect dropdown #69038
Conversation
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 -
This is a screencast of the current changes - Before - Screen.Recording.2025-02-05.at.10.45.04.movAfter - Screen.Recording.2025-02-05.at.10.32.17.movAfter discussing and refining the approach, I will take care of the failing test cases. |
@himanshupathak95 thanks for the PR. |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Thanks for the suggestions @afercia 🙌🏻 I have removed the unit tests that accounted for the "Custom" option, and I have also updated the PR description. Please take a look whenever time permits and let me know if it tests well for you 🙇🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @himanshupathak95
Looks good to me. The only thing left is adding a changelog entry in packages/components/CHANGELOG.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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:
Testing Instructions
Testing Instructions for Keyboard
Screencast
Before -
Screen.Recording.2025-02-05.at.10.45.04.mov
After -
Screen.Recording.2025-02-05.at.10.32.17.mov