-
Notifications
You must be signed in to change notification settings - Fork 77
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
fix(block): add accessible label for slotted controls #9502
Conversation
@geospatialem could you help test this out? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like JAWS only seems to be supported with aria-labelledby
in this case, but NVDA does not provide support.
In some initial sleuthing, it seems like there may be a support issue with NVDA and the shadow DOM with aria-
named attributes similar to what is outline in this post.
I'm not sure the best path forward, as it seems browsers and AT is still catching up to support with shadow DOM. Maybe we just account for JAWS here, and mention NVDA is out of scope? Otherwise there are some hacky loop holes, maybe using a live region? WDYT?
- Use an ARIA live region instead of IDREFs. (This is the same technique used by canvas-based applications, such as Google Docs.) This option is pretty heavy-handed, but I suppose you could use it as a last resort.
Yeah we might want to mention NVDA is out of scope until it catches up with shadow dom. The next best option would be aria-live but I don't think thats a great solution. |
Co-authored-by: Kitty Hurley <khurley@esri.com>
@@ -370,7 +370,7 @@ export class Block | |||
headerContent | |||
)} | |||
{hasControl ? ( | |||
<div class={CSS.controlContainer}> | |||
<div aria-labelledby={IDS.header} class={CSS.controlContainer}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the control actually labeled by the entire header? When I run this through VO it works but UX wise I'm not 100% sure it makes sense. The entire header includes icons, heading, and description. That's potentially a lot of content to label a control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its just the title and icon.
calcite-design-system/packages/calcite-components/src/components/block/block.tsx
Lines 342 to 347 in ac33969
const headerContent = ( | |
<header class={CSS.header} id={IDS.header}> | |
{this.renderIcon()} | |
{this.renderTitle()} | |
</header> | |
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
Before merging, does the PR summary scope need to be updated to fit the label over description?
…orkflow * origin/main: (26 commits) revert: refactor: add simpler `componentFocusable` util (deprecates `LoadableComponent`) (#9515) chore(linting): enable selector-pseudo-element-colon-notation rule (#9518) refactor(storybook): refactor tooltip simple story interface (#9538) refactor(dom): consolidate transition/animation waiting utils (#9341) refactor(storybook): build storybook simple story args interfaces using component class (#9457) chore: release next fix(block): add accessible label for slotted controls (#9502) feat(action-bar, action-pad): add expandLabel and collapseLabel to messages (#9497) chore: release next feat(action-menu, combobox, dropdown, input-date-picker, popover): allow logical placements for flipPlacements property. #8825 (#9490) fix(popover): correct border radius on close button (#9485) fix(list-item): hide nested list items by default (#9474) refactor: move component types into component specific interfaces files (#9527) chore: release next fix(alert): pause auto-close alert when link focused (#9503) refactor(storybook): consolidate storybook and component types (#9500) refactor(calcite-block-section,calcite-card): consolidate interfaces imports (#9517) chore: release next fix(block-section): restore block toggling when clicking on the header switch (#9472) chore: release next ...
Related Issue: #8037
Summary