Polish the focus style for the segmented control. #33842
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ntsekouras
reviewed
Aug 3, 2021
There was a problem hiding this comment.
Specifically there appears to be a bug in how the ${ CONFIG.controlBoxShadowFocus }; variable is output. It generates the following incorrect box shadow, which then doesn't render at all:
Great catch Joen! I pushed an update to fix the value and updated the tests where needed.
|
Size Change: -4 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
|
Ah, thanks for doing that, it's good education for me to see where these properties are stored! |
|
Nice, thanks for the help! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Followup to feedback in #31937 (comment), this PR polishes the focus style for the new Segmented Control. Before:
After:
The focus style for buttons, unputs and selected blocks across the block editor is 1.5px blue focusring, which changes the shape from 1px inputs or controls, or sits outset of solid blue primary buttons.
In this case, the usage of focus-within feels correct, as when you tab to the segmented control, pressing tab again sets focus on the next control, not the next option inside; you have to use the arrow keys to change the property inside.
Note: This PR is a draft because it needs a bit more work. Specifically there appears to be a bug in how the
${ CONFIG.controlBoxShadowFocus };variable is output. It generates the following incorrect box shadow, which then doesn't render at all:The two commas are wrong. Here's how it should be:
This PR simply outputs the corrected value directly, but ideally we find a way to output the variable correctly before we land this. If you know how to, please feel free to push directly to this branch.
How has this been tested?
Insert a "Post Featured Image" block and focus the segmented control in the inspector.
Checklist:
*.native.jsfiles for terms that need renaming or removal).