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

[EuiTable][a11y] select row Checkbox elements must have correct labels #7542

Closed
alexwizp opened this issue Feb 23, 2024 · 8 comments · Fixed by #7672
Closed

[EuiTable][a11y] select row Checkbox elements must have correct labels #7542

alexwizp opened this issue Feb 23, 2024 · 8 comments · Fixed by #7672
Assignees
Labels
a11yReviewNeeded Accessibility design or code review accessibility feature request

Comments

@alexwizp
Copy link
Contributor

We've received reports of a few accessibility (a11y) issues concerning the table component and the select/deselect row functionality. Could we consider adding an aria-label attribute to address these checkbox-related concerns?

image

Proposed solution

Update the input with an aria-label like:

<input
+ aria-label="Select | deselect row 1"
  class="euiCheckbox__input"
  type="checkbox" id="0"
  data-test-subj="bulk-actions-row-cell"
  tabindex="-1"
  data-datagrid-interactable="true"
>
@cee-chen cee-chen added accessibility a11yReviewNeeded Accessibility design or code review labels Feb 23, 2024
@cee-chen
Copy link
Contributor

@1Copenut Does this match your audit/reviews? Also, does a generic "check this row" label help all that much, or do we need to be more specific or individual in the screen reader text we offer?

@1Copenut
Copy link
Contributor

@cee-chen Yes, I've been seeing this in audits for Security Solution this past week. I think the label could be as simple as Check | Uncheck row 1 if that's the only functionality for the box. It answers the question of intent and meets the WCAG success criteria.

@cee-chen
Copy link
Contributor

cee-chen commented Feb 23, 2024

OK hold on, so you do need per-row differentiation? That's not as straightforward as it seems because row numbers are affected by pagination and sorting.

For example, is the first row on the second page (with 20 items per page) considered "Row 1" or is it considered "Row 21"?

@alexwizp
Copy link
Contributor Author

@cee-chen @1Copenut I think it would be ideal if we could calculate rows starting from 0 without linking to pages, filters, sorting, and other operations affecting the number of elements in the visible table area. However, I understand that this might be a bit more complex than just using the index of the data-item in the incoming data array.

On the other hand, any solution you provide will address accessibility issues, so let's choose the option that is simpler for implementing

@1Copenut
Copy link
Contributor

1Copenut commented Mar 4, 2024

@cee-chen && @alexwizp This is a tough question to get the right answer for without user research tbh. I can see both arguments:

  1. We calculate the unique row labels from 0 and attach to each checkbox, regardless of filtering, pagination, etc. This means a user might hear labels for checking/unchecking a row that are non-sequential, IE rows 1, 7, 10, 12, and 15. That may be confusing but it may not. We don't have enough information to say definitively.
  2. We assume the row labels are re-calculated as users paginate, filter, etc. If we're showing five rows, we have five sequential labels that are re-rendered from 0, IE rows 0, 1, 2, 3, 4. We do not concern ourselves with the actual row index. This might be okay but again, not enough information.

I feel like data grids are information dense and meant to be filtered, flipped, sorted. Maybe a way forward is to own that we're not counting indexes and just be clear with the labeling. What if our label was

aria-label="Check current view row 1"

@cee-chen
Copy link
Contributor

cee-chen commented Mar 4, 2024

@1Copenut Are there other accessible table libraries out there with selection functionality that we can shamelessly copy take a leaf from? I'd be really surprised if we were the only design system/library to encounter this problem.

@1Copenut
Copy link
Contributor

1Copenut commented Mar 5, 2024

I have a couple of good ideas based on how Adobe is creating accessible labels in their useGridList hook. Basically they're concatenating two accessible labels by referencing IDs in an aria-labelledby attribute. I think this could be a way forward, and will have it on the docket to discuss next week in the Grooming meeting.

@1Copenut
Copy link
Contributor

@alexwizp I took another look at this over the last couple of days. EUI provides a way for consumers to create accessible checkbox labels in the data grid components. I updated the description and proposed solution in https://github.com/elastic/security-team/issues/8577.

I'm going to close this issue because it's necessary to create accessible labels by picking the best column from data being presented, rather than hardcoding it into design system components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11yReviewNeeded Accessibility design or code review accessibility feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants