Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aria attributes for selection #490

Closed
louiebert opened this issue Oct 11, 2019 · 5 comments · Fixed by #491
Closed

Aria attributes for selection #490

louiebert opened this issue Oct 11, 2019 · 5 comments · Fixed by #491

Comments

@louiebert
Copy link
Contributor

Summary

There should be aria attributes for when rows are selectable by adding aria-multiselectable to the grid and aria-selected to selected rows.

Expected Behavior

Aria attributes are rendered in the appropriate scenarios.

Current Behavior

If a grid has selectable rows, neither the grid or the rows have the correct aria attributes.

Possible Solution

There could be an additional boolean prop on the grid for indicating multiselect. There could also be another method callback prop on the grid as a way to indicate to an FDTRow that it is selected, so that the correct aria attributes are rendered. That callback prop would provide the row index and would return a boolean value of whether or not the row is selected.

Context

Screen readers need these aria attributes to properly decipher a grid and its state.

@pradeepnschrodinger
Copy link
Collaborator

We're happy to accept a PR for this.

I fear that if we go by the callback prop on the grid approach, we might be extending our basic grid with features that might not be necessary for every user. Also this will slightly complicate the design since other similar features will result in adding even more callbacks/props.

Should we instead focus on having a generic callback function that FDT uses to calculate props for each row? This would probably align with the existing rowHeightGetter prop which is called by FDT for each row in the buffer/viewport. Since selection of rows aren't a direct part of the API, and more like an implementation detail from the user, this approach makes more sense to me.

@wcjordan, @Tanner-MS, any thoughts?

@Tanner-MS
Copy link
Contributor

I was thinking of a generic callback function for any attributes you want to get added to the div that is role="row". This can be put on the rowSettings like the rowHeightGetter.

@wcjordan
Copy link
Member

I like the idea of a generic function which applies attributes to the row.

@louiebert
Copy link
Contributor Author

louiebert commented Oct 16, 2019

So we should go with something like rowAttributesGetter(rowIndex) that returns an object of key-values that are attributes to be applied on the div that is role="row"? The return of that callback would be spread within the attributes for the row div. This way, it can contain aria-selected or any other attributes that users wish to include on the DOM element.

Additionally, for clarification, we aren't looking to replace rowHeightGetter, just add a new callback prop that does something similar, right?

@pradeepnschrodinger
Copy link
Collaborator

So we should go with something like rowAttributesGetter(rowIndex) that returns an object of key-values that are attributes to be applied on the div that is role="row"?

Sounds good. You'll likely place it around here, passed from the bufferer component perhaps.

Additionally, for clarification, we aren't looking to replace rowHeightGetter, just add a new callback prop that does something similar, right?

Yea, we'll just add the new prop.

pradeepnschrodinger pushed a commit that referenced this issue Oct 29, 2019
Added function props `gridAttributesGetter` and `rowAttributesGetter`.

These functions can be used to return the grid/row HTML attributes. These will be directly passed to the DIV elements by FDT.

`rowAttributesGetter` is invoked for each visible row (including buffer) with the `rowIndex` as the first param.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants