Polish focus styles #3401
Polish focus styles #3401
Conversation
jasmussen
added
[Focus] Accessibility (a11y)
|
Awesome work. It's working pretty well globally. Two questions:
|
editor/block-switcher/index.js
Outdated
| @@ -110,9 +110,7 @@ function BlockSwitcher( { blocks, onTransform } ) { | |||
| } } | |||
| className="editor-block-switcher__menu-item" | |||
| icon={ ( | |||
| <span className="editor-block-switcher__block-icon"> | |||
There was a problem hiding this comment.
The block icon is not always a dashicon, this wrapper was necessary to correctly add the spacing between the icon and text when we have custom elements as icons. Not saying we shouldn't remove it but it was there on purpose and we should make sure it doesn't break. #3238
I think you're targetting svg but block icons are not always svg, they can be anything.
I did, thanks!
I think we do, as there are apparently some very specific requirements around contrast between shapes and backgrounds, and the background itself doesn't count as a shape. But since we are moving this all to mixins, adjusting these in the future if we find that we can, should be easier to do in one place instead of 20 different scss files. I restored the span and fixed things with a different approach. Good catch, Riad, thank you! |
jasmussen
force-pushed
the
polish/focus-styles
branch
from
e6c5a6c
to
21a18d9
Compare
|
Rebased. I think this is ready for a final pair of eyes. CC: @mtias @karmatosed |
This is a work in progress. Pushing so I can continue work from the coworking space.
- Improve toggles, search fields - Cleaner separation of code for borderradii
Also improve radius consistency further.
- Styles the in-between inserter focus style - Fix some issues with the block-docked toolbar stacking left-borders - Restore the icon span to the transformer, and re-do style neutralization.
jasmussen
force-pushed
the
polish/focus-styles
branch
from
21a18d9
to
a3b92c9
Compare
|
Got a thumbs up in chat. Merging. |
This is a rather mammoth PR to improve focus styles across the board. Outside of the switch styles, which need more work and should be addressed separtely, this fixes #3382 and fixes #3039.
The impetus for this design is that we use a lot of "naked" components. Movers, ellipsis menus, icon-only buttons, which when given the traditional focus style appear immaterial. By giving a considered style and background, it gives solidity to the button.
Because it's a mammoth PR that touches everything at once, I've been able to reduce and trim a ton of styles, move many of the focus styles into mixins for reusability, and I've overall been able to do a ton of consistency work using SCSS variables. Border radii, menu item paddings, focus styles and alignments should all improve as of this.
On the downside, we should test this PR pretty thoroughly.
Edit: Forgot to mention — this PR also both unifies and changes the hover style. It's now very dark gray everywhere, except for formatting controls where there's still that 1px border.
I like this, I think it works, but there's not a lot of visual contrast between neutral and hover states now. I don't think it's a problem, but I wanted to surface it in case others do.
Screenshots: