Unify focus styles, and pave the way for them to be customizable #21141

jasmussen · 2020-03-25T10:34:23Z

This PR does a number of things, the most important one perhaps just being a ton of code cleanup and simplification. It:

Unifies all focus styles to a single style — a thicker blue border.
It creates a new SCSS variable that defines the stroke thickness of this focus border.
It retires nearly all the focus mixins we used to use, in favor of just these unified styles.
It removes a ton of dead code, and reduces specificity selectors where they aren't needed anymore.
It changes the default focus stroke-width to 1.5px in width. This works as a shape change in addition to the color change, but it's balanced against the simplified UI.
~~Additionally, it adds a new focus highlight CSS variable, --border-width-focus, which opens up the door to users customizing these.~~ — Out for now, can be revisited.

A couple of GIFs:

Here, a user customized the CSS variable in the inspector:

jasmussen · 2020-03-25T10:35:49Z

packages/base-styles/_mixins.scss

-@mixin block-style__is-active() {
-	color: $white;
-	background: $dark-gray-primary;
+@mixin input-style__focus() {


These mixins arguably should also be removed. However they are fairly widely used, and in places that are affected by upstream CSS bleed. That's why I omitted these for now. They can be revisited separately.

jasmussen · 2020-03-25T10:36:31Z

packages/base-styles/_variables.scss

+$border-width-focus: 1.5px;
+$border-width-tab: 4px;
+
+:root {


Is there a better place than this SCSS file to output these? I imagine there is, because right now the variable appears duplicated a bunch of times, and it shouldn't be. I would appreciate advice here.

What's the reasoning for using a CSS variable here. It seems inconsistent with the other variables (grids, colors...)

They're both variables, that's all the reasoning there is. And that's why I was asking for advice on what might be a better place. It also gets duplicated when here, so it really isn't the best place — just not sure where to put it.

I think the best way to do it would be something like

$border-width-focus: var( --border-width-focus, 1.5px );

and just use $border-width-focus everywhere without this root rule.

That said, I'd prefer if we avoid introducing these variables in this PR in order to give it more thoughts and also be consistent with the variables we propose.

The reason for doing it the way it's been done currently, is to provide a fallback for IE11.

It can potentially be automated using a Post CSS plugin. Maybe by replacing var above with a custom one config or something

Good argument for leaving it to a separate PR.

github-actions · 2020-03-25T10:39:13Z

Size Change: +13.2 kB (1%)

Total Size: 879 kB

Filename	Size	Change
`build/block-editor/style-rtl.css`	11.2 kB	+171 B (1%)
`build/block-editor/style.css`	11.2 kB	+175 B (1%)
`build/components/style-rtl.css`	16.1 kB	+296 B (1%)
`build/components/style.css`	16 kB	+297 B (1%)
`build/edit-post/style-rtl.css`	12 kB	+3.67 kB (30%)	🚨
`build/edit-post/style.css`	12 kB	+3.67 kB (30%)	🚨
`build/edit-site/style-rtl.css`	4.61 kB	+1.19 kB (25%)	🚨
`build/edit-site/style.css`	4.61 kB	+1.19 kB (25%)	🚨
`build/edit-widgets/style-rtl.css`	3.74 kB	+1.17 kB (31%)	🚨
`build/edit-widgets/style.css`	3.74 kB	+1.17 kB (31%)	🚨
`build/editor/style-rtl.css`	3.47 kB	+84 B (2%)
`build/editor/style.css`	3.47 kB	+81 B (2%)

ℹ️

View Unchanged

Filename	Size	Change
`build/a11y/index.js`	998 B	0 B
`build/annotations/index.js`	3.45 kB	0 B
`build/api-fetch/index.js`	3.39 kB	0 B
`build/autop/index.js`	2.58 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	6.02 kB	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	760 B	0 B
`build/block-editor/index.js`	102 kB	0 B
`build/block-library/editor-rtl.css`	7.21 kB	0 B
`build/block-library/editor.css`	7.21 kB	0 B
`build/block-library/index.js`	111 kB	0 B
`build/block-library/style-rtl.css`	7.5 kB	0 B
`build/block-library/style.css`	7.51 kB	0 B
`build/block-library/theme-rtl.css`	669 B	0 B
`build/block-library/theme.css`	671 B	0 B
`build/block-serialization-default-parser/index.js`	1.65 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/blocks/index.js`	57.5 kB	0 B
`build/components/index.js`	191 kB	0 B
`build/compose/index.js`	6.21 kB	0 B
`build/core-data/index.js`	10.7 kB	0 B
`build/data-controls/index.js`	1.04 kB	0 B
`build/data/index.js`	8.26 kB	0 B
`build/date/index.js`	5.36 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.05 kB	0 B
`build/edit-navigation/index.js`	2.4 kB	0 B
`build/edit-navigation/style-rtl.css`	95 B	0 B
`build/edit-navigation/style.css`	95 B	0 B
`build/edit-post/index.js`	92.3 kB	0 B
`build/edit-site/index.js`	9.04 kB	0 B
`build/edit-widgets/index.js`	4.43 kB	0 B
`build/editor/editor-styles-rtl.css`	423 B	0 B
`build/editor/editor-styles.css`	426 B	0 B
`build/editor/index.js`	42.8 kB	0 B
`build/element/index.js`	4.44 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	6.95 kB	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	1.93 kB	0 B
`build/html-entities/index.js`	621 B	0 B
`build/i18n/index.js`	3.57 kB	0 B
`build/is-shallow-equal/index.js`	710 B	0 B
`build/keyboard-shortcuts/index.js`	2.3 kB	0 B
`build/keycodes/index.js`	1.7 kB	0 B
`build/list-reusable-blocks/index.js`	2.99 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/media-utils/index.js`	4.84 kB	0 B
`build/notices/index.js`	1.57 kB	0 B
`build/nux/index.js`	3.01 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/plugins/index.js`	2.54 kB	0 B
`build/primitives/index.js`	1.5 kB	0 B
`build/priority-queue/index.js`	780 B	0 B
`build/redux-routine/index.js`	2.83 kB	0 B
`build/rich-text/index.js`	14.5 kB	0 B
`build/server-side-render/index.js`	2.55 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/url/index.js`	4.01 kB	0 B
`build/viewport/index.js`	1.6 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

jasmussen · 2020-03-27T09:46:50Z

Okay, rebased and removed the CSS variable for now.

I still think we should create a CSS variable for the focus style, so that it can be easily customized for example in options. But that is perhaps something for a different time.

mapk · 2020-03-27T15:27:33Z

I tried this out today. In Chrome the focus states look wonderful. I did notice that the checkboxes and color circles still didn't reflect this new change.

and

But that can always be another PR.

In Firefox however, the focus states seemed a bit awkward. They were even around the element. Some items showed a stronger bottom-right shadow, while others showed a thicker top border.

jasmussen · 2020-03-30T06:52:10Z

Thank you Mark. I'm having a hard time reproducing the bottom right shadow issue in Firefox. The GIF you're showing has a thicker bottom border to indicate the active tab, this just happens to be the same color as the focus style. But I'm open to suggestions for how to tweak the details here — it feels important to keep the bottom border thickness even while showing focus.

I did notice a little animation flicker that I'll see if I can address!

jasmussen · 2020-03-30T06:58:33Z

Addressed the jumpiness.

Forgot to mention that yes, good call on the color swatches and checkboxes. There are some elements inside the canvas as well, like when a social link button (not the block) has focus.

But as you allude to, I'd like to revisit those separately as this PR is becoming a little big.

mkaz

👍 Tested it out and it looks good tested on Edge and FF on Win10.
Similar color swatch comments and social links as you mentioned but other focuc styles look fine

jasmussen requested review from ajitbohra, chrisvanpatten, ellatrix, jorgefilipecosta and youknowriad as code owners Mar 25, 2020

jasmussen self-assigned this Mar 25, 2020

jasmussen requested a review from mtias Mar 25, 2020

jasmussen added [Type] Code Quality [Type] Enhancement Accessibility (a11y) labels Mar 25, 2020

jasmussen reviewed Mar 25, 2020

View changes

jasmussen force-pushed the try/tweak-focus-stroke branch from 1a22569 to 9948524 Compare Mar 27, 2020

jasmussen force-pushed the try/tweak-focus-stroke branch from 6d5ea59 to a90ae14 Compare Mar 30, 2020

jasmussen force-pushed the try/tweak-focus-stroke branch from 32c45c9 to 5048298 Compare Mar 30, 2020

jasmussen added 10 commits Mar 31, 2020

Add focus thickness variable.

df95d0e

Refine placeholder focus.

7e32c30

Retire a number of unused mixins.

c8e9246

Fix focus style for focused toolbar toggle.

1fc4869

Retire switch mixin, unify focus styles.

c3ecb8c

Unify input focus styles.

303ac2e

Retire dashed line focus style mixin.

86af210

Remove menu item focus mixin.

8392135

Remove the block style mixins.

443418f

Enable a CSS variable to customize the focus width.

71b2305

jasmussen added 3 commits Mar 31, 2020

Unify focus of custom select box.

d87507a

Retire CSS variable for now.

4948922

Address jumpiness.

4205f73

jasmussen force-pushed the try/tweak-focus-stroke branch from 5048298 to 4205f73 Compare Mar 31, 2020

mkaz approved these changes Mar 31, 2020

View changes

jasmussen merged commit 0e3e844 into master Apr 1, 2020
3 checks passed

jasmussen deleted the try/tweak-focus-stroke branch Apr 1, 2020

github-actions bot added this to the Gutenberg 7.9 milestone Apr 1, 2020

mapk mentioned this pull request Apr 6, 2020

Input Fields: Replace Placeholders with Labels #13900

Open

WordPress / gutenberg Public

Unify focus styles, and pave the way for them to be customizable #21141

Unify focus styles, and pave the way for them to be customizable #21141

jasmussen commented Mar 25, 2020 •

edited

jasmussen Mar 25, 2020

jasmussen Mar 25, 2020

youknowriad Mar 25, 2020 •

edited

jasmussen Mar 25, 2020

youknowriad Mar 25, 2020

jasmussen Mar 25, 2020

youknowriad Mar 25, 2020

jasmussen Mar 25, 2020

github-actions bot commented Mar 25, 2020 •

edited

jasmussen commented Mar 27, 2020

mapk commented Mar 27, 2020

jasmussen commented Mar 30, 2020

jasmussen commented Mar 30, 2020

mkaz left a comment

WordPress / gutenberg Public

Unify focus styles, and pave the way for them to be customizable #21141

Unify focus styles, and pave the way for them to be customizable #21141

Conversation

jasmussen commented Mar 25, 2020 • edited

jasmussen Mar 25, 2020

Choose a reason for hiding this comment

jasmussen Mar 25, 2020

Choose a reason for hiding this comment

youknowriad Mar 25, 2020 • edited

Choose a reason for hiding this comment

jasmussen Mar 25, 2020

Choose a reason for hiding this comment

youknowriad Mar 25, 2020

Choose a reason for hiding this comment

jasmussen Mar 25, 2020

Choose a reason for hiding this comment

youknowriad Mar 25, 2020

Choose a reason for hiding this comment

jasmussen Mar 25, 2020

Choose a reason for hiding this comment

github-actions bot commented Mar 25, 2020 • edited

jasmussen commented Mar 27, 2020

mapk commented Mar 27, 2020

jasmussen commented Mar 30, 2020

jasmussen commented Mar 30, 2020

mkaz left a comment

jasmussen commented Mar 25, 2020 •

edited

youknowriad Mar 25, 2020 •

edited

github-actions bot commented Mar 25, 2020 •

edited