Document body should be a focusable element #365

juanca · 2020-06-17T19:34:46Z

@testing-library/user-event version: @testing-library/user-event@10.4.0
Testing Framework and version: jest@25.1.0
DOM Environment: jsdom@16.2.0
node: node/10.20.1

Relevant code or config (I'm also using React)

it('manages the page tab sequence', () => {
  function TestComponent() {
    return (<ul role="listbox">
      <li role="option" tabindex="0">First option</li>
      <li role="option" tabindex="-1">Second option</li>
    </ul>);
  }

  render(<TestComponent />);

  expect(document.body).toHaveFocus();

  userEvent.tab();
  expect(screen.getByText('First option')).toHaveFocus();

  userEvent.tab();
  expect(document.body).toHaveFocus();

  userEvent.tab({ shift: true });
  expect(screen.getByText('First option')).toHaveFocus();
});

What you did:

Ran the test.

What happened:

The test failed.

    expect(element).toHaveFocus()

    Expected
      <body><div><ul role="listbox"><li role="option" tabindex="0">First option</li><li role="option" tabindex="-1">Second option</li></ul></div></body>
    Received:
      <li role="option" tabindex="0">First option</li>

      76 | 
      77 |       userEvent.tab();
    > 78 |       expect(document.body).toHaveFocus();
         |                             ^
      79 | 
      80 |       userEvent.tab({ shift: true });
      81 |       expect(screen.getByText('First option')).toHaveFocus();

Reproduction repository:

Problem description:

It looks like the tab method does not consider the body as a tabable element. That is actually not true -- at least from doing a quick experiment in Chrome.

The following screenshot is the experiement.

Render a button
Look at active element (body)
Press tab
Look at active element (button)
Press tab
Look at active element (body)

Suggested solution:

Assuming the focus trap is on the document, always set the body as the first tabable element.

The text was updated successfully, but these errors were encountered:

juanca · 2020-06-17T22:16:00Z

Possibly related:

Looks like we need to integrate act in userEvent.tab. Otherwise, we should recommend using act with tab. I noticed the user event was not waiting for my stateful component updates. I can look into reproducing this with the same example: use state to manage tab indices (focus => change indices structure).

Update: I think perhaps this might be intended. I was using useImperativeHandle in my components which is a point made in https://kentcdodds.com/blog/write-fewer-longer-tests

React's act utility is built-into React Testing Library. There are very few times you should have to use it directly if you're using React Testing Library's async utilities:

When using jest.useFakeTimers()

When using useImperativeHandle and calling functions directly that call state updaters.

When testing custom hooks and calling functions directly that call state updaters.

marcosvega91 · 2020-06-18T03:00:42Z

Hi @juanca I think that body is not in the list of focusable elements

nickmccurdy · 2020-06-18T05:21:26Z

Whenever I press tab past the end of the document (both on Mac and Windows, and without any focus traps), document.activeElement is set back to the body, but my keyboard's cursor actually goes outside of the browser viewport and into the browser chrome (including the tabs and navigation bar items). So I don't think it's accurate to say that the body can be focused traditionally, but it can be the active element, and the number of times you'd have to press tabs to loop back into the beginning of the page is nondeterministic (as you don't know how many tabs the user has open or what browser UI buttons they have in a test).

The MDN documentation on activeElement seems to confirm this as well:

When there is no selection, the active element is the page's <body> or null.

However, <body> may still be used by activeElement (if null isn't used in its place). We should decide if toHaveFocus should emulate the tab key and be renamed to something more accurate like toBeActiveElement, or if it should literally only emulate the behavior of focusable elements. Alternatively we could have both functions with different names if there's enough demand, but the similarity between them may be too confusing.

I'm also not sure how we should handle continuing to tab past the end of the document when there's no focus trap. Should we add an arbitrary number of fake buttons, loop back around immediately, or error because the functionality is undefined outside of a browser?

juanca · 2020-06-18T15:24:34Z

@nickmccurdy I appreciate the analysis and clarification. I think you're 100% right.

I'm also not sure how we should handle continuing to tab past the end of the document when there's no focus trap. Should we add an arbitrary number of fake buttons, loop back around immediately, or error because the functionality is undefined outside of a browser?

My latest train of thought: I think it is sensible to simulate a single active element outside the page tab sequence.

When a developer is testing a UI element, it seems reasonable to test the case of tabbing past the UI element. If the UI element was a single page application, it seems reasonable to tab out of the application.
As it now stands, the jsdom implementation sets the active element to be document.body on first render (given a React test). This allows a developer to test a UI element from lack of focus to focus without any additional testing overhead. It seems reasonable to provide the reverse: focus to lack of focus without any additional testing overhead.

Suggested theoretical solution:

When there is no focused element (document.activeElement === document.body), pressing tab will focus the first thing in the page tab sequence.
When there is no focused element (document.activeElement === document.body), pressing shift+tab will focus the last thing in the page tab sequence.
When the focused element is the last element in the page tab sequence, pressing tab will set the active element to the body
- I think this is simulated with document.activeElement.blur()
When the focused element is the first element in the page tab sequence, pressing shift+tab will set the active element to the body
- I think this is simulated with document.activeElement.blur()

Outside of scope:

Renaming or splitting toHaveFocus seems to be a problem but I think it might be an additional issue instead of the same issue.

Whaddya'll think?

nickmccurdy · 2020-06-18T16:06:02Z

That sounds good.

Maybe we should just leave toHaveFocus alone. It's not a perfectly accurate name, but it is practical, and I think it would be too confusing to have a separate matcher for the unfocused state. Since activeElement is actually body when nothing is focused in JSDOM, I think it's fair to treat that as focused, even if it's slightly different from browser semantics.

See testing-library#365 for context

* Cycle focus between the body and page tab sequence See #365 for context * Update documentation on `tab` method to reflect cycling with document.body Closes #365

kentcdodds · 2020-06-21T01:59:37Z

🎉 This issue has been resolved in version 12.0.3 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

nickmccurdy added accuracy Improves the accuracy of how behavior is simulated needs investigation Someone has to do research on this labels Jun 18, 2020

nickmccurdy removed the needs investigation Someone has to do research on this label Jun 18, 2020

juanca added a commit to juanca/user-event that referenced this issue Jun 19, 2020

Cycle focus between the body and page tab sequence

efa87a3

See testing-library#365 for context

juanca mentioned this issue Jun 19, 2020

Cycle focus between the body and page tab sequence #368

Merged

4 tasks

kentcdodds closed this as completed in #368 Jun 21, 2020

kentcdodds added the released label Jun 21, 2020

SimonFinney mentioned this issue Jun 16, 2021

build: upgrade testing dependencies, update configuration, fix tests carbon-design-system/ibm-security#1006

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document body should be a focusable element #365

Document body should be a focusable element #365

juanca commented Jun 17, 2020

juanca commented Jun 17, 2020 •

edited

marcosvega91 commented Jun 18, 2020

nickmccurdy commented Jun 18, 2020 •

edited

juanca commented Jun 18, 2020 •

edited

nickmccurdy commented Jun 18, 2020

kentcdodds commented Jun 21, 2020

Document body should be a focusable element #365

Document body should be a focusable element #365

Comments

juanca commented Jun 17, 2020

juanca commented Jun 17, 2020 • edited

marcosvega91 commented Jun 18, 2020

nickmccurdy commented Jun 18, 2020 • edited

juanca commented Jun 18, 2020 • edited

nickmccurdy commented Jun 18, 2020

kentcdodds commented Jun 21, 2020

juanca commented Jun 17, 2020 •

edited

nickmccurdy commented Jun 18, 2020 •

edited

juanca commented Jun 18, 2020 •

edited