Wp-calypso: Gutenboarding: page selector is capturing focus

[razvanpapadopol]

In Gutenboarding, in design picker step, once I select a design and page selector dialog opens, I can tab-rotate between page layouts, but I cannot tab to "back" button, "domain picker" and other elements visible on screen. Issue reported here: https://github.com/Automattic/wp-calypso/pull/38583#pullrequestreview-337251522

Ref: https://reakit.io/docs/dialog/#non-modal-dialogs

[yansern]

UPDATE: I misunderstood the context of this issue. Sorry. :)

Here's the currently observed behaviour:

1. Current Focus is First Design Selector Item.
2. Press SHIFT+TAB.
3. Current Focus is Onboarding Block. This is the problem here. There is no visual cue, and in fact this shouldn't be focusable. See explanation below.
4. Press SHIFT+TAB.
5. Current Focus is Domain Picker.
6. Press SHIFT+TAB.
7. Current Focus is Back Button.

The HTML for Onboarding Block is:
<div id="block-b7568444-1fd9-4895-a8a6-a2e15bce630c" class="wp-block editor-block-list__block block-editor-block-list__block is-selected" data-type="automattic/onboarding" tabindex="0" aria-label="Block: Onboarding" style="transform-origin: center center;”>

It has a tabindex=0 which causes it to be the next element in line to be in focus. The reason why there is no visual cue (like a focus outline) is because it is intentional that we're using CSS to hide all of the block toolbars.

In fact, this element shouldn't be focusable at all. Since we're working on #38635 which is to move over from hiding using CSS to using the __experimentalUIParts props from InnerBlocks (See: https://github.com/WordPress/gutenberg/pull/18173), using the property hasFocusedUI: false would most probably get rid of the tabindex=0 attribute.

When this onboarding block becomes unfocusable, the focus will move back to domain picker.

[yansern]

I can tab-rotate between page layouts, but I cannot tab to "back" button, "domain picker" and other elements visible on screen.

Just adding a note for clarification. What this means is that user is not allowed to SHIFT-TAB out from the page selector items into domain picker or back button, as explained in the slack conversation here: p1579861261176200-slack-C0KDTA48Y

See https://github.com/Automattic/wp-calypso/issues/38664#issuecomment-582296979

[Addison-Stavlo]

Interesting. ESC works to close the page layout selector and then the tab navigation for everything else can be used again. I wonder why the focus is locked into the page layout selector until it is closed. 🤔

[yansern]

Interesting. ESC works to close the page layout selector and then the tab navigation for everything else can be used again. I wonder why the focus is locked into the page layout selector until it is closed. 🤔

@Addison-Stavlo The page selector uses reakit modal/dialog which has an option for trapping focus within the modal, when modal is set to true. See: https://reakit.io/docs/dialog/#non-modal-dialogs

[yansern]

Just adding a note for clarification. What this means is that user is not allowed to SHIFT-TAB out from the page selector items into domain picker or back button, as explained in the slack conversation here: p1579861261176200-slack-C0KDTA48Y

I just want to say that it actually makes more sense usability wise if there is no focus trapping - user is allowed to SHIFT-TAB out from the page selector items into domain picker or back button.

[yansern]

This is the conclusion from the research & conversation on p1580972528349800-slack-CNMU1UHHB. PR for the below here: https://github.com/Automattic/wp-calypso/pull/39336

1. We will continue using <Dialog>. This is because it has hideOnEsc and hide event hook readily available for us, so no extra work needed to write that behaviour again.

2. The modal={ false } option is used to disable focus trapping. This is so you can tab in and out of the page selector.

3. The hideOnClickOutside={ false } is used to ensure when elements outside of the page selector (like the buttons on the top bar) is focused, the page selector remains on the page.

4. When modal={ false } is used, the page selector renders as a direct child of design selector instead of through a portal. This causes the page selector UI not to spread across the entire viewport. I have found a pure CSS way to make the page selector UI spreads across the entire viewport even when by default it is constrained by the width of the parent, and that is width: 100vw; margin-left: calc( (100vw - 100%) / -2 );.

5. <Portal> and <DialogBackdrop> is removed. And the 2 alternative ideas proposed, one related to <Popover>, the other one related to rendering the page selector through a <Portal> is no longer necessary now that we have a pure CSS solution to render the page selector across the entire viewport.

6. No focus redirection. While it is nice to be able to tab from the last page selector item into the create site button, it actually ended up preventing user from being able to tab into the report bug button (or in production, I imagine there will be a live chat button). So I think the best solution is not to mess with browser's tab ordering, and let things be the way it is. This also minimizes the amount of code needed to write, including passing references from one component to another.