Don't use disabled on focusable UI controls e.g. "Reset" buttons

[afercia]

Noticed while reviewing #14919 for the Range Control, see #14919 (comment), and applies to other buttons as well, for example the Font Size picker "Reset" button.

The Reset button in the font size picker:

When using the keyboard and pressing the Reset button, the button gets a disabled HTML attribute. Thus, it's not focusable any longer and a focus loss occurs.

Worth noting modern browsers try to implement a smart behavior by the means of a so called sequential focus navigation starting point. Among other things, this feature is meant to help in this kind of situations: pressing Tab again should make the tab order start (more or less) from the place where focus was.

However, this is not guaranteed to work with assistive technologies. Not to mention some browsers don't have this feature (typically IE 11).

To avoid this kind of focus loss, Gutenberg uses a different pattern in a few places. Instead of using the HTML attribute disabled:

• noop the button
• use aria-disabled to communicate the button inert state
• style the button as it was disabled

This pattern should be extended to all the cases where a UI control that currently has focus gets disabled. Starting from these "Reset" buttons, I'd like to propose to review all the cases where disabled is used across the codebase.

[draganescu]

Hi @afercia, I re-tested this and I noticed that while the button still gets a disabled attribute, the tabbing order is respected, as in pressing tab moves the focus to line height, just as when the reset button is not disabled.

[afercia]

Hi @draganescu. I think you're confusing the so called sequential focus navigation starting point implemented in modern browsers with the native tab order. As mentioned above, those are two different things. The sequential focus navigation starting point isn't implemented in all browsers and there's no guarantee it works as expected when assistive technologies are in use. To get an idea, you should test with IE 11.

[youknowriad]

I don't see any "reset" button on the font size control anymore. So I'm guessing this issue doesn't apply anymore.

[afercia]

The font size picker mentioned on this issue is just an example of the pattern that needs to be fixed. More controls do have a 'Reset' button or other buttons that get a disabled attribute while they're focused. Using the disabled attribute is the pattern that should be checked everywhere.

Specifically, the font size picker now has a withReset property set to false so that it doesn't show the Reset button. However, the component still uses the disabled attribute, thus potentially triggering a focus loss.

Other examples after a quick search in the codebase:

Combobox control:

gutenberg/packages/components/src/combobox-control/index.tsx

Lines 343 to 349 in a538973

	<Button
	className="components-combobox-control__reset"
	icon={ closeSmall }
	disabled={ ! value }
	onClick={ handleOnReset }
	label={ __( 'Reset' ) }
	/>

Range control:

gutenberg/packages/components/src/range-control/index.tsx

Lines 315 to 323 in a538973

	<Button
	className="components-range-control__reset"
	disabled={ disabled || value === undefined }
	variant="secondary"
	isSmall
	onClick={ handleOnReset }
	>
	{ __( 'Reset' ) }
	</Button>

Box control:

gutenberg/packages/components/src/box-control/index.tsx

Lines 167 to 175 in a538973

	<Button
	className="component-box-control__reset-button"
	variant="secondary"
	isSmall
	onClick={ handleOnReset }
	disabled={ ! isDirty }
	>
	{ __( 'Reset' ) }
	</Button>

As said earlier, this issue isn't just about the 'Reset' buttons. it is about any control that dynamically gets a disabled attribute while it is focused.

[afercia]

Specifically to the Font size picker, there's at least one occurrence where it still shows the Reset button:

• Go to the Site Editor
• Select a Group block
• In the Settings sidebar, go to the Styles tab
• Observe the Typography panel shows the preset sizes
• Click 'Set custom size'
• The Typography panel switches to the UI to set a custom size
• Observe there is a 'Reset' button after the Size slider

[youknowriad]

Good catch, I think adding the __experimentalIsFocusable to these Button components should probably solve the issue.