Uds 2143 - Implement: MVP Accessibility Carousels - #1726
Conversation
# [@asu/component-header-footer-v1.4.1](https://github.com/asu/asu-unity-stack/compare/@asu/component-header-footer-v1.4.0...@asu/component-header-footer-v1.4.1) (2026-05-12) ### Bug Fixes * **component-header-footer:** fix header dropdown height ([efa0bb7](efa0bb7))
# [@asu/unity-bootstrap-theme-v1.38.1](https://github.com/ASU/asu-unity-stack/compare/@asu/unity-bootstrap-theme-v1.38.0...@asu/unity-bootstrap-theme-v1.38.1) (2026-05-13) ### Bug Fixes * **unity-bootstrap-theme:** blockquote css update for maxwidth standards ([92976a5](92976a5)) * **unity-bootstrap-theme:** padding-left: 0 applied to uds-blockquote ([aa558f6](aa558f6)) * **unity-bootstrap-theme:** removed padding right from the blockquote css ([ff90b82](ff90b82))
|
Storybook deployed at https://unity-uds-staging.s3.us-west-2.amazonaws.com/pr-1726/index.html |
mlsamuelson
changed the title
davidornelas11
left a comment
davidornelas11
left a comment
There was a problem hiding this comment.
Left a few inline comments. Main ones: aria-labelledby isn't pointing to an actual element id, the pagination group still doesn't have its aria-label (this was already flagged on UDS-2150), and role="figure" on the image gallery carousel gets silently dropped now. Also flagged the aria-live/role="alert" bit since #1763 already replaces it with a cleaner approach. Prev/next button disabled state work looks great though. One more thing worth a look (outside this diff): ImageBulletItems in BulletItems.jsx still has the old "Slide view X" label and aria-selected never updates - probably needs the same treatment at some point.
| let cardDiv = slides[i].querySelectorAll(`.card`) | ||
| if (cardDiv && cardDiv[0]) { | ||
| cardDiv[0].setAttribute("aria-live", "polite"); | ||
| cardDiv[0].setAttribute("role", "alert"); |
There was a problem hiding this comment.
role="alert" plus an explicit aria-live="polite" on the same element conflicts (alert already implies assertive), and this announces the whole card body on every slide change instead of something short. Looks like #1763 already dropped this exact block for a cleaner aria-hidden/gliderTrack approach, might be easiest to just pull that over instead of patching this in place.
| if (header) { | ||
| let allTexts = [] | ||
| getText(slides[i], allTexts) | ||
| gliderElement.setAttribute("aria-labelledby", allTexts[0]); |
There was a problem hiding this comment.
aria-labelledby needs to reference an element id, not raw text - allTexts[0] is just the heading's text content, so this won't resolve to anything for a screen reader. Also getText is called on the whole slide instead of just header, so it could grab text that comes before the heading.
| gliderElement.setAttribute("aria-labelledby", allTexts[0]); | ||
| gliderElement.removeAttribute("aria-label"); | ||
| } else { | ||
| gliderElement.setAttribute("aria-label", `Card ${i + 1}`); |
There was a problem hiding this comment.
This fallback runs for any carousel without a heading in the slide, not just cards, an image/gallery slide with no caption would get announced as "Card 1". Might be worth making this generic ("Slide 1") instead.
|
|
||
|
|
||
| // Update bullet accessibility | ||
| const bullets = gliderElement.querySelectorAll(".glide__bullet"); |
There was a problem hiding this comment.
Heads up, for the image gallery bullets .glide__bullet matches the <img>, not the <button role="option"> wrapping it, so disabled/aria-current won't actually do anything there.
| aria-labelledby={ariaLabelledBy} | ||
| // role={role} | ||
| // role="group" | ||
| role="region" |
There was a problem hiding this comment.
role is still a prop here but it's dead now that role is hardcoded to "region" - ImageGalleryCarousel passes role="figure" and that'll get silently overridden. Also the commented-out lines above can probably go.
| // role={role} | ||
| // role="group" | ||
| role="region" | ||
| aria-label="Carousel pagination" |
There was a problem hiding this comment.
This aria-label should be on the bullets group container, not here - that's the exact gap Andrew flagged on UDS-2150 (pagination group missing its label). BaseBulletItemContainer still only has role="group" with nothing on it.
5 participants
Description
Checklist
Important Reminders
Links