Skip to content

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

Merged
carrillo-erik merged 2 commits intolinode:developfrom
carrillo-erik:fix/M3-7574
Sep 13, 2024
Merged

fix: [M3-7574] - Fix missing labels for User Permission radio buttons#10908
carrillo-erik merged 2 commits intolinode:developfrom
carrillo-erik:fix/M3-7574

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 coliu-akamai and jaalah-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
@github-actions
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
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

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants