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

[Bug][a11y]: ActionGroup should have role="toolbar" as default / selected button should use aria-pressed #3221

Closed
1 task done
majornista opened this issue May 19, 2023 · 1 comment · Fixed by #3227
Closed
1 task done
Labels
a11y Issues related to accessibility bug Something isn't working triage An issue needing triage

Comments

@majornista
Copy link
Contributor

majornista commented May 19, 2023

Code of conduct

  • I agree to follow this project's code of conduct.

Impacted component(s)

ActionGroup

Expected behavior

Since ActionGroup uses the RovingTabindexController to manage focus using arrow keys to navigate between items, we should add role="toolbar" when no other role or selection mode is specified, so that screen readers will switch into forms mode and keyboard navigation will behave as expected.

Also, when selects is undefined, selected buttons should use aria-pressed rather than aria-checked.

Actual behavior

ActionGroup does not use role="toolbar" when no other role or selection mode is specified, so arrow keys will continue to navigate using a screen reader's browse/virtual cursor mode rather than move focus as they should for a toolbar.

Also, selected items with role="button" use aria-checked, when they should use aria-pressed:

See:

default:
// if user defines .selected
if (this.selected.length) {
const selections: ActionButton[] = [];
const updates = options.map(async (option) => {
await option.updateComplete;
option.setAttribute(
'aria-checked',
option.selected ? 'true' : 'false'
);
option.setAttribute('role', 'button');
if (option.selected) {
selections.push(option);
}
});
if (applied) break;
await Promise.all(updates);
this.setSelected(
selections.map((button) => {
return button.value;
})
);
} else {
this.buttons.forEach((option) => {
option.setAttribute('role', 'button');
});
this.removeAttribute('role');
break;
}
.

Screenshots

No response

What browsers are you seeing the problem in?

No response

How can we reproduce this issue?

  1. Go to '...'
  2. Click on '....'
  3. Scroll to '....'
  4. Check console
  5. See error

Sample code that illustrates the problem

See:

default:
// if user defines .selected
if (this.selected.length) {
const selections: ActionButton[] = [];
const updates = options.map(async (option) => {
await option.updateComplete;
option.setAttribute(
'aria-checked',
option.selected ? 'true' : 'false'
);
option.setAttribute('role', 'button');
if (option.selected) {
selections.push(option);
}
});
if (applied) break;
await Promise.all(updates);
this.setSelected(
selections.map((button) => {
return button.value;
})
);
} else {
this.buttons.forEach((option) => {
option.setAttribute('role', 'button');
});
this.removeAttribute('role');
break;
}

Logs taken while reproducing problem

No response

@majornista majornista added bug Something isn't working triage An issue needing triage labels May 19, 2023
majornista added a commit to majornista/spectrum-web-components that referenced this issue May 22, 2023
…oolbar" as default

Selected button without selects="single" or selected="multiple" should use aria-pressed.
majornista added a commit to majornista/spectrum-web-components that referenced this issue May 23, 2023
majornista added a commit to majornista/spectrum-web-components that referenced this issue May 24, 2023
Westbrook pushed a commit to majornista/spectrum-web-components that referenced this issue May 30, 2023
…oolbar" as default

Selected button without selects="single" or selected="multiple" should use aria-pressed.
Westbrook pushed a commit to majornista/spectrum-web-components that referenced this issue May 30, 2023
Westbrook pushed a commit to majornista/spectrum-web-components that referenced this issue May 30, 2023
majornista added a commit to majornista/spectrum-web-components that referenced this issue May 30, 2023
Westbrook pushed a commit that referenced this issue May 30, 2023
…nd buttons

* fix(action-group): should have role="toolbar" as default, selected button without selects="single" or selected="multiple" should use aria-pressed

* docs(action-group): #3221 [a11y]: ActionGroup add accessibility example to docs

* docs(action-group): #3221 [a11y]: ActionGroup update docs example

* docs(action-group): #3221 Update README.md wording to clarify the developer responsibility
@Westbrook Westbrook added the a11y Issues related to accessibility label Jun 2, 2023
@najikahalsema
Copy link
Collaborator

completed via #3227

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Issues related to accessibility bug Something isn't working triage An issue needing triage
Projects
None yet
3 participants