Jest-dom: toBeActiveDescendant

Created on 17 Feb 2020  路  10Comments  路  Source: testing-library/jest-dom

Describe the feature you'd like:

I'm testing composite components that use aria-activedescendant to move focus. I would like to test whether some item is "virtually" focused or not after pressing arrow keys:

const Test = () => (
  <Composite aria-label="composite">
    <CompositeItem>item1</CompositeItem>
    <CompositeItem>item2</CompositeItem>
    <CompositeItem>item3</CompositeItem>
  </Composite>
);

const { getByText, getByLabelText } = render(<Test />);
const composite = getByLabelText("composite");
const item1 = getByText("item1");
const item2 = getByText("item2");
const item3 = getByText("item3");

composite.focus();

pressArrowRight();
expect(composite).toHaveFocus();
expect(item1).toBeActiveDescendant();

pressArrowRight();
expect(composite).toHaveFocus();
expect(item2).toBeActiveDescendant();

...

Teachability, Documentation, Adoption, Migration Strategy:

toBeActiveDescendant

toBeActiveDescendant(element: Element)

This allows you to check whether the given element is currently referenced in the aria-activedescendant attribute on the element which currently has DOM focus.

Examples

<ul role="listbox" aria-activedescendant="option1" data-testid="listbox">
  <li role="option" id="option1" aria-selected="true" data-testid="option1">option 1</li>
  <li role="option" id="option2">option 2</li>
  <li role="option" id="option3">option 3</li>
</ul>
const listbox = getByTestId('listbox')
const option1 = getByTestId('option1')

listbox.focus()
expect(option1).toBeActiveDescendant()

enhancement

Most helpful comment

I'm even offering myself to do it, unless @diegohaz wants to work it.

Ah, feel free to take it! :)

All 10 comments

Hi, thanks for opening this issue and providing so much detail :+1:

I follow the general gist of it but have some concern regarding the name. The name implies that this is "just" expect(listbox).toHaveAttribute('aria-activedescendant', option1.id). It's not clear where the composite widget comes from until you see the the implementation.

The listbox could still have the aria-activedescendant attribute even without having focus and you might want to assert that this is explicitly preserved after blurring the widget. At least I don't know of any restriction of aria-activedescendant.

You're essentially suggesting:

- expect(listbox).toHaveFocus();
- expect(listbox).toHaveAttribute('aria-activedescendant', option1.getAttribute('id'))
+ expect(option1).toBeActiveDescendant();

Personally I would prefer a composite assertion (following the composite nature of the widget):

- expect(listbox).toHaveFocus();
- expect(listbox).toHaveActiveDescendant(option1)

and then maybe add an additional helper with a bit more verbose name such as toHaveVirtualFocus() (or something else).

Regarding implementation:

  1. check that aria-activedescendant is on an element supporting this attribute
  2. We should assert that the active descendant is an actual descendant of or owned by the composite.
    or
  3. the element is a textbox with aria-controls pointing to an element supporting aria-activedescendant.

I'm all for it.

There are interesting alternative ways to put it in @eps1lon's response. I still think I prefer what @diegohaz originally suggested. That is, I prefer expect(option).toBeActiveDescendant() over expect(listbox).toHaveActiveDescendant(option). With the former you only need the option, you do not need to grab what its listbox is, and the matcher will check that whatever element points to that option as the active descendant is also the currently active element as well. With the latter, I'm not sure if that assertion implies checking that listbox is the currently focused element.

As for the name, toHaveVirtualFocus() sounds good, but I like that toBeActiveDescendant() is more aligned with the naming of this feature in the code.

No strong opinions here, I'm open to be challenged in all this.

With the former you only need the option

I generally want to optimize for readability. It's the thing we do more often than writing.

Oh ok. Fair enough.

But in my case I do not see it less readable. Just wanted to clear that up. Because I too favor readability. What is it less readable about the proposed api?

Anyway, let鈥檚 wait for @diegohaz鈥檚 thoughts on this. In any case my concerns are kind of nit picks because in any of these forms this app will be a welcome feature. No strong feelings either way.

What is it less readable about the proposed api?

So readable was probably the wrong word. "reasonable" or "being a test that I can follow" may be more appropriate. It may be unrelated but I've recently read some "older" test of mine using document.activeElement and every time I had to ask myself: "what element is actually active?" and "is it actually active?". It seems like the same would apply here. The implicit assumption is that it uses the active element i.e. document.activeElement but I would suspect that an explicit assertion about what the active element is would go a long way in describing the behavior that we're testing.

Ok, I can relate to that. Thanks for clarifying it. Let's wait for @diegohaz's input then on what he things of all this. I hope we can get this soon. I'm even offering myself to do it, unless @diegohaz wants to work it.

Thank you for the quick and thoughtful responses! :)

I see how expect(option).toBeActiveDescendant() could be confusing. Even if you know what aria-activedescendant means (and unfortunately most developers don't), it's not obvious if this is only testing the aria attribute or the DOM focus as well.

The feature I'm proposing here is more an equivalent to toHaveFocus(), but for widgets where focus is managed via the aria-activedescendant attribute. So, toHaveVirtualFocus() seems to be more descriptive.

I was also curious about the term virtual. I usually use it to refer to this focus management strategy, but I hadn't found any mention to this term on the WAI-ARIA docs.

Found references on the MDN docs though: https://developer.mozilla.org/en-US/docs/Web/Accessibility/Keyboard-navigable_JavaScript_widgets#Managing_focus_inside_groups

aria-activedescendant: managing a 'virtual' focus

An audacious alternative is to enhance toHaveFocus() so it returns true for elements that have this virtual focus too. It makes sense if you consider that, both in the UI and for screen readers, those elements should behave as they were actually focused.

An audacious alternative is to enhance toHaveFocus() so it returns true for elements that have this virtual focus too

These match different CSS pseudo-classes so I wouldn't merge these. It's oftentimes intended to outline the widget container and their active descendant.

I'm even offering myself to do it, unless @diegohaz wants to work it.

Ah, feel free to take it! :)

I may start to work on this soon, and I'm still torn on how to name it. Though it's not just the name conundrum, but also how the API would work.

Will outline the alternatives I distill from the discussion so far. Let me know if something here is being misrepresented. Also, would like to know if you see a path forward where we could offer ore than one of these alternatives at the same time (they do not seem to be strictly conflicting, but it could be confusing).

1. expect(option).toBeActiveDescendant()

The original proposal. Receives only the actual option element and asserts it is the active descendant of the element that's currently in focus.

Could be named: toHaveVirtualFocus and keep the same interface.

Cons

Assumes we always want to check the active descendant on the currently focused element. It could happen that I want to assert what's the active descendant of a listbox that's not necesarily focused.

2. expect(listbox).toHaveActiveDescendant(option)

Would not check that listbox has the focus, only that its active descendant is the given option element. Users would need to assert the listbox focus themselves if they want to, or use document.activeElement directly in place of listbox.

Solves the "Cons" part of the previous alternative. Based on this suggestion in a comment above: https://github.com/testing-library/jest-dom/issues/207#issuecomment-586882194

Cons

A bit more work if users want to assert that the owner element of the active descendant also has the actual focus (could be a pretty common scenario). Users could even omit that extra check by mistake or ignorance in tests that require would benefit from it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

gregberge picture gregberge  路  6Comments

anyexinglu picture anyexinglu  路  4Comments

BurntCaramel picture BurntCaramel  路  5Comments

benmonro picture benmonro  路  3Comments

gnapse picture gnapse  路  6Comments