Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
270 stars 74 forks source link

Consistent focus/blur eventing across components #5147

Open jcfranco opened 1 year ago

jcfranco commented 1 year ago

Overview

Currently, developers have to rely on focusin/focusout (composable + bubbling) from component internals or using, internal, blur/focus events for a few components.† The goal for this effort is to provide a general solution for focus/blur events given that these events are commonly used and expected in many web development scenarios.

Also for consideration, setFocus has introduced a pattern that can lead to inconsistent focus behavior. Existing usage should be revisited to see if we can consolidate in favor of consistency (one focus pattern to rule them all).

† In general, this works but leads to boilerplate code as focusin/focusout fires when you move between elements inside a component and would need to check where focus lies when handling the event.

Tasks

Additional resources

cc @benelan @geospatialem

benelan commented 1 year ago

Findings

Disabled behavior

It turns out delegatesFocus does not take into account tabindex or pointer-events: none when determining the first focusable element on click and focus(). This is an issue for us because components emulated as disabled will still be delegated focus if they are the first element in the flat tree.

Using inert on the mock "disabled" element solves this issue, but there isn't enough browser support at the moment.

Checked behavior

delegatesFocus also doesn't understand our custom checked property (e.g. in radio-group). Meaning it will always focus the first radio-group-item in the flat tree.

However, focus is delegated to elements with the native checked property (e.g. radio-button-group), even if the radio-button isn't the first element.

Focus method

Calling the focus() method on components that delegate focus only works correctly once the component is fully loaded. In order to get rid of our async setFocus method, users would need to ensure the component is ready before using focus().

Examples This correctly delegates focus to the internal input: ```js customElements.whenDefined("calcite-filter").then(() => document.querySelector("calcite-filter").setFocus()); ``` This does as well, but not without the timeout: ```js customElements .whenDefined("calcite-filter") .then(() => setTimeout(() => document.querySelector("calcite-filter").focus(), 1000)); ``` This works too: ```js (async () => { await customElements.whenDefined("calcite-date-picker"); const el = await document .querySelector("calcite-date-picker") .componentOnReady(); requestAnimationFrame(() => el.focus()); })(); ```

Next steps

Resources

Here is a codepen demonstration: https://codepen.io/benesri/pen/PoavMWM

Here is the WICG issue where they made that decision: https://github.com/WICG/webcomponents/issues/830

Specifically this snippet from the issue body:

Since we want to change the behavior of focus delegation to not be related to sequential focus navigation, we should probably remove the tabindex priority thing as well in this case. So we should always delegate focus to the first focusable area in DOM/composed-tree order

and this comment near the bottom:

Sounds like there was a consensus to move forward with what's being proposed thus far:

  • Programmatic focus would focus the first element in the flat tree that's programmatically focusable.
  • Mouse focus would focus the first element in the flat tree that's mouse focusable.
  • Keyboard focus would focus the first element in tab index order that's keyboard focusable.

Skipped components

expand list ### Selected value Skipped due to the inconsistency between `radio-group` and `radio-button-group` delegating focus to the first element in the flat tree versus the `checked` element. - `radio-group` ### Disabled issue If a "mock disabled" element is first in the flat tree, it will be delegated focus. A lot of our components can have slotted content as the first focusable element. - `alert` - `accordion` - `accordion-item` - `block` - `block-section` - `card` - `combobox` - `combobox-item-group` - `date-picker-month` - `flow` - `flow-item` - `list` - `list-item` - `list-item-group` - `modal` - `notice` - `option-group` - `panel` - `pick-list` - `pick-list-group` - `pick-list-item` - `popover` - `shell` - `shell-center-row` - `shell-panel` - `stepper` - `stepper-item` - `select` - `tab` - `tabs` - `tab-nav` - `tile` - `tile-select` - `tile-select-group` - `tree` - `tip` - `tip-group` - `tree-item` - `value-list` - `value-list-item` - `label` ? - `rating` ? ### Nothing to delegate focus to The components don't need to delegate focus to another element. - `action` - `avatar` - `button` - `chip` - `checkbox` - `date-picker-day` - `date-picker-month-header` - `dropdown-item` - `combobox-item` - `fab` - `graph` - `handle` - `icon` - `link` - `progress` - `input-message` - `input-text` - `input` - `input-number` - `option` - `scrim` - `switch` - `tab-title` - `textarea` - `tooltip` - `radio-button` - `radio-group-item`
benelan commented 1 year ago

@geospatialem I moved this to Stalled because a lot of the components need Franco's feedback. But can you please verify the components that have been completed so far when you have time? You can use the setFocus method to ensure the first focusable element is focused. Here is an example for filter: https://codepen.io/benesri/pen/XWYvoGa?editors=1010

geospatialem commented 1 year ago

@benelan Thanks for the thorough list to work through! I wasn't able to verify the setFocus method for the components listed below.

Is there an additional step needed to ensure setFocus is selecting the first focusable element?

benelan commented 1 year ago

Taking another look at inline-editable. The rest don't have setFocus() methods so you can use a timeout and focus()

  customElements
    .whenDefined("calcite-date-picker")
    .then(() => setTimeout(() => document.querySelector("calcite-date-picker").focus(), 1000));
benelan commented 1 year ago

This works for inline-editable

(async () => {
  await customElements.whenDefined("calcite-inline-editable");
  const el = await document
    .querySelector("calcite-inline-editable")
    .componentOnReady();
  requestAnimationFrame(() => el.setFocus());
})();

Edit

The above code also works with the native focus() method so you don't need a timeout.

geospatialem commented 1 year ago

Awesome, thanks for the insights! ✨ The above components are verified, will await for discussion on the skipped components upon Franco's return.