adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
13.11k stars 1.14k forks source link

[RAC][Table] add `aria-colindex` to `Column` and `Cell` #4968

Open romansndlr opened 1 year ago

romansndlr commented 1 year ago

Provide a general summary of the feature here

In react-spectrum each column and cell gets a aria-colindex attribute: https://react-spectrum.adobe.com/react-spectrum/TableView.html. This is very helpful for testing and I'd love to see the exact same thing supported for RAC.

๐Ÿค” Expected Behavior?

Expose an index prop in some way so that is can be applied to the columns and cells. Or even better would be for it to be applied behind the scenes.

๐Ÿ˜ฏ Current Behavior

There is no way to add aria-colindex to the Cell or the Column.

๐Ÿ’ Possible Solution

No response

๐Ÿ”ฆ Context

This helps a lot with testing, ideally when testing you want to assert a cell's content based on the column. In order to do that you need to know the index of the column inside the row.

๐Ÿ’ป Examples

No response

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

reidbarber commented 1 year ago

We do this in React Spectrum's TableView because we are using virtualization.

If all of the columns are present in the DOM, it is not necessary to set this attribute as the user agent can automatically calculate the column index of each cell or gridcell. However, if only a portion of the columns is present in the DOM at a given moment, this attribute is needed to provide an explicit indication of the column of each cell or gridcell with respect to the full table.

From https://www.w3.org/TR/wai-aria-1.2/#aria-colindex

That being said, we may want to offer a way to enable this in RAC for those using virtualization, or see if we could detect it automatically.

jaknas commented 1 year ago

We do this in React Spectrum't TableView because we are using virtualization.

If all of the columns are present in the DOM, it is not necessary to set this attribute as the user agent can automatically calculate the column index of each cell or gridcell. However, if only a portion of the columns is present in the DOM at a given moment, this attribute is needed to provide an explicit indication of the column of each cell or gridcell with respect to the full table.

From https://www.w3.org/TR/wai-aria-1.2/#aria-colindex

That being said, we may want to offer a way to enable this in RAC for those using virtualization, or see if we could detect it automatically.

Slightly off-topic, but are you folks planning to bring virtualization into RAC Table? I was unsuccessful when trying to integrate something like tanstack virtual, with regards to accessibility, drag and drop and some other RAC Table features.

P.S the documentation mentions support for infinite and virtualized scrolling, which I'm not sure is true? Or there are just no examples showcasing that.

reidbarber commented 1 year ago

@jaknas It isn't in the near-term plans. We will definitely remove that from the docs before the GA release. I think it is something we will consider adding in the future, however.

romansndlr commented 1 year ago

Again, just to clarify, the reason I'm interested in this is testing. There is no other straightforward way to assert a cell's value based on the column it is in. Also seems like y'all already maintain the index internally so I'm assuming it shouldn't be to crazy to apply it. I'm willing to take a crack at it if any of you are willing to review it.

snowystinger commented 1 year ago

We're happy to review a contribution!

Side note, what testing library are you using? You could make use of data-testid https://codesandbox.io/s/romantic-shtern-knsyck?file=/src/App.js

I'd have a look through this as well https://github.com/testing-library/dom-testing-library/issues/583

devongovett commented 1 year ago

I don't think this attribute is supposed to be there unless some of the columns are missing as Reid mentioned. I'd recommend using a custom data attribute for testing instead.

romansndlr commented 1 year ago

I'm using Playwright to test and I'm totally fine with using a data-attribute. I just need some indication on the cell to what column it is in. From what I can tell, there is no way for me to do that in user land.