fix(button): provides context for AT users when used as reference element for collapsible content #7658

anveshmekala · 2023-09-01T20:28:54Z

Related Issue: #6033

Summary

Provides context of expanded for AT users when calcite-button is used as reference element for collapsible content.

…ment for collapsible content

…ed-context

driskull

👍

I wonder if this deserves its own expanded prop? Otherwise users would never know about setting aria-expanded and having it work as expected.

anveshmekala · 2023-09-15T00:59:58Z

I wonder if this deserves its own expanded prop? Otherwise users would never know about setting aria-expanded and having it work as expected.

Using prop seems like a good option. we can explore having an prop which needs to be set by the user when they use button as trigger for any floating/expandable UI. Defining this helps a lot of components which can be used as trigger elements. (ex: fab , action) . It will be additional work for user but they will have the benifit of consistent screen reader experience.

Thoughts @jcfranco

jcfranco · 2023-09-19T18:43:21Z

For the time being, I think we should skip the prop for a few reasons:

API concerns
- we already have expanded in some cases where this might fit nicely, but it doesn't translate as well w/ open, which is the preferred prop name.
- we would have to add matching props for other, if not all, attributes.
This follows existing a11y practices.
At this point, this is mostly needed for triggers (reference elements). We could revisit this later as components are used in more a11y patterns.

jcfranco

🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸
🎸🤘🎸🎸🎸🤘🎸🤘🎸🎸🎸🤘🎸🎸🤘🤘🤘🎸🤘🎸
🎸🎸🤘🎸🤘🎸🎸🤘🎸🎸🎸🤘🎸🤘🎸🎸🎸🎸🤘🎸
🎸🎸🎸🤘🎸🎸🎸🤘🎸🎸🎸🤘🎸🎸🤘🤘🎸🎸🤘🎸
🎸🎸🎸🤘🎸🎸🎸🤘🎸🎸🎸🤘🎸🎸🎸🎸🤘🎸🎸🎸
🎸🎸🎸🤘🎸🎸🎸🎸🤘🤘🤘🎸🎸🤘🤘🤘🎸🎸🤘🎸
🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸🎸

jcfranco · 2023-09-19T18:46:28Z

packages/calcite-components/src/components/button/button.e2e.ts

@@ -644,4 +644,27 @@ describe("calcite-button", () => {
    expect(button1.textContent.length).toBeLessThan(longText.length);
    expect(button1.getAttribute("title")).toEqual(longText);
  });
+
+  it("should set aria-expanded attribute on shadowDOM element when used as trigger", async () => {


Could you test if the accessible helper picks this up (also checking that it fails when the ARIA attribute isn't set on the internal button)? If so, I'd suggest using that to simplify this test.

accessible helper is unable to pick this one up.

jcfranco · 2023-09-19T18:54:06Z

packages/calcite-components/src/utils/globalAttributes.ts

@@ -1,8 +1,8 @@
 import { createObserver } from "./observers";

 type AttributeObject = { [k: string]: any };
-type AllowedGlobalAttribute = "lang" | "role";
-const allowedGlobalAttributes = ["lang", "role"];
+type AllowedGlobalAttribute = "lang" | "role" | "aria-expanded";


Popover also sets aria-controls, so does that also need to be set here? Not sure if we need aria-describedby to support tooltip. cc @geospatialem

IMO, aria-controls need not be parsed on to the native button for the following reasons:

It has minimal support across screen readers in terms of providing context as per https://a11ysupport.io/tech/aria/aria-controls_attribute.

For scenarios where screen reader should jump directly onto the opened element, the popover is currently doing that with or without aria-controls.

Agree that there isn't widespread coverage for aria-controls. However with the fix, JAWS doesn't provide context in all cases.

For instance, in the original use case where a popover is provided with checkboxes JAWS will only provide context that the button is not expanded when a user changes the checked state prior to pressing the escape key. Maybe the addition would help support JAWS in all cases?

In VoiceOver, context is provided when user check/uncheck the checkbox before closing the popup.

Windows OS + JAWS updates this week mitigated the issue from last week. 💪🏻

geospatialem

This will be a fantastic fix! 🌟

Observed the button doesn't appear to be selectable/clickable via the Enter key when testing with NVDA and Firefox, but not sure what could be causing the issue.

Here's the code tested with (taken from the original issue Codepen):

<calcite-label layout="inline"
  ><span>Age groups:</span>
  <calcite-button id="test-category-filter-button" icon-end="chevron-down" width="half" label="Any age groups">
    Any age groups
  </calcite-button>
  <calcite-popover
    label="Select types"
    reference-element="test-category-filter-button"
    auto-close
    placement="bottom"
    id="category-filter-popover"
  >
    <p style="padding: 12px; margin: 0">
      <calcite-label layout="inline">
        <calcite-checkbox value="infant"></calcite-checkbox>
        infant
      </calcite-label>
      <calcite-label layout="inline">
        <calcite-checkbox value="toddler"></calcite-checkbox>
        toddler
      </calcite-label>
      <calcite-label layout="inline">
        <calcite-checkbox value="preschool"></calcite-checkbox>
        preschool
      </calcite-label>
      <calcite-label layout="inline">
        <calcite-checkbox value="school age"></calcite-checkbox>
        school age
      </calcite-label>
    </p>
  </calcite-popover>
</calcite-label>

github-actions · 2023-09-28T01:43:56Z

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

anveshmekala · 2023-09-29T20:11:36Z

This will be a fantastic fix! 🌟

Observed the button doesn't appear to be selectable/clickable via the Enter key when testing with NVDA and Firefox, but not sure what could be causing the issue.

Is it possible that NVDA is in browse mode? Browse mode can intercept certain keys from working as expected.

geospatialem · 2023-09-29T20:56:43Z

Observed the button doesn't appear to be selectable/clickable via the Enter key when testing with NVDA and Firefox, but not sure what could be causing the issue.

Is it possible that NVDA is in browse mode? Browse mode can intercept certain keys from working as expected.

That was the trick! ✨ Only occurs in Firefox, and was discovered as a similar report (which was closed) in nvaccess/nvda#4903. More info: https://dequeuniversity.com/screenreaders/nvda-keyboard-shortcuts#nvda-browse_focus_modes

geospatialem

LGTM 🚀

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 1.9.0</summary> ## [1.9.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@1.8.0...@esri/calcite-components@1.9.0) (2023-10-03) ### Features * **action-group:** Add css custom properties to define gap and padding when layout is "grid" ([#7763](#7763)) ([b9bd0de](b9bd0de)) * **action-menu:** Add appearance property to configure trigger appearance ([#7867](#7867)) ([36ceaf1](36ceaf1)) * **alert:** Make component responsive ([#7755](#7755)) ([66222b5](66222b5)) * **block, input-time-picker, notice:** Adds `open`, `close`, `beforeOpen`, and `beforeClose` events for consistent event handling ([#7494](#7494)) ([04ce758](04ce758)) * **checkbox:** Add WCAG AA recommended minimum 24px square hotspot optimized for touch users ([#7773](#7773)) ([f13e2c4](f13e2c4)) * **flow, flow-item:** Allow calciteFlowItemBack event to be cancelled ([#7855](#7855)) ([b74b4df](b74b4df)) * **input-date-picker, input-time-picker:** Add status property ([#7915](#7915)) ([4d53346](4d53346)) * **input-time-zone:** Add max-items support ([#7705](#7705)) ([c698c27](c698c27)) * **input-time-zone:** Add time zone offset and name mode ([#7913](#7913)) ([80bd6ae](80bd6ae)) * **list:** Add newIndex and oldIndex event details to calciteListOrderChange event ([#7874](#7874)) ([0d5cc20](0d5cc20)) * **pagination:** Enable responsive layout ([#7722](#7722)) ([933a910](933a910)) * **panel, flow-item:** Add support for collapsing content ([#7857](#7857)) ([855754d](855754d)) ### Bug Fixes * **action-bar:** Improve overflowing horizontal actions. ([#7877](#7877)) ([52f2d2a](52f2d2a)) * **action-bar:** Overflow actions when overflowActionsDisabled is set to false ([#7873](#7873)) ([3dcceb0](3dcceb0)) * **action-menu:** Update active selection to not use the action's active property ([#7911](#7911)) ([50f85f1](50f85f1)) * **alert:** Regression fix to restore `calciteAlertBeforeOpen` and `calciteAlertOpen` event emitting ([#7767](#7767)) ([6bbae35](6bbae35)) * **button:** Provides context for AT users when used as reference element for collapsible content ([#7658](#7658)) ([50cb3a6](50cb3a6)) * **combobox:** Clears input value on blur ([#7721](#7721)) ([e04cc4e](e04cc4e)) * **combobox:** Fix filtering to avoid showing unrelated items ([#7704](#7704)) ([b6d32f3](b6d32f3)) * **dropdown-group:** Set selectionMode on slotted dropdown-item children ([#7897](#7897)) ([aa0731a](aa0731a)) * **dropdown:** Support dispatching enter key event on dropdown trigger ([#7752](#7752)) ([ba92463](ba92463)) * **select:** Allow setting an option value to an empty string ([#7769](#7769)) ([adca6ec](adca6ec)) * **stepper:** Improves AT Users experience with screen readers ([#7691](#7691)) ([071dec7](071dec7)) * **tab-nav:** Update indicator position when tab icon changes ([#7768](#7768)) ([cb877b3](cb877b3)) * **table:** Fix selection display in RTL ([#7907](#7907)) ([b4c8508](b4c8508)) * **tree:** Avoid modifying selected items for "none" selection-mode ([#7904](#7904)) ([0bd4a12](0bd4a12)) * **tree:** Fix tree keyboard selection issue ([#7908](#7908)) ([53d6c12](53d6c12)) * **tree:** Update tree selection per design spec ([#7852](#7852)) ([06b3594](06b3594)) </details> <details><summary>@esri/calcite-components-react: 1.9.0</summary> ## [1.9.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@1.8.0...@esri/calcite-components-react@1.9.0) (2023-10-03) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize undefined versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^1.9.0-next.18 to ^1.9.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

fix(button): provides context for AT users when used as reference ele…

e4ad8fc

…ment for collapsible content

github-actions bot assigned anveshmekala Sep 1, 2023

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Sep 1, 2023

anveshmekala added 6 commits September 1, 2023 18:12

revert

00c5137

update global attributes in shadowDOM

f9ba23f

Merge branch 'main' into anveshmekala/6033-fix-button-provided-expand…

2714056

…ed-context

Merge branch 'main' into anveshmekala/6033-fix-button-provided-expand…

0d13e98

…ed-context

add tests

e291d46

simplify test

192571f

anveshmekala marked this pull request as ready for review September 13, 2023 23:00

anveshmekala requested a review from a team as a code owner September 13, 2023 23:00

driskull approved these changes Sep 13, 2023

View reviewed changes

anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Sep 18, 2023

jcfranco approved these changes Sep 19, 2023

View reviewed changes

geospatialem reviewed Sep 20, 2023

View reviewed changes

github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Sep 28, 2023

geospatialem self-requested a review September 29, 2023 20:57

geospatialem approved these changes Sep 29, 2023

View reviewed changes

anveshmekala merged commit 50cb3a6 into main Sep 29, 2023
14 checks passed

anveshmekala deleted the anveshmekala/6033-fix-button-provided-expanded-context branch September 29, 2023 21:00

github-actions bot added this to the 2023 September Priorities - Main release milestone Sep 29, 2023

calcite-admin mentioned this pull request Sep 29, 2023

chore: release main #7718

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(button): provides context for AT users when used as reference element for collapsible content #7658

fix(button): provides context for AT users when used as reference element for collapsible content #7658

anveshmekala commented Sep 1, 2023 •

edited

Loading

driskull left a comment

anveshmekala commented Sep 15, 2023

jcfranco commented Sep 19, 2023

jcfranco left a comment

jcfranco Sep 19, 2023

anveshmekala Sep 19, 2023

jcfranco Sep 19, 2023

anveshmekala Sep 19, 2023 •

edited

Loading

geospatialem Sep 20, 2023

anveshmekala Sep 29, 2023

geospatialem Sep 29, 2023

geospatialem left a comment

github-actions bot commented Sep 28, 2023

anveshmekala commented Sep 29, 2023

geospatialem commented Sep 29, 2023

geospatialem left a comment

fix(button): provides context for AT users when used as reference element for collapsible content #7658

fix(button): provides context for AT users when used as reference element for collapsible content #7658

Conversation

anveshmekala commented Sep 1, 2023 • edited Loading

Summary

driskull left a comment

Choose a reason for hiding this comment

anveshmekala commented Sep 15, 2023

jcfranco commented Sep 19, 2023

jcfranco left a comment

Choose a reason for hiding this comment

jcfranco Sep 19, 2023

Choose a reason for hiding this comment

anveshmekala Sep 19, 2023

Choose a reason for hiding this comment

jcfranco Sep 19, 2023

Choose a reason for hiding this comment

anveshmekala Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

geospatialem Sep 20, 2023

Choose a reason for hiding this comment

anveshmekala Sep 29, 2023

Choose a reason for hiding this comment

geospatialem Sep 29, 2023

Choose a reason for hiding this comment

geospatialem left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 28, 2023

anveshmekala commented Sep 29, 2023

geospatialem commented Sep 29, 2023

geospatialem left a comment

Choose a reason for hiding this comment

anveshmekala commented Sep 1, 2023 •

edited

Loading

anveshmekala Sep 19, 2023 •

edited

Loading