Skip to content

MudCollapse, MudStack, MudToolbar: Improve accessibility#9059

Merged
henon merged 3 commits intoMudBlazor:devfrom
danielchalmers:a11y-6
May 26, 2024
Merged

MudCollapse, MudStack, MudToolbar: Improve accessibility#9059
henon merged 3 commits intoMudBlazor:devfrom
danielchalmers:a11y-6

Conversation

@danielchalmers
Copy link
Member

@danielchalmers danielchalmers commented May 25, 2024

Description

MudCollapse

  • Add aria-expanded="@Expanded"

MudStack

  • Add role="group"

MudToolbar

  • Add role="toolbar"
  • Move code to .razor.cs file
  • Tweak xml docs

How Has This Been Tested?

visually

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

MudCollapse
- Add `aria-expanded="@expanded"`

MudMainContent
- Use semantic `main` tag instead of plain `div`

MudStack
- Add `role="group"`
@github-actions github-actions bot added enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library PR: needs review labels May 25, 2024
@codecov
Copy link

codecov bot commented May 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.65%. Comparing base (28bc599) to head (f39f6ac).
Report is 235 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #9059      +/-   ##
==========================================
+ Coverage   89.82%   90.65%   +0.82%     
==========================================
  Files         412      398      -14     
  Lines       11878    12376     +498     
  Branches     2364     2403      +39     
==========================================
+ Hits        10670    11219     +549     
+ Misses        681      620      -61     
- Partials      527      537      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielchalmers danielchalmers added the accessibility Accessibility concerns (ARIA, keyboard, focus, screen readers, contrast) label May 25, 2024
@danielchalmers
Copy link
Member Author

cc @igotinfected 2 of 3

@ScarletKuro ScarletKuro requested a review from henon May 25, 2024 08:19
@danielchalmers
Copy link
Member Author

danielchalmers commented May 25, 2024

@igotinfected Do you think MudCard should use header and footer for MudCardHeader and MudCardActions? Or role=img for MudCardMedia?

@danielchalmers danielchalmers changed the title MudCollapse, MudMainContent, MudStack: Improve accessibility MudCollapse, MudStack, MudToolbar: Improve accessibility May 25, 2024
@danielchalmers
Copy link
Member Author

@igotinfected Replaced MudMainContent improvements with MudToolbar

@igotinfected
Copy link
Member

@igotinfected Do you think MudCard should use header and footer for MudCardHeader and MudCardActions? Or role=img for MudCardMedia?

footer and header make sense to me, the only thing to be careful about is that headers and footers may not contain nested headers/footers. I don't see a use case where that would make sense though with MudCard 🤷🏼

@igotinfected Replaced MudMainContent improvements with MudToolbar

Awesome! Like for radiogroup, having the option to pass an aria-label or aria-labelledby should be considered, though that is already possible with UserAttributes.

@danielchalmers
Copy link
Member Author

@henon Ready for review

Copy link
Contributor

@jperson2000 jperson2000 left a comment

Choose a reason for hiding this comment

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

LGTM. These are great improvements, and ya even fixed up the docs. Do you want to know of other Toolbar components to update? I wasn't sure if you had a plan for those already.

@danielchalmers
Copy link
Member Author

LGTM. These are great improvements, and ya even fixed up the docs. Do you want to know of other Toolbar components to update? I wasn't sure if you had a plan for those already.

@jperson2000 I'd love to know what you have in mind! I'm doing it pretty much at random for the low hanging fruit right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility Accessibility concerns (ARIA, keyboard, focus, screen readers, contrast) enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants