fix(accordion, accordion-item): improve a11y #7560

jcfranco · 2023-08-22T00:08:37Z

Related Issue: #5553

Summary

Updated HTML to improve a11y.

jcfranco · 2023-08-22T17:30:08Z

@geospatialem @driskull Since axe doesn't catch this, would adding tests that assert on the structure be useful? On the one hand, they rely on implementation details and there's no feedback on the effect on AT. On the other, it at least adds some coverage behind the change.

driskull

question ❔

driskull · 2023-08-22T17:32:58Z

packages/calcite-components/src/components/accordion-item/accordion-item.tsx

@@ -204,7 +205,7 @@ export class AccordionItem implements ConditionalSlotComponent {
            </div>
            {this.renderActionsEnd()}
          </div>
-          <div class={CSS.content}>
+          <div aria-live="polite" class={CSS.content} id={IDS.slotContainer} role="region">


I thought we were still trying to use unique ids for each component instance? Even though its in shadowDOM.

Yeah, I think we were following this pattern since the early days because we didn't have shadow DOM enabled consistently. If AT picks this up smoothly, we can keep things simple for the only-need-unique-ID-in-the-component's-shadow-DOM case.

Hm, yeah, I'm not sure if this is an unique id issue and/or shadow DOM issue but JAWS isn't providing context with the current solution. Maybe we can try out an unique id?

@geospatialem I made some tweaks. Could you test again?

driskull · 2023-08-22T17:33:47Z

I think a basic structure test would be fine here. 👍

jcfranco · 2023-08-22T17:26:21Z

packages/calcite-components/src/components/accordion-item/resources.ts

@@ -18,3 +18,7 @@ export const CSS = {
  iconEnd: "icon--end",
  headerContainer: "header-container",
 };
+
+export const IDS = {


Sidebar: we can update our conventions on using ID if needed within shadow DOM since IDs are encapsulated.

Whoops, forgot to submit this comment earlier. 😅

jcfranco · 2023-08-22T17:42:44Z

packages/calcite-components/src/components/accordion-item/accordion-item.tsx

@@ -204,7 +205,7 @@ export class AccordionItem implements ConditionalSlotComponent {
            </div>
            {this.renderActionsEnd()}
          </div>
-          <div class={CSS.content}>
+          <div aria-live="polite" class={CSS.content} id={IDS.slotContainer} role="region">


Yeah, I think we were following this pattern since the early days because we didn't have shadow DOM enabled consistently. If AT picks this up smoothly, we can keep things simple for the only-need-unique-ID-in-the-component's-shadow-DOM case.

driskull · 2023-08-22T17:49:55Z

If AT picks this up smoothly, we can keep things simple for the only-need-unique-ID-in-the-component's-shadow-DOM case.

Sounds good. We'll just have to test AT.

driskull

👍 Looks good!

github-actions · 2023-08-31T01:43:39Z

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.

geospatialem

Good to go in both JAWS and NVDA. 🙌🏻

geospatialem · 2023-08-31T16:17:38Z

Good to go in both JAWS and NVDA. 🙌🏻

@jcfranco After looking at the failing e2e, wondering if this could be fixed with the guid utility? It looks like the current value of IDS.section is "section". While it does work with AT, it would fail the test since its not an unique id?

geospatialem

🙌🏻 LGTM

jcfranco · 2023-08-31T21:39:20Z

@geospatialem Thanks for testing!

@driskull It looks like we'll be able to use IDs unique to each shadow root after all. 🎉

driskull · 2023-08-31T21:41:36Z

Nice! 🎉

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 1.7.0</summary> ## [1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2023-09-01) ### Features * **action-bar, action-pad, action-group:** Add label properties for group context ([#7415](#7415)) ([b34f36d](b34f36d)) * **combobox:** Add single-persist selection mode ([#7583](#7583)) ([dab06a3](dab06a3)) * **flow:** Add support for custom flow-item elements ([#7608](#7608)) ([197adfe](197adfe)) * **input-date-picker:** Normalize year to current century for user typed values only ([#7638](#7638)) ([a1db718](a1db718)) * **input-number:** Add integer property ([#7646](#7646)) ([cd66a6d](cd66a6d)) * **input-time-picker:** Support fractional seconds ([#7532](#7532)) ([c2bf34b](c2bf34b)) * **meter:** Add Meter component ([#7401](#7401)) ([47163ed](47163ed)) * **sheet:** Add Sheet component ([#7561](#7561)) ([f12a393](f12a393)) * **sheet:** Update default widths ([#7617](#7617)) ([47d2c0b](47d2c0b)) * **shell:** Add "Sheets" Slot ([#7579](#7579)) ([e798765](e798765)) * **table:** Add Table and related components ([#7607](#7607)) ([b067e72](b067e72)) ### Bug Fixes * **accordion, accordion-item:** Improve a11y ([#7560](#7560)) ([b5170b6](b5170b6)) * Add drag styles for improved UX ([#7644](#7644)) ([afbb764](afbb764)) * **block, block-section:** Improve a11y ([#7557](#7557)) ([1f44f6b](1f44f6b)) * **chip-group:** Add existence checks ([#7586](#7586)) ([5ca64f1](5ca64f1)) * **color-picker:** Update value when alphaChannel is toggled ([#7563](#7563)) ([1f753dd](1f753dd)) * **combobox:** Prevent deselecting items via keyboard in single-persist mode ([#7634](#7634)) ([4f5f8b0](4f5f8b0)) * **combobox:** Update combobox height to follow design spec ([#7558](#7558)) ([ec08845](ec08845)) * **date-picker:** Set start of week to monday in zh-CN ([#7578](#7578)) ([7e385cb](7e385cb)) * **dropdown:** Prevents navigating dropdown items with Tab key ([#7527](#7527)) ([3ea658d](3ea658d)) * Ensure label only focuses the first labelable child ([#7553](#7553)) ([426159c](426159c)) * **flow:** Catch error when beforeBack promise is rejected ([#7601](#7601)) ([cb70671](cb70671)) * **input-date-picker, input-time-picker:** Do not show dropdown affordance when read-only ([#7559](#7559)) ([5a3f19c](5a3f19c)) * **input, input-number:** Correctly sanitize numbers when pasting string with 'e' ([#7648](#7648)) ([b8bc11c](b8bc11c)) * **list, sortable-list, value-list:** Emit calciteListOrderChange when dragging between lists ([#7614](#7614)) ([4653581](4653581)) * **list:** Fixes dragging nested list items ([#7555](#7555)) ([c25f7b3](c25f7b3)) * **list:** Stop emitting calciteListChange when a list-item is disabled or closed. ([#7624](#7624)) ([7008463](7008463)) * **loader:** Tweak loading animations to work in Safari ([#7564](#7564)) ([2103654](2103654)) * **modal:** Catch error when beforeClose promise is rejected ([#7600](#7600)) ([70405d0](70405d0)) * **modal:** Handle removal of open attribute and prevent multiple beforeClose calls ([#7470](#7470)) ([f31588f](f31588f)) * **rating:** Adds focus outline on click ([#7341](#7341)) ([af30073](af30073)) * **segmented-control:** Refresh items when added dynamically ([#7567](#7567)) ([2e36eb3](2e36eb3)) * **split-button:** Update divider and borders to follow design spec ([#7568](#7568)) ([8df59ab](8df59ab)) * **tree-item:** Move focus outline to item label area ([#7581](#7581)) ([1327cef](1327cef)) * **tree-item:** Updates state when selection changes programmatically for `selection-mode='ancestors'` ([#7572](#7572)) ([40758c5](40758c5)) * **tree:** Improve keyboard navigation ([#7618](#7618)) ([826a5cb](826a5cb)) </details> <details><summary>@esri/calcite-components-react: 1.7.0</summary> ## [1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2023-09-01) ### Bug Fixes * Make sure components are defined in environments like in codesandbox ([#7632](#7632)) ([7005cce](7005cce)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^1.7.0-next.22 to ^1.7.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>

* origin/main: (35 commits) ci: make sure to exit on maitenance milestone failure (#7656) chore: release next fix(block): provide textual name on collapse and expansion to AT (#7652) chore: release main (#7571) chore: release next fix(block, block-section): improve a11y (#7557) chore: release next fix: add drag styles for improved UX (#7644) fix(input, input-number): correctly sanitize numbers when pasting string with 'e' (#7648) chore: release next feat(flow): add support for custom flow-item elements (#7608) chore: release next fix(list, sortable-list, value-list): Emit calciteListOrderChange when dragging between lists (#7614) feat(input-number): add integer property (#7646) chore: release next fix(accordion, accordion-item): improve a11y (#7560) refactor(stepper, stepper-item): `getElementProp` is refactored out in favor of inheritable props set directly on parent (#7593) docs(contributing): update the commit message format example URL (#7641) chore: release next feat(input-date-picker): normalize year to current century for user typed values only (#7638) ...

fix(accordion, accordion-item): improve a11y

7bb3c22

github-actions bot assigned jcfranco Aug 22, 2023

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

jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 22, 2023

jcfranco marked this pull request as ready for review August 22, 2023 17:26

jcfranco requested a review from a team as a code owner August 22, 2023 17:26

driskull reviewed Aug 22, 2023

View reviewed changes

jcfranco commented Aug 22, 2023

View reviewed changes

add test

2eb3611

jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 23, 2023

driskull approved these changes Aug 23, 2023

View reviewed changes

tweak following accordion pattern

36c324c

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

geospatialem self-requested a review August 31, 2023 16:07

geospatialem approved these changes Aug 31, 2023

View reviewed changes

fix a11y tests

060b69a

jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. Stale Issues or pull requests that have not had recent activity. labels Aug 31, 2023

fix id refs

473c94f

geospatialem self-requested a review August 31, 2023 21:34

geospatialem approved these changes Aug 31, 2023

View reviewed changes

jcfranco removed the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 31, 2023

jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 31, 2023

jcfranco merged commit b5170b6 into main Aug 31, 2023

jcfranco deleted the jcfranco/5553-improve-accordion-and-accordion-item-a11y branch August 31, 2023 21:45

github-actions bot added this to the 2023 August Priorities milestone Aug 31, 2023

calcite-admin mentioned this pull request Aug 31, 2023

chore: release main #7571

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(accordion, accordion-item): improve a11y #7560

fix(accordion, accordion-item): improve a11y #7560

jcfranco commented Aug 22, 2023 •

edited

Loading

jcfranco commented Aug 22, 2023

driskull left a comment

driskull Aug 22, 2023

jcfranco Aug 22, 2023

geospatialem Aug 23, 2023

jcfranco Aug 31, 2023

driskull commented Aug 22, 2023

jcfranco Aug 22, 2023

jcfranco Aug 22, 2023 •

edited

Loading

jcfranco Aug 22, 2023

driskull commented Aug 22, 2023

driskull left a comment

github-actions bot commented Aug 31, 2023

geospatialem left a comment

geospatialem commented Aug 31, 2023

geospatialem left a comment

jcfranco commented Aug 31, 2023

driskull commented Aug 31, 2023

fix(accordion, accordion-item): improve a11y #7560

fix(accordion, accordion-item): improve a11y #7560

Conversation

jcfranco commented Aug 22, 2023 • edited Loading

Summary

jcfranco commented Aug 22, 2023

driskull left a comment

Choose a reason for hiding this comment

driskull Aug 22, 2023

Choose a reason for hiding this comment

jcfranco Aug 22, 2023

Choose a reason for hiding this comment

geospatialem Aug 23, 2023

Choose a reason for hiding this comment

jcfranco Aug 31, 2023

Choose a reason for hiding this comment

driskull commented Aug 22, 2023

jcfranco Aug 22, 2023

Choose a reason for hiding this comment

jcfranco Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

jcfranco Aug 22, 2023

Choose a reason for hiding this comment

driskull commented Aug 22, 2023

driskull left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 31, 2023

geospatialem left a comment

Choose a reason for hiding this comment

geospatialem commented Aug 31, 2023

geospatialem left a comment

Choose a reason for hiding this comment

jcfranco commented Aug 31, 2023

driskull commented Aug 31, 2023

jcfranco commented Aug 22, 2023 •

edited

Loading

jcfranco Aug 22, 2023 •

edited

Loading