chore[actionbutton, actiongroup]: fix application of aria-labels #3230

marissahuysentruyt · 2024-10-10T18:02:49Z

Description

All action buttons were, by default, receiving the aria-label attribute. When clickable elements have visible text, typically they should not receive an aria-label because a screen reader will read the visible text aloud. The button component was applying aria-labels conditionally (when a button didn't have visible text/label), so the same approach was brought to action button. Now, any action button that has "regular" label text does not receive the aria-label, and if an action button is icon-only, with its hideLabel: true arg set, an aria-label will be applied.

Because action buttons are used in the action group component as well, some refactoring had to be done to make sure any arguments used to create icon-only action buttons were from the action group args, as opposed to the action button args. For instance, instead of the action buttons' label arg getting set to empty string to render an icon-only action button within an action group like so:

ActionGroup.args = {
        content: [
		{
			iconName: "AlignTop",
			label: "",
		},
		{
			iconName: "AlignBottom",
			label: "",
		},
		{
			iconName: "AlignMiddle",
			label: "",
		},
	]
}

...the code for action group now relies on its own iconOnly control to hide the action buttons' labels.

ActionGroup.args = {
	iconOnly: true,
	content: [
		{
			iconName: "AlignTop",
			label: "Align top",
		},
		{
			iconName: "AlignBottom",
			label: "Align bottom",
		},
		{
			iconName: "AlignMiddle",
			label: "Align middle",
		},
	]
}

This should ensure that the label for each action button within content gets passed appropriately to the action button component, and set as the value for its aria-label.

White space/empty lines in the story files were removed during this work, as well as some corrections made to the sentence-case of our story names on the documentation pages.

Jira/Specs

CSS-853
Github issue

Pending Questions

Should we go through each instance of ActionButton to make sure we're using label and hideLabel correctly? For instance, the breadcrumbs action button is icon-only, and has no label text or aria-label. Is that separate work from this PR? With just a quick search, card, contextual help, pagination may be other components to look into.

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

Pull down the branch to run locally or visit the deploy preview.
Inspect the action button docs page in the browser.
Verify that any of the action buttons that have visible label text do not have an aria-label attribute.
Verify that any of the icon-only buttons (screenshot below) have an aria-label attribute that corresponds to the label text set in the controls. By default, you should see aria-label="Edit" for the icon-only buttons.

Visit the story page and change the label text via the "Label" control. Verify that your new label is set as the aria-label for the icon-only action buttons.

(I used "paradiddle" 🥁)

Using a screen reader (like NVDA on AssistivLabs or VoiceOver with Safari), use the keyboard to tab focus onto the action buttons.
Any icon-only action buttons show have their aria-label announced by the screenreader. You should hear something like "Edit, toggle button." You should also hear "submenu" for the action buttons with the "hold icon."
Next visit the action group docs page
In the inspector, verify that the "Default" action group has no aria-labels. The action button labels should be within <span>s.

Verify that the "Icon-only" and "Quiet, icon-only" have aria-labels set to their label text. You should see "Edit," "Copy," and "Delete."

Feel free to inspect a few more of the action buttons within the action group stories. The justified stories have different label text to correspond better to their icons.
Additionally, you can inspect the action group within the action bar component. Those buttons should no longer have the aria-label attribute since they have visible text.

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

If components have been modified, VRTs have been run on this branch:

VRTs have been run and looked at.
Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
If my change impacts other components, I have tested to make sure they don't break.
✨ This pull request is ready to merge. ✨

changeset-bot · 2024-10-10T18:02:53Z

⚠️ No Changeset found

Latest commit: e3beebc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2024-10-10T18:06:25Z

🚀 Deployed on https://pr-3230--spectrum-css.netlify.app

github-actions · 2024-10-10T18:14:32Z

File metrics

Summary

Total size: 4.31 MB^*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

marissahuysentruyt · 2024-10-10T19:09:40Z

components/actionbutton/stories/template.js

@@ -69,7 +69,7 @@ export const Template = ({
 	const { updateArgs } = context;
 	return html`
 		<button
-			aria-label=${ifDefined(label)}
+			aria-label=${ifDefined(hideLabel ? label : undefined)}


This is the same way Button handles the aria-labels

Is this preferable to wrapping the label and value in the condition and omitting altogether if the condition is false?

It is - this is the way lit-html manages attributes

marissahuysentruyt · 2024-10-10T19:11:41Z

components/actiongroup/stories/actiongroup.stories.js

+	iconOnly: true,
 	content: [
 		{
 			iconName: "AlignTop",
-			label: "",
+			label: "Align top",


Instead of creating icon-only buttons by leaving label as an empty string, I wanted to utilize the iconOnly control for action group. I also think having the label for each action button here better aligns with the guidance that "action buttons should always have a label."

marissahuysentruyt · 2024-10-10T19:13:37Z

components/actiongroup/stories/template.js

@@ -129,59 +130,24 @@ export const OverflowOption = (context) => Container({
 	`
 }, context );

-export const ActionButtonOptions = (args, context ) => Container({


Correct me if I'm wrong, but I think this was basically recreating the Template with all of the same arg values as the default args. I ended up removing this template, in favor of just using the template itself, with the default args, in TreatmentTemplate.

marissahuysentruyt · 2024-10-10T19:14:43Z

components/actiongroup/stories/template.js

+		{ heading: "Default", },
+		{ iconOnly: true, heading: "Icon-only", },
+		{ iconOnly: true, areQuiet: true, heading: "Quiet, icon-only", },
+		].map(({ heading, areQuiet, iconOnly }) => Container({
+			withBorder: false,
+			heading: heading,
+			content: Template({
+				...args,
+				areQuiet, 
+				iconOnly,
+			}, context)}


Similar to the comment I made in the stories file- I opted for using the iconOnly arg for action group, instead of hideLabel from action button.

marissahuysentruyt · 2024-10-10T19:35:17Z

components/actiongroup/stories/template.js

+						label: "Delete",
+					},
+					{
+						iconName: "More",


In my screenreader testing, this button was just announced as "toggle button." Do we have a specific label for a button like this, that probably triggers a popover/menu?

marissahuysentruyt · 2024-10-10T19:36:32Z

components/actiongroup/stories/template.js

-				})
-			}
-		)}
+			withBorder: false,


These changes just fixed up some indentation, removed the border from around each container, and I think passed context to the templates, instead of the parent Container()

cdransf · 2024-10-10T22:00:39Z

components/actionbutton/stories/template.js

@@ -69,7 +69,7 @@ export const Template = ({
 	const { updateArgs } = context;
 	return html`
 		<button
-			aria-label=${ifDefined(label)}
+			aria-label=${ifDefined(hideLabel ? label : undefined)}


Is this preferable to wrapping the label and value in the condition and omitting altogether if the condition is false?

marissahuysentruyt · 2024-10-11T14:18:56Z

@cdransf Because the button behavior was called out in the issue, I went straight to the button template to see how it was working there.

castastrophe · 2024-10-11T17:21:59Z

components/actionbutton/stories/template.js

@@ -69,7 +69,7 @@ export const Template = ({
 	const { updateArgs } = context;
 	return html`
 		<button
-			aria-label=${ifDefined(label)}
+			aria-label=${ifDefined(hideLabel ? label : undefined)}


It is - this is the way lit-html manages attributes

components/actiongroup/stories/actiongroup.stories.js

- ensures that actiongroup arg types are used in some of the documentation template instead of actionbutton args - removes white space from stories file - corrects some sentence-case instances for story names

marissahuysentruyt added the a11y Questions or topics for the accessibility team label Oct 10, 2024

marissahuysentruyt added the size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. label Oct 10, 2024

marissahuysentruyt marked this pull request as ready for review October 10, 2024 19:09

marissahuysentruyt commented Oct 10, 2024

View reviewed changes

marissahuysentruyt added the skip_vrt Add to a PR to skip running VRT (but still pass the action) label Oct 10, 2024

marissahuysentruyt requested review from jawinn, aramos-adobe, pfulton, cdransf, castastrophe and rise-erpelding October 10, 2024 21:03

marissahuysentruyt self-assigned this Oct 10, 2024

cdransf approved these changes Oct 10, 2024

View reviewed changes

castastrophe approved these changes Oct 11, 2024

View reviewed changes

marissahuysentruyt requested review from castastrophe and 5t3ph October 11, 2024 19:31

marissahuysentruyt added 3 commits October 14, 2024 10:03

chore: aria-label only appears on icon-only button

ecb0616

chore: refactors action group to use different args

96d1920

- ensures that actiongroup arg types are used in some of the documentation template instead of actionbutton args - removes white space from stories file - corrects some sentence-case instances for story names

chore: fix context passing to template and indentation

e3beebc

marissahuysentruyt force-pushed the marissahuysentruyt/css-853-accessible-names-action-button branch from 9a4f993 to e3beebc Compare October 14, 2024 14:03

marissahuysentruyt merged commit f2d8c40 into main Oct 14, 2024
12 checks passed

marissahuysentruyt deleted the marissahuysentruyt/css-853-accessible-names-action-button branch October 14, 2024 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore[actionbutton, actiongroup]: fix application of aria-labels #3230

chore[actionbutton, actiongroup]: fix application of aria-labels #3230

marissahuysentruyt commented Oct 10, 2024 •

edited

Loading

changeset-bot bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

marissahuysentruyt Oct 10, 2024 •

edited

Loading

cdransf Oct 10, 2024

castastrophe Oct 11, 2024

marissahuysentruyt Oct 10, 2024

marissahuysentruyt Oct 10, 2024 •

edited

Loading

marissahuysentruyt Oct 10, 2024

marissahuysentruyt Oct 10, 2024

marissahuysentruyt Oct 10, 2024

cdransf Oct 10, 2024

marissahuysentruyt commented Oct 11, 2024

castastrophe Oct 11, 2024

chore[actionbutton, actiongroup]: fix application of aria-labels #3230

chore[actionbutton, actiongroup]: fix application of aria-labels #3230

Conversation

marissahuysentruyt commented Oct 10, 2024 • edited Loading

Description

Jira/Specs

Pending Questions

How and where has this been tested?

Validation steps

Regression testing

Screenshots

To-do list

changeset-bot bot commented Oct 10, 2024 • edited Loading

⚠️ No Changeset found

github-actions bot commented Oct 10, 2024 • edited Loading

github-actions bot commented Oct 10, 2024 • edited Loading

File metrics

Summary

marissahuysentruyt Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marissahuysentruyt Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marissahuysentruyt commented Oct 11, 2024

Choose a reason for hiding this comment

marissahuysentruyt commented Oct 10, 2024 •

edited

Loading

changeset-bot bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

marissahuysentruyt Oct 10, 2024 •

edited

Loading

marissahuysentruyt Oct 10, 2024 •

edited

Loading