Gutenboarding: Improve a11y and lints

[sirreal]

Changes proposed in this Pull Request

Add and use Reakit to implement the page selector as a Dialog. This should improve overall usability of the page selector.

This fixes some lint issues as well as simplifies some styling, for example the full-width page selector overlay. Click outside and escape close the page selector.

Screens

Before

After

Follow up

• Use Reakit Rover for tab access to designs and templates in the page selector.
  used Card as="button" for now.
• Move to css-in-JS which should help clarity and consistency across css/js. There are a lot of duplicated variables and logic right now.
• Transitions are likely to move to React Spring

Testing instructions

• Visit /gutenboarding.
• Complete the first step.
• Navigating between designs should work using keyboard (tabbing).
• On focus / hover over a design there should be a transparent overlay with text.
• Pick a design and verify click outside and escape work as expected.
• In page layout selector focus should be by default on the first item and tabbing should work.
• Focus should return to the design that was selected.
• Designs are improved (paObgF-RB-p2)

[matticbot]

Test live: https://calypso.live/gutenboarding?branch=gutenboarding/improve-a11y-and-lints

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~5546 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-gutenboarding     +20883 B  (+1.2%)    +5554 B  (+1.2%)
entry-login                -30 B  (-0.0%)       -8 B  (-0.0%)
entry-main                 -27 B  (-0.0%)       +0 B

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~316087 bytes added 📈 [gzipped])

name                   parsed_size             gzip_size
settings-security        +226594 B  (+104.5%)   +52720 B   (+96.1%)
settings-performance     +226594 B  (+109.4%)   +52720 B   (+96.8%)
settings-discussion      +226594 B  (+133.3%)   +52720 B  (+123.4%)
settings-writing         +226586 B   (+50.5%)   +52715 B   (+47.7%)
settings                 +226586 B   (+44.6%)   +52717 B   (+41.2%)
marketing                +226586 B   (+53.3%)   +52713 B   (+51.0%)
woocommerce                 -171 B    (-0.0%)      -76 B    (-0.0%)
domains                      -64 B    (-0.0%)      -19 B    (-0.0%)
email                        -56 B    (-0.0%)      -20 B    (-0.0%)
checkout                     -32 B    (-0.0%)      -16 B    (-0.0%)
purchases                    -28 B    (-0.0%)      -19 B    (-0.0%)
wp-super-cache               -24 B    (-0.0%)       -8 B    (-0.0%)
hosting                      -24 B    (-0.0%)      -11 B    (-0.0%)
site-blocks                  -16 B    (-0.0%)       -5 B    (-0.0%)
security                     -16 B    (-0.0%)       -3 B    (-0.0%)
privacy                      -16 B    (-0.0%)       -5 B    (-0.0%)
plans                        -16 B    (-0.0%)       -6 B    (-0.0%)
notification-settings        -16 B    (-0.0%)       -5 B    (-0.0%)
me                           -16 B    (-0.0%)       -5 B    (-0.0%)
help                         -16 B    (-0.0%)       -5 B    (-0.0%)
checklist                    -16 B    (-0.0%)       -5 B    (-0.0%)
account-close                -16 B    (-0.0%)       -5 B    (-0.0%)
account                      -16 B    (-0.0%)       -5 B    (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~46 bytes removed 📉 [gzipped])

name                                                         parsed_size           gzip_size
async-load-my-sites-current-site-domain-warnings                   -16 B  (-0.0%)       -4 B  (-0.0%)
async-load-my-sites-checklist-wpcom-checklist-component-jsx        -16 B  (-0.0%)       -6 B  (-0.0%)
async-load-design-blocks                                           -16 B  (-0.0%)      -20 B  (-0.0%)
async-load-design                                                  -16 B  (-0.0%)       -8 B  (-0.0%)
async-load-reader-sidebar                                          -12 B  (-0.0%)       -8 B  (-0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[sirreal]

Sections (~360785 bytes added 📈 [gzipped])

😮 Another wild ICFY report❗️ Why would this affect all those sections so badly?

[diegohaz]

There's no value in adding those aria attributes if the component is not accessible.

Use Reakit Rover for tab access to designs and templates in the page selector.

I'd not use Rover here. Rover will apply the roving tabindex method to the cards, which means that, by default, users won't be able to Tab through the cards. And since they have no clue that they should use arrow keys (as they would with widgets like toolbar, menu etc.), keyboard users may still not be able to navigate through this page.

That could be solved by overwriting the tabIndex prop rendered by Rover, which would allow users to Tab and use arrow keys at the same time! 🙌

But that's not the only problem! 😔

Rover only adds roving tabindex in one dimension. Sighted users using the keyboard could expect that they could navigate the card list as if it were a grid (bidimensional), since that's what they see. There's an ongoing conversation around adding a Grid module to Reakit: ariakit/ariakit#435

Instead of overcomplicating this, I'd just make this component tabbable.

Tabbable, which is used by Rover underneath, may help with this, specially because this component is a div. Tabbable would also make it clickable with Enter and Space.

A simpler (and better?) solution though is to just turn this element into a button. <Card as="button" should do the trick I guess.

[simison]

I'm a little surprised by Calypso color variable here instead of Gutenberg variable. Not a blocker for merging, just something we'll likely need to audit at some point.

[razvanpapadopol]

It's the color used in design and also in the current page template selector. The closest Gutenberg variable would be $dark-gray-600: #40464d; but it has less blue and more grey.

I agree we need to define together with design team which parts should be using Gutenberg styles and where should come in Calypso theming.

Also we should take into consideration that mixing color variables will increase the complexity of dark theme support ( #38587 )

[simison]

Lookin' good!

Some notes;

• Couldn't get design picker "selected style" (or the text you mention) visible by mouse nor by keyboard by tabbing or with arrow keys. Tabbing does work, just I don't see visually any indication. Tested on Chrome.
• At page selector, could selected page that I tab into turn darker in color, so that tabbing over selected pages wouldn't seem "invisible"? I'd expect same style when I hover already selected page.
  
• Scroll position followed nicely tabbed selection in both design picker and page selector
• Once I tabbed to the last page in the grid, I could see designs peaking:
  
• Esc cancels all page selections and returns to design picker, just like clicking outside page selector does as well. I expected my page selections to remain tho; they got wiped out.
• I can tab-rotate in pages, but I cannot escape them? Should we tab to "back" button, "domain picker" and other elements after the last page in the grid? Good for follow up PR.

[simison]

ci/wp-desktop failure seems unrelated — rebasing might help.

[dwolfe]

We can borrow from the existing starter page template selector styles - the suggestion below works for page selector cards. Still need to figure out how to work around a specificity collision with .is-elevated to use the same style on the design selector cards.

	&:focus {
		box-shadow: 0 0 0 1px $white,
					0 0 0 3px $blue-medium-500;
		// Windows High Contrast mode will show this outline, but not the box-shadow.
		outline: 2px solid transparent;
	}

	&:hover {
		border: 3px solid var( --color-neutral-dark );
		padding: 0;
	}

[razvanpapadopol]

Thanks for the suggestion. Opened this issue to try this out: #38665

[razvanpapadopol]

Couldn't get design picker "selected style"

Fixed in 15b1510

tabbing over selected pages wouldn't seem "invisible"? I'd expect same style when I hover already selected page.

Fixed in 9234d31

I could see designs peaking

Fixed in 50a7ff9

I can tab-rotate in pages, but I cannot escape them? Should we tab to "back" button, "domain picker" and other elements after the last page in the grid? Good for follow up PR.

When you are selecting pages (layouts) you're basically in a modal so the focus is captured to that context. So I'd say pressing ESC and then tabbing to another element is somehow expected. But given the fact we don't display an actual overlay on the rest of the page so all the action buttons are visible and clickable, we should discuss this further. Created #38664

Esc cancels all page selections and returns to design picker, just like clicking outside page selector does as well. I expected my page selections to remain tho; they got wiped out.

To be implemented