Esri / calcite-design-system

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

calcite-button's aria-label being set to entire contents of calcite-popover #5777

Open phenderson00 opened 2 years ago

phenderson00 commented 2 years ago

Actual Behavior

We are using a calcite-popover with a calcite-button as its reference element. When the button gets focus, the entire contents of the popover is read before the popover has been opened. I can see that the button in the shadow root has the entire popover contents in its aria-label.

Expected Behavior

When the calcite-button gets focus, only the button text should be read by the screen reader. Once the button is clicked and the calcite-popover that it is the reference element for is opened, then the popover should be read.

Reproduction Sample

https://codesandbox.io/s/calcite-button-screenreader-issue-3tcvow?file=/src/App.js

Reproduction Steps

  1. Open the DevTools console
  2. Activate the element selector (Ctrl + Shift + C OR box with cursor icon in top left of DevTools console)
  3. Click on the calcite-button that reads 'Any age groups'
  4. In the Elements tab of the DevTools console, expand the calcite-button element and the shadow-root if not already expanded
  5. See that the derived button element has all of the calcite-popover content in its aria-label

Reproduction Version

0.35.0

Relevant Info

Windows 10, Chrome 107.0.5304.107

Regression?

No response

Impact

@driskull suggested a workaround (setting the label property on the calcite-button) that gets us past this issue, so it is not blocking any work.

Esri team

Professional Services - Midwest Delivery Center

geospatialem commented 1 year ago

Research if repo outside of React to determine next steps.

geospatialem commented 1 year ago

Can repro outside of React with https://codepen.io/geospatialem/pen/ExeOZBO.

Workaround (non-framework example) adding the label prop to the calcite-button mentioned above: https://codepen.io/geospatialem/pen/gOdQMPN

For a potential fix...

🌟 1. Could be related to the utils/label.ts getLabelText function if we could add in context from the component's innerText when label isn't defined (possibly useful to other components which have a non-required label prop), 🌟 recommended approach, reference Matt's note below.

  1. Could set the component's label default to the innerText, or
  2. Could require the label prop on the component
driskull commented 1 year ago

I think we should probably adjust the label.ts function to no longer get the whole textContent.

https://github.com/Esri/calcite-components/blob/bcf349edb79c03271357912da84aca5ab3396cec/src/utils/label.ts#L144-L146