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

fix: [M3-7574] - Fix missing labels for User Permission radio buttons #10908

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

carrillo-erik
Copy link
Contributor

Description 📝

This PR fixes the problem in which the "None", "Read-Only", and "Read-Write" radio buttons under the "Specific Permissions" of the User Permissions page do not have any label or ARIA label, making it difficult to interact with the page using screen readers.

Changes 🔄

List any change relevant to the reviewer.

  • Replaces native <label /> element with MUI equivalent components.
  • Changes the content used by screen readers to express information to the user.
  • Changes the label positioning of the radio buttons in accordance with Advisory Technique G162 of the WCAG 2.1 standards.

Target release date 🗓️

09/11/2024

Preview 📷

Before After
m3-7475-before m3-7475-after

How to test 🧪

Prerequisites

(How to setup test environment)

  • Visit the User Permissions landing page and activate the VoiceOver Utility (⌘ + F5) on your MAC computer.

Verification steps

(How to verify changes)

  • Use your keyboard to navigate to the Specific Permissions panel and to select different permissions for different entities.
  • Pay attention to the audio output of the VoiceOver utility and/or inspect the text transcript to verify the expected output.
  • Verify that all Permission-related functionality works as expected.
  • Verify that there are no unexpected visual regressions.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@carrillo-erik carrillo-erik self-assigned this Sep 9, 2024
@carrillo-erik carrillo-erik requested a review from a team as a code owner September 9, 2024 16:28
@carrillo-erik carrillo-erik requested review from jaalah-akamai and coliu-akamai and removed request for a team September 9, 2024 16:28
@carrillo-erik carrillo-erik added the Accessibility Contains accessibility improvements or changes label Sep 9, 2024
Copy link

github-actions bot commented Sep 9, 2024

Coverage Report:
Base Coverage: 86.64%
Current Coverage: 86.64%

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looks good

value="null"
/>
}
label="None"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using the span.MuiFormControlLabel-label selector on the TableRow, we could apply the bold style here.

Honestly not sure which is better. Sometimes I'm hesitant with using MUI classnames because they could change in future MUI releases and cause the UI to break

Suggested change
label="None"
slotProps={{
typography: {
sx: (theme) => ({ fontFamily: theme.font.bold }),
},
}}
label="None"

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome thanks Erik! 🎉

don't forget the changeset!

@coliu-akamai coliu-akamai added Approved Multiple approvals and ready to merge! Missing Changeset labels Sep 9, 2024
@carrillo-erik carrillo-erik merged commit 46da3e1 into linode:develop Sep 13, 2024
18 of 19 checks passed
nikhagra-akamai pushed a commit to nikhagra-akamai/manager that referenced this pull request Sep 23, 2024
…linode#10908)

* fix: [M3-7574] - Fix missing label for user permission radio btns

* Add changeset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Contains accessibility improvements or changes Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants