Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
279 stars 76 forks source link

[Action Bar] Add `aria-expanded` to expand/collapse button #7003

Open geospatialem opened 1 year ago

geospatialem commented 1 year ago

Summary

Add context to the action-bar's expand/collapse button when it is expanded. Surfaced as part of MapViewer's a11y audit in the Spring of 2022.

Actual Behavior

Context is provided if the action can be collapsed or expanded. However, no context is provided for AT to know an expansion or collapse has taken place.

Expected Behavior

Announce if the component is expanded or collapsed to AT by adding an aria-expanded property (true/false) boolean, which provides additional context to AT.

Reproduction Sample

https://developers.arcgis.com/calcite-design-system/components/action-bar/#sample

Reproduction Steps

  1. Open a screen reader of choice (VoiceOver, NVDA, or JAWS)
  2. Open the Developer site sample
  3. Navigate via keyboard to the "Expand/Collapse" action
  4. Observe while the context is provided if the action can be collapsed or expanded, no context is provided for AT to know an expansion or collapse has taken place

Reproduction Version

1.4.0

Working W3C Example/Tutorial

No response

Relevant Info

No response

Regression?

No response

Priority impact

p4 - not time sensitive

Esri team

Calcite (dev)

driskull commented 7 months ago

@geospatialem one issue I'm seeing here is that calcite-action already has a active property which translates to aria-pressed. It seems like both aria-pressed and aria-expanded are conflicting with each other.

Should we maybe refactor this so they can't both be set? Is there some workaround for when the following happens?

<button aria-expanded="true" aria-pressed="true">
  Toggle
</button>
geospatialem commented 7 months ago

@geospatialem one issue I'm seeing here is that calcite-action already has a active property which translates to aria-pressed. It seems like both aria-pressed and aria-expanded are conflicting with each other.

Should we maybe refactor this so they can't both be set? Is there some workaround for when the following happens?

That sounds like a good plan forward.

It looks like aria-pressed is associated with the active property, does that sound right?

https://github.com/Esri/calcite-design-system/blob/f199b3cafc40438c32a12cad2aef4d26be4526f6/packages/calcite-components/src/components/action/action.tsx#L318

If so, @driskull - what would you think about removing aria-pressed, which shouldn't indicate a "selected" or "active" state, but rather adds "press" context for AT users. And instead consider a new translation to the button's aria-label indicating its active state, similar to what is added when indicator is true?

https://github.com/Esri/calcite-design-system/blob/f199b3cafc40438c32a12cad2aef4d26be4526f6/packages/calcite-components/src/components/action/action.tsx#L302

driskull commented 7 months ago

It looks like aria-pressed is associated with the active property, does that sound right?

Yes, correct.

what would you think about removing aria-pressed, which shouldn't indicate a "selected" or "active" state, but rather adds "press" context for AT users. And instead consider a new translation to the button's aria-label indicating its active state, similar to what is added when indicator is true?

Shouldn't we keep aria-pressed because some of these buttons are toggle buttons. right? active means that its "on". If we remove it, we lose that context. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-pressed

driskull commented 7 months ago

I think we'll need to setup some time to discuss a plan moving forward here.

We already have a property for toggling (active) but we don't have a property for expand status. We would need a new property to handle this because aria-expanded needs to be added to the internal button element.

geospatialem commented 7 months ago

We already have a property for toggling (active) but we don't have a property for expand status. We would need a new property to handle this because aria-expanded needs to be added to the internal button element.

Ah, gotcha, thanks for the context - was initially thinking the action component was providing a selection state, rather than leveraging aria-pressed - it's more clear now.

Concur with your original statement above- we'd want to ensure aria-pressed and aria-expanded aren't used together with the action-bar, and when the component is expanded, ensure aria-expanded takes precedence. Per your proposal above, a new property with conditions sounds like the best path forward, if you still agree?

driskull commented 7 months ago

Yes, I agree. We'll have to deprecate active and have a new property for handling toggled/expanded state.

driskull commented 7 months ago

I guess we would have to add something like:

state: "pressed" | "unpressed" | "expanded" | "collapsed";
state: "on" | "off" | "expanded" | "collapsed";
state: "active" | "inactive" | "expanded" | "collapsed";

Would we need to support the mixed state of aria-pressed?

driskull commented 7 months ago

When something is "expanded" would it get the same background color affordance as when "pressed/active"?

driskull commented 4 months ago

I guess we could just add an expanded property on action which would take precedence over active? What do you think? is that easier than a single mode?

driskull commented 2 months ago

I think this needs a design/api discussion before it can move forward.

github-actions[bot] commented 2 months ago

cc @geospatialem, @brittneytewks

geospatialem commented 2 months ago

The above is blocked and sending to the freezer 🥶 until accessibility API supports shadow DOM further for implementation. As this is not currently feasible within shadow DOM, since the component relies on an internal button via the shadow DOM.

Some resources for consideration and checking on the Accessibility object model (AOM), once supported: