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 aria label to action th #14303

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

yogesh16
Copy link

Description

This pull requests, add aria-label="actions" to action th
Issue #14299

Visual changes

Before
Screenshot 2024-09-20 at 7 26 38 AM
After
Screenshot 2024-09-20 at 7 26 29 AM

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@ju5t
Copy link
Contributor

ju5t commented Sep 21, 2024

the table header for that column lacks an accessible name, which is required for elements with the columnheader role.

This is not true. I agree that it's something that should be added, but it's not required. I would argue that the label should be more descriptive though. From an accessibility standpoint a user wouldn't know what to expect if there are just 'actions'.

Perhaps you can loop over all the defined actions in the table and make it look like:

aria-label="Edit, move or delete records"

Or if it's just one action:

aria-label="Edit records"

@zepfietje zepfietje linked an issue Sep 26, 2024 that may be closed by this pull request
@zepfietje zepfietje changed the title 🎉 Add aria label to action th Add aria label to action th Sep 26, 2024
@zepfietje zepfietje added this to the v3 milestone Sep 26, 2024
@zepfietje zepfietje added the a11y label Sep 26, 2024
@danharrin
Copy link
Member

There could be many actions, especially if they are grouped, so I don't think listing them is what we should do here.

Also, this current PR is only suitable for English apps since the string is not translated

@ju5t
Copy link
Contributor

ju5t commented Oct 7, 2024

@danharrin that's true. Perhaps it's possible to count all specified actions to decide if the translation should be plural?

@danharrin
Copy link
Member

Sure, but it doesn't just need to adapt to the plural it also needs to be translated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

Table actions column header WCAG compliance
4 participants