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

Font Library: Unlabelled back button within the Fonts modal dialog #58060

Closed
afercia opened this issue Jan 22, 2024 · 2 comments · Fixed by #58309
Closed

Font Library: Unlabelled back button within the Fonts modal dialog #58060

afercia opened this issue Jan 22, 2024 · 2 comments · Fixed by #58309
Assignees
Labels
[Feature] Font Library [Feature] Typography Font and typography-related issues and PRs [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Jan 22, 2024

Description

The 'back' button within the Fonts modal dialog is completely unlabelled.

  • No visible text
  • No aria-label or aria-labelledby or any other labelin

As such, this button has no accessible name and it is not accessible. One more case of an unlabeled control Cc @ciampo

Honetlym I'm surprised that after 7 years of histoyr of this project, contributors still struggle to understand such basic requirements as providing appropriate labeling for user interface controls. This code has been developed, reviewed, and merged with no attention to basic HTML semantics and accessibility requirements.

<Button
variant="tertiary"
onClick={ handleBack }
icon={ chevronLeft }
size="small"
/>

Looking at the code, it is evident there's no labeling. I'm nor sure I understand how such a code can pass a review and be merged. Not to mention the Button componetn is open to misues, as mentioned in other issues.

Screenshot

Screenshot 2024-01-22 at 10 06 29

Remediation:

  • Preferred: add visible text as the button content.
  • Establish a consistent pattern for all such 'Back' buttons in the UI. There's already an issue for this.
  • Alternatively, add an aria-label and expose the accessible name via a tooltip

:

Step-by-step reproduction instructions

  • Go to Site editor > Styles > Typographi > click a fonr title to open the Fonts modal dialog
  • In the Library tab, click one of the listed fonts.
  • Observe the Back button is unlabeled.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] Typography Font and typography-related issues and PRs labels Jan 22, 2024
@t-hamano
Copy link
Contributor

This issue may also be related to #54431, which mentions focus order. As mentioned in this comment, I think we need to refactor the modal with a Navigator component.

My understanding is that proper refactoring by the Navigator component should also resolve the issue with the back button label.

@ciampo
Copy link
Contributor

ciampo commented Jan 24, 2024

As such, this button has no accessible name and it is not accessible. One more case of an unlabeled control

As mentioned previously, pinging me personally on every single instance of an unlabeled control is not the best way to handle reporting such occasions, as I'm not single-handedly responsible for building and maintaining Gutenberg's UI, nor I have the capacity to reply to all pings. Also, should I be unable to contribute to the project (as I will actually be from February to June), the ping would go unnoticed.

Honetlym I'm surprised that after 7 years of histoyr of this project, contributors still struggle to understand such basic requirements as providing appropriate labeling for user interface controls. This code has been developed, reviewed, and merged with no attention to basic HTML semantics and accessibility requirements.
Looking at the code, it is evident there's no labeling. I'm nor sure I understand how such a code can pass a review and be merged.

As it was put eloquently in a similar issue, "an accessibility bug doesn't mean that contributors lack accessibility training".
It's great that those bugs are noticed, flagged, and addressed, as everyone working on this project is intentioned in improving it day after day (cc @priethor )

Not to mention the Button componetn is open to misues, as mentioned in other issues.

#51740 sounds like the correct place where we should continue the broader conversation. I seem to remember that we already agreed on a general approach, so if you have any capacity, it would be great to get some help in carrying out that plan of action!

@afercia afercia self-assigned this Jan 26, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Font Library [Feature] Typography Font and typography-related issues and PRs [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants