Regression with setting focus in test environment

[nwhittaker]

Check existing issues

• [X] I have checked for existing issues to avoid duplicates

Actual Behavior

In an automated test, if multiple components are given focus in quick succession, there's a race condition as to which one will actually retain focus.

Expected Behavior

In an automated test, if multiple components are given focus in quick succession, the last component to gain focus retains it.

Reproduction Sample

https://codepen.io/nwhittaker-esri/pen/eYbRGqP

Reproduction Steps

1. Visit code sample
2. Click the Set focus button and see the first input retains focus despite the 2nd input being focused after.

Reproduction Version

1.8.0

Relevant Info

The code sample aims to mimic how an automated test interacts with components. Specifically via dispatched events instead of calling and awaiting setFocus() directly.

The issue appears to stem from changes in https://github.com/Esri/calcite-design-system/pull/7255 and https://github.com/Esri/calcite-design-system/pull/7277.

Regression?

1.4.3

Priority impact

p2 - want for current milestone

Impact

This issue represents a significant amount of effort for the Field Maps team to debug and rewrite tests in order to upgrade beyond Calcite 1.4.3.

Calcite package

• [X] @esri/calcite-components
• [ ] @esri/calcite-components-react

Esri team

ArcGIS Field Apps

[jcfranco]

We'll have to do some exploratory work here first, but we might be able to help here by avoiding calls to setFocus internally when the host is clicked in some cases. As you probably saw in the issues you referenced, our component's rely on focus being async to make sure the component is rendered before we can do any reliable internal focusing.

For your use case, is it mainly calcite-input wreaking havoc in tests or are there other components that are causing issues? Would it also be possible to see one of your example tests? Also, what's a reasonable timeline for a fix? We can do this via chat or a call too if it makes it easier. 😀

[nwhittaker]

…but we might be able to help here by avoiding calls to setFocus internally when the host is clicked in some cases.

At minimum it'd be nice to bail early if the element already has focus (if possible). Otherwise we don't really have any way to determine when setFocus finishes in that case since we can't wait for an update to document.activeElement.

---

For your use case, is it mainly calcite-input wreaking havoc in tests or are there other components that are causing issues?

calcite-input was convenient for the test case, but I primarily had problems testing keyboard navigation amongst a collection of calcite-radio-button elements (within a custom group element). I suspect it's a potential issue for any component with a setFocus implementation, though, due to its waiting for the next animation frame in componentFocusable.

---

Would it also be possible to see one of your example tests?

Most of the test failures revolve around testing keyboard navigation since the browser's logic for determining the next element is dependent on the current activeElement.

In this example, the keyboard events were sometimes dispatched to the same element instead of the next one, if I recall correctly. The waitForFocus calls were added to work around the regression.

…
expect(radioA.checked).toBeTruthy()
expect(radioA.tabIndex).toBe(0)

userEvent.keyboard('[ArrowDown]')
await waitForFocus(radioB)
expect(radioA.checked).toBeFalsy()
expect(radioA.tabIndex).toBe(-1)
expect(radioB.checked).toBeTruthy()
expect(radioB.tabIndex).toBe(0)

userEvent.keyboard('[ArrowDown]')
await waitForFocus(radioC)
…

async function waitForFocus(element: Element) {
  return waitFor(() => {
    if (document.activeElement === element) return
    throw new Error()
  })
}

---

Also, what's a reasonable timeline for a fix?

We have some workarounds in place now, so we're no longer blocked -- but I could see the issue being a gotcha with new features going forward. Happy to discuss more on Teams, if needed -- but I imagine a phased approach could be worth considering:

1. Higher priority issue to bail setFocus early in cases where focus is delegated(?) or the component already has focus.
2. Lower priority issue to explore eliminating animation frame waiting -- or clarify why/when it's necessary and possibly limiting it to those specific cases.

[geospatialem]

A spike effort will be conducted early in 2024 to help determine a timing estimate, with the goal to include prior to the February schedule if in alignment.