ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
29 stars 9 forks source link

Need ability to override tabIndex on controls #2094

Closed msmithNI closed 1 month ago

msmithNI commented 1 month ago

Dependency of: table keyboard navigation ( #1137 )

😯 Problem to Solve

Currently several of our controls have a hardcoded tabIndex in their template. This is problematic for the table keyboard navigation feature, when we need to programmatically change tabIndex (for 'roving tabIndex' as we programmatically focus only a specific element).

The controls we initially need this for are those used in focusable parts of the table:

Desired behavior:

πŸ’ Proposed Solution

Current plan (based on prototyping) is to override the tabIndex property and add @attr:

@attr({ attribute: 'tabindex', converter: nullableNumberConverter })
public override tabIndex!: number;

In templates, if a control contains another focusable control, it should set tabIndex on the inner control, i.e. tabindex="${x => x.tabIndex}" In cases where tabIndex is currently set conditionally if not disabled
(tabindex="${x => (x.disabled ? null : 0)}"), we need something closer to
tabindex="${x => (x.disabled ? null : (typeof x.tabIndex === 'number' ? x.tabIndex : 0))}"

For some of these controls, we haven't yet forked / brought in the template from FAST, which we'll need to do as part of this work.

πŸ“‹ Tasks

m-akinc commented 1 month ago

Apparently the need to forward the tabindex is browser dependent. I noticed that if I set tabindex="-1" on a button, it prevents tabbing to it in Chrome, but not in Firefox. Didn't check Safari.

Also, we don't need to forward tabindex for the Checkbox, becauase it doesn't delegate focus to the shadow root. Turns out for Checkbox, we don't need to forward to a shadow DOM element, but we need to avoid overwriting a provided tabindex value with a hardcoded 0.