💅 a11y/noNoninteractiveTabindex incorrectly labels treegrid child roles as non-interactive

[drwpow]

Environment information

N/A

Rule name

lint/a11y/noNoninteractiveTabindex

Playground link

https://biomejs.dev/playground/?code=ZQB4AHAAbwByAHQAIABmAHUAbgBjAHQAaQBvAG4AIABUAHIAZQBlAEcAcgBpAGQAKAApACAAewAKACAAIAByAGUAdAB1AHIAbgAgACgACgAgACAAIAAgADwAZABpAHYAIAByAG8AbABlAD0AIgB0AHIAZQBlAGcAcgBpAGQAIgAgAGEAcgBpAGEALQBhAGMAdABpAHYAZQBkAGUAcwBjAGUAbgBkAGUAbgB0AD0AIgBjAGUAbABsAC0AMQAtADIAIgA%2BAAoAIAAgACAAIAAgACAAPABkAGkAdgAgAHIAbwBsAGUAPQAiAGMAbwBsAHUAbQBuAGcAcgBvAHUAcAAiAD4ACgAgACAAIAAgACAAIAAgACAAPABkAGkAdgAgAHIAbwBsAGUAPQAiAHIAbwB3ACIAPgAKACAAIAAgACAAIAAgACAAIAAgACAAPABkAGkAdgAgAGkAZAA9ACIAYwBvAGwALQAxACIAIAByAG8AbABlAD0AIgBjAG8AbAB1AG0AbgBoAGUAYQBkAGUAcgAiACAAdABhAGIASQBuAGQAZQB4AD0AewAwAH0APgBDAG8AbAB1AG0AbgAgADEAPAAvAGQAaQB2AD4ACgAgACAAIAAgACAAIAAgACAAIAAgADwAZABpAHYAIABpAGQAPQAiAGMAbwBsAC0AMgAiACAAcgBvAGwAZQA9ACIAYwBvAGwAdQBtAG4AaABlAGEAZABlAHIAIgAgAHQAYQBiAEkAbgBkAGUAeAA9AHsAMAB9AD4AQwBvAGwAdQBtAG4AIAAyADwALwBkAGkAdgA%2BAAoAIAAgACAAIAAgACAAIAAgACAAIAA8AGQAaQB2ACAAaQBkAD0AIgBjAG8AbAAtADMAIgAgAHIAbwBsAGUAPQAiAGMAbwBsAHUAbQBuAGgAZQBhAGQAZQByACIAIAB0AGEAYgBJAG4AZABlAHgAPQB7ADAAfQA%2BAEMAbwBsAHUAbQBuACAAMwA8AC8AZABpAHYAPgAKACAAIAAgACAAIAAgACAAIAA8AC8AZABpAHYAPgAKACAAIAAgACAAIAAgADwALwBkAGkAdgA%2BAAoAIAAgACAAIAAgACAAPABkAGkAdgAgAHIAbwBsAGUAPQAiAHIAbwB3AGcAcgBvAHUAcAAiACAAdABhAGIASQBuAGQAZQB4AD0AIgAwACIAIABhAHIAaQBhAC0AZQB4AHAAYQBuAGQAZQBkAD0AIgBmAGEAbABzAGUAIgA%2BAAoAIAAgACAAIAAgACAAIAAgADwAZABpAHYAIAByAG8AbABlAD0AIgByAG8AdwAiAD4ACgAgACAAIAAgACAAIAAgACAAIAAgADwAZABpAHYAIABpAGQAPQAiAGMAZQBsAGwALQAxAC0AMQAiACAAcgBvAGwAZQA9ACIAZwByAGkAZABjAGUAbABsACIAIAB0AGEAYgBJAG4AZABlAHgAPQB7ADAAfQA%2BAEEAPAAvAGQAaQB2AD4ACgAgACAAIAAgACAAIAAgACAAIAAgADwAZABpAHYAIABpAGQAPQAiAGMAZQBsAGwALQAxAC0AMgAiACAAcgBvAGwAZQA9ACIAZwByAGkAZABjAGUAbABsACIAIAB0AGEAYgBJAG4AZABlAHgAPQB7ADAAfQA%2BAEIAPAAvAGQAaQB2AD4ACgAgACAAIAAgACAAIAAgACAAIAAgADwAZABpAHYAIABpAGQAPQAiAGMAZQBsAGwALQAxAC0AMwAiACAAcgBvAGwAZQA9ACIAZwByAGkAZABjAGUAbABsACIAIAB0AGEAYgBJAG4AZABlAHgAPQB7ADAAfQA%2BAEMAPAAvAGQAaQB2AD4ACgAgACAAIAAgACAAIAAgACAAPAAvAGQAaQB2AD4ACgAgACAAIAAgACAAIAA8AC8AZABpAHYAPgAKACAAIAAgACAAPAAvAGQAaQB2AD4ACgAgACAAKQA7AAoAfQA%3D

Expected result

Both treegrid and grid are described to work like spreadsheets on the web, where data can be edited and individual cells can be focused within and navigated using arrow keys.

Treegrid takes it one step further and allows expanding/collapsing of rows and columns like a tree.

So when trying to implement focus behavior, this throws an error when it shouldn’t:

// The HTML element div is non-interactive. Do not use tabIndex. (❌ Incorrect)
<div role="gridcell" tabIndex={0}>
                     ~~~~~~~~~~~~

This conflicts with the recommended behavior of treegrid children:

Right Arrow
If focus is on a collapsed row, expand the row. …

In other words, a gridcell MUST be focusable when inside a treegrid if it’s expandable (via tabIndex), and also MUST be able to receive other keyboard commands.

lint/a11y/noNoninteractiveTabindex is correct in other instances, so it seems this is only a problem with grid and treegrid children.

Other notes

• The useSemanticElements rule does apply to this example but is irrelevant; noNoninteractiveTabindex still raises the same errors
• Worth noting that, yes, by default many gridcells (and related roles) aren’t interactive, this issue is in certain scenarios they ARE REQUIRED to be
• The “roving tabindex” approach is optional here (where only the focusable element receives tabIndex={0} and everything else gets tabIndex={-1}), but also doesn’t affect the outcome

Code of Conduct

• I agree to follow Biome's Code of Conduct

[drwpow]

I’d be open to raising a PR, but wanted to discuss first whether or not it would be sufficient to remove gridcell (and friends—row, rowgroup, etc.) from the “always non-interactive” list.

I’m inclined to do so because looking at the base grid role docs, we see keyboard interactions suggested even for native <table> elements. This leads me to believe that even though the base elements aren’t focusable using browser defaults, they are still recommended to be in many usecases. Therefore tabIndex shouldn’t be discouraged.

However, if we wanted to take a more complex approach, where gridcell and friends are only interactive under certain conditions (e.g. only if a child of treegrid), I’d feel less confident opening a PR until we defined that criteria (and whether or not we could lint that if the components were broken up). Either way, open to thoughts!

[ematipico]

For these cases, where elements can be interactive based on certain conditions, we assume they can always be interactive.

So, to answer your question @drwpow, let's remove them :)

[drwpow]

Works for me! Will open up a PR with that change then 🙏

[Conaclos]

@drwpow I am landing important refactoring to biome_aria. I recommend working on the current issue once #4495 got merged.

[drwpow]

@drwpow I am landing important refactoring to biome_aria. I recommend working on the current issue once #4495 got merged.

Great work on that! I didn’t get super far but did notice that it was basically a binary check of “is this a subclass or widget or not” and was going to make a worse hack than your more-correct fix.

Agree with the reasoning stated there, too. After having worked with my own treegrid component some more, and reading more on it, I was just going to have the following roles be possibly-interactive (the other roles I can’t say for sure they should never get tabindex, but I couldn’t find recommendations they do):

• row
• rowgroup
• columnheader
• rowheader
• gridcell
• cell*

* Note: cell is the debatable one here, since it seems like children of table are usually NOT interactive. But in some of the W3 examples they have things like <table role="grid"> … <td> and assert that td elements are just gridcell roles because they’re children, even though I wasn’t aware that is expected behavior (I still thought that elements retained their default roles regardless of parent/children and role was always needed to change that).

All that said, just seems like the easiest path to just say “cell can get tabindex sometimes” rather than “only gridcell can ever get tabindex and we must do deep analysis to make sure it’s a child of grid” which just sounded like a lot of work 😅

If any future PRs make those test cases possible then you can close this issue! But I can also start on this once #4241 is in a good place to make this change

[Conaclos]

For these cases, where elements can be interactive based on certain conditions, we assume they can always be interactive.
So, to answer your question @drwpow, let's remove them :)

The fix should be easy: we just need to edit AriaRole::is_interactive. We should add proper comments and a link to the current issue.

In other words, a gridcell MUST be focusable when inside a treegrid if it’s expandable (via tabIndex), and also MUST be able to receive other keyboard commands.

I think, at some point a proper, we should add a proper Aria query that allows retrieving the roles of parent nodes.
This might allow us to check this sort of thing.

[drwpow]

Sorry—realized after that PR, I never came back to open a PR 😅. I have dug in pretty deep since #4241 merged, and I realize a major missing piece here and in eslint-plugin-jsx-a11y (and its upstream engine aria-query) is the missing HTML in ARIA spec. It provides a lot of critical info in how browsers build the accessibility tree.

I’m hitting related issues like <ul role="tablist"> throwing an error (playground) (which, FWIW eslint-plugin-jsx-a11y doesn’t have, so that is a departure either way) that could be fixed by including this spec.

This is a departure from eslint-plugin-jsx-a11y, which is still bound by longstanding bugs, and is (still) giving folks bad information. But it’s led to folks who need to hit their a11y targets to fork the plugin. It’s sort of a gigantic mess that doesn’t get enough attention—the popular thing is broken, which leads to a bad cycle of inaccessible markup, which makes it seem correct because of all the code that reinforces it. Anyway, all that said, it’s a tough choice between “mirror eslint-plugin-jsx-a11y for the ecosystem” vs “do the correct thing.”

Given 2 options, which road is Biome more interested in? I think there are probably changes in some of the ARIA logic either way, but I know this is a big decision that cascades to pretty much all the ARIA-related rules wither HTML in ARIA is part of the equation or not.