MudAlert, MudToggleIconButton, MudChip: Improve accessibility

[danielchalmers]

Description

Part of a series in #8901

Alert:

• Using aria-live="assertive" so screen readers immediately announce the alert's content to users
• Add localizable aria-label for the close icon, with a default English one provided
• Change from a Toggle to a regular button to show intent
• Add mud-alert-close-button class

ToggleIconButton:

• Add ToggleIconButton.AriaLabel, ToggleIconButton.ToggledAriaLabel

Chip:

• Add role="button" to indicate the chip is interactive
• Add localizable aria-label for the close icon, with a default English one provided - Closes Chip close button does not have an aria label #6278
• Add role="group" to ChipSet

MudElement:

• Documents MudElement and uses sequence numbers for the attributes instead of them all being on index 1

Originally I added AriaLabel properties to the button but that required extra workarounds for backwards compatibility and just seemed unnecessary. Not sure which criteria we used for the Title property but that isn't necessarily needed either 🤷

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.

[danielchalmers]

I'm no accessibility expert but I'm learning. My goal is to make a noticeable difference in the usability for people with disabilities

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.50%. Comparing base (28bc599) to head (c6c76c8).
Report is 206 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8966      +/-   ##
==========================================
+ Coverage   89.82%   90.50%   +0.67%     
==========================================
  Files         412      395      -17     
  Lines       11878    12128     +250     
  Branches     2364     2364              
==========================================
+ Hits        10670    10976     +306     
+ Misses        681      620      -61     
- Partials      527      532       +5     

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

[henon]

@igotinfected FYI

[ScarletKuro]

Rest LGTM

[igotinfected]

Awesome! LGTM, the only thing I'm not sure about is the default aria-labels. Now that there is a localizer, should all user-facing text be provided using resources in advance? The benefit is that once translations are provided by the community users wouldn't have to think about overriding aria-labels for useful defaults like Close alert to ensure everything is localized.

I did so in MudNavGroup, but it's not quite the same case:

MudBlazor/src/MudBlazor/Components/NavMenu/MudNavGroup.razor

Lines 7 to 17 in 3fdc056

	<nav @attributes="UserAttributes"
	class="@Classname"
	disabled="@_navigationContext.Disabled"
	style="@Style"
	aria-label="@Title">
	<button @onclick="ExpandedToggleAsync"
	tabindex="@ButtonTabIndex"
	class="@ButtonClassname"
	aria-controls="@_navigationContext.MenuId"
	aria-expanded="@_navigationContext.Expanded.ToString().ToLowerInvariant()"
	aria-label="@Localizer[nameof(LanguageResource.MudNavGroup_ToggleExpand), Title ?? _navigationContext.MenuId]">

Also thank you for the accessibility issue @danielchalmers ❤️ Bit busy with work again but I'll try crossing some things off the list if/when I find the time :)

[henon]

True, those aria label parameters which we can expect to be the same for all instances of a component, such as "Close" can be removed and added to the localized resources instead.

[danielchalmers]

True, those aria label parameters which we can expect to be the same for all instances of a component, such as "Close" can be removed and added to the localized resources instead.

@henon Could you show me how it would be done for this PR?

[henon]

I think @igotinfected can explain it more expertly. A part of the solution is also seen above in his code snippet:

aria-label="@Localizer[nameof(LanguageResource.MudNavGroup_ToggleExpand), Title ?? _navigationContext.MenuId]"

[igotinfected]

You can double click the LanguageResource.resx file in Rider to open up a useful localization window (this will show other languages in the table if other .resx are present):

For Visual Studio I don't remember but there should be something similar. I remember using a resx manager extension though to make it a little easier.

You can then add your key, like this one for MudNavGroup:

You can use {0..} to add input parameters to the string, this is useful for example when you need to pass a value into the middle of a translation, and allows changing the position of that parameter for different languages.

The keys added there will be auto-generated in the LanguageResource.Designer.cs file, so we can reference that key with LanguageResource.<key name>. Better than using a magic string that could potentially change :)

And you use it like this in the .razor or .cs file:

@inject InternalMudLocalizer Localizer

@Localizer[nameof(LanguageResource.<key name>), <parameters if resource key has parameters>]

The @Localizer call will return the localized string based on the current culture of the application.

[danielchalmers]

@igotinfected Nice, thanks a lot for that, PR has been updated! So you think the focus shouldn't be AriaLabel properties for elements with an existing purpose but the language resource file instead? And still have AriaLabels (if relevant) for core components like button, icon button, etc.

[danielchalmers]

@henon I bound the role attribute to a backing string in the Chip but this prevents the user setting their own even when it's null. Does it make sense to switch the @attributes=UserAttributes to a backing field dictionary that we can have our own conditionally added but the user's one will take precedence?

Like (psuedocode)

<div @attributes="@ComponentAttributes">
</div>

@code {
    public Dictionary<string, object?> UserAttributes { get; set; } = new Dictionary<string, object?>();

    private Dictionary<string, object?> ComponentAttributes
    {
        get
        {
            var dict = new Dictionary<string, object?>();
            
            if (IsClickable)
            {
                dict.Add("role", "button");
            }

            foreach (var attr in UserAttributes)
            {
                dict[attr.Key] = attr.Value;
            }

            return dict;
        }
    }
}

[igotinfected]

@igotinfected Nice one, thanks a lot for that, really helpful. So you think the focus shouldn't be AriaLabel properties for elements with an existing purpose but the language resource file instead? And still have AriaLabels (if relevant) for core components like button, icon button, etc.

I think it will be a case by case, we can provide sensible defaults but still allow users to provide their own values when it makes sense to do so. Probably something that will come up in the future once localization really gets going.

The goal should be for all components/html tags users don't have direct access to (say a filter button integrated into MudDataGrid) to be covered from an accessibility point of view, and the base components to allow setting aria properties that need to be accessible (e.g. AdornmentAriaLabel).

[igotinfected]

Also regarding UserAttributes, something worth noting if you don't know already, the order of @attributes="@UserAttributes and other parameters matters.

Say there is role="notbutton" in UserAttributes:

<div @attributes="@UserAttributes" role="button"></div>
<!-- will output -->
<div role="button"></div>

<div role="button" @attributes="@UserAttributes"></div>
<!-- will output -->
<div role="notbutton"></div>

[danielchalmers]

@henon Ready for review. This is the simplest way of handling the attributes but I believe it does mean that if someone write <MudChip role="test"> it will be ignored and the role will either be button or null because of the precedence of the binding.

[henon]

@attributes=UserAttributes to a backing field dictionary that we can have our own conditionally added but the user's one will take precedence?

If we want the user's attributes to take precedence then we need to put @attributes=UserAttributes after all other attributes, I believe

[danielchalmers]

If we want the user's attributes to take precedence then we need to put @attributes=UserAttributes after all other attributes, I believe

Because it's a binding it still takes over @henon

[henon]

Hmm I am not sure I get that but I think we worry about it when somebody really needs this. No need going down that rabbit hole right now.

[henon]

Thanks!