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

Add <select> to treegrid's focusable element list #2741

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stephanieleary
Copy link

@stephanieleary stephanieleary commented Jun 29, 2023

Adds <select> to the list of elements that should be able to receive focus in a treegrid row.

Fixes #2740.


WAI Preview Link (Last built on Thu, 29 Jun 2023 19:25:03 GMT).

Adds `select` to the list of elements that should be able to receive focus in a treegrid row.

Fixes w3c#2740.
@mcking65
Copy link
Contributor

mcking65 commented Jul 2, 2023

Hi @stephanieleary,

I am uncertain that the treegrid example could include a select in a cell with only this change. A task force member who is an engineer will need to review the code and test it.

From my limited study of the code, it does not appear to implement all features of the pattern. If I understand the code correctly, it does not support cells that include widgets that require arrow keys to operate.

I assume that when tabindex is set for elements in a cell on line 33, that navigation of the tree grid will break if tabindex is set to 0 on any element that consumes up/down/left/right. A select uses up/down. So, I am assuming that the change in this PR will cause treegrid nav to fail when var focusable = getFocusableElements(cell)[0] || cell; is executed because getFocusableElements would include the select.

Per the pattern definition, if a child of a cell consumes arrow keys, then focus needs to be set on the cell when navigating the grid. If the user is going to operate the child widget, the treegrid needs to provide a mechanism for moving focus inside the cell, which disables grid navigation, and then back to the cell. In some implementations, which we don't have an example of in the APG, pressing enter on the cell opens the contents into a dialog.

Another approach that can be simpler when it is feasible is to make the cell content a button. Activating the button changes the content, e.g. replaces it with an input. The user action of completing the input with enter or canceling the input with Escape restores the button. This approach is illusttrated is in the data grid example 2: Sortable Data Grid With Editable Cells. In example 2, note how the values in the description column are editable. The type column includes a widget similar to a select; it can't be opened with down arrow though.

@a11ydoer a11ydoer requested a review from jongund July 11, 2023 18:51
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Is treegrid Example code able to support nested selects.

The full IRC log of that discussion <jugglinmike> Topic: Is treegrid Example code able to support nested selects
<jugglinmike> github: https://github.com//pull/2741
<jugglinmike> Matt_King: Stephanie_Leary was trying to use the treeview pattern to make a treeview that included a select in a cell
<jugglinmike> Matt_King: I replied that adding a select, that could interfere with the navigation in the treeview
<jugglinmike> Matt_King: I looked around in the code, and I tried to understand the impact of the change on the code. I believe that it will only have negative consequences--not positive
<jugglinmike> Matt_King: I wrote something up to that effect in my reply, but I would like it if someone could actually test it
<jugglinmike> Matt_King: I'm pretty sure that the TreeGrid code is very purpose-built for the kind of tree grid we are exemplifying in APG. I don't think it's quite robust enough to support this kind of extension
<jugglinmike> jongund: I can take a look
<jugglinmike> Matt_King: You've already signed up for a bunch of things in this meeting; that's very generous!

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 this pull request may close these issues.

Treegrid Example: getFocusableElements() does not support including <select> elements in cells
3 participants