fix(wcag): Legend panel issues#3353
fix(wcag): Legend panel issues#3353kenchase wants to merge 1 commit intoCanadian-Geospatial-Platform:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses WCAG/accessibility issues identified during the Legend Panel review by improving ARIA semantics, focus management, and DOM ID uniqueness across GeoView core UI components (and the custom legend plugin), plus updating accessibility documentation and translations.
Changes:
- Standardize/expand unique ID generation patterns (often prefixed with
mapId+containerType) to prevent duplicate IDs and strengthen ARIA relationships. - Improve keyboard/screen-reader support in Legend/Geolocator (ARIA live regions for loading, better ARIA labeling/state, focus restoration for fullscreen/lightbox flows).
- Harden accessibility primitives: shared
visuallyHiddenutility, Switch label enforcement + label↔control association, and styling foraria-disabledicon buttons.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/geoview-custom-legend/src/components/layer-item.tsx | Pass mapId/containerType into core LegendLayer from the custom legend plugin. |
| packages/geoview-core/src/ui/switch/switch.tsx | Make label required and add generated IDs to associate labels with switches. |
| packages/geoview-core/src/ui/switch/switch-style.ts | Improve switch focus indication styling. |
| packages/geoview-core/src/ui/style/themeOptionsGenerator.ts | Add styling for aria-disabled="true" on outlined icon buttons. |
| packages/geoview-core/src/ui/style/default.ts | Introduce shared visuallyHidden sr-only style utility. |
| packages/geoview-core/src/ui/panel/panel.tsx | Improve panel semantics/ARIA and align panel element IDs with naming conventions. |
| packages/geoview-core/src/ui/linear-progress/linear-progress.tsx | Add optional aria-label support for progress bars. |
| packages/geoview-core/src/core/components/toggle-all/toggle-all.tsx | Require source/containerType, update IDs, and adjust tooltips/switch props for A11Y. |
| packages/geoview-core/src/core/components/notifications/notifications.tsx | Update notifications trigger ID/ARIA and align with dialog semantics. |
| packages/geoview-core/src/core/components/nav-bar/buttons/measurement.tsx | Update Switch labels to be non-nullable. |
| packages/geoview-core/src/core/components/legend/legend.tsx | Pass mapId/containerType into LegendLayer; add fullscreen focus restoration ref; update ToggleAll usage. |
| packages/geoview-core/src/core/components/legend/legend-styles.ts | Remove truncation styles; reuse visuallyHidden; tweak layout padding. |
| packages/geoview-core/src/core/components/legend/legend-layer.tsx | Add ARIA live status announcements, stronger ARIA attributes, and unique IDs for collapse/name linkage. |
| packages/geoview-core/src/core/components/legend/legend-layer-items.tsx | Improve visibility toggle labeling and stable ID generation format. |
| packages/geoview-core/src/core/components/legend/legend-layer-ctrl.tsx | Improve icon button ARIA labeling/state; use aria-disabled for focus-retaining “disabled” behavior. |
| packages/geoview-core/src/core/components/legend/legend-layer-container.tsx | Make WMS legend images interactive via ButtonBase; add region semantics + aria-labelledby for collapsible content. |
| packages/geoview-core/src/core/components/legend/legend-fullscreen.tsx | Improve fullscreen legend semantics, IDs, title text, and focus restoration behavior. |
| packages/geoview-core/src/core/components/layers/right-panel/layer-details.tsx | Update Switch labels to be non-nullable. |
| packages/geoview-core/src/core/components/layers/layers-toolbar.tsx | Add containerType, unique IDs, and updated ToggleAll usage. |
| packages/geoview-core/src/core/components/layers/layers-panel.tsx | Pass containerType through to LayersToolbar. |
| packages/geoview-core/src/core/components/geolocator/geolocator.tsx | Add ARIA live loading announcements and modal semantics conditional on focus trap mode. |
| packages/geoview-core/src/core/components/geolocator/geolocator-style.ts | Replace duplicated sr-only CSS with shared visuallyHidden. |
| packages/geoview-core/src/core/components/geolocator/geolocator-result.tsx | Use aria-disabled for clear-filters focus behavior + guard handler when disabled. |
| packages/geoview-core/src/core/components/export/export-modal.tsx | Update dialog/menu/input IDs to include mapId for uniqueness. |
| packages/geoview-core/src/core/components/export/export-modal-button.tsx | Simplify export button ARIA to dialog semantics and update props. |
| packages/geoview-core/src/core/components/details/feature-info-table.tsx | Update lightbox initialization signature to support alt text + focus restoration IDs. |
| packages/geoview-core/src/core/components/details/coordinate-info.tsx | Update Switch label to be non-nullable. |
| packages/geoview-core/src/core/components/data-table/filter-map.tsx | Update Switch label to be non-nullable. |
| packages/geoview-core/src/core/components/data-table/data-table.tsx | Update lightbox init call signature to include altText + focus restoration ID. |
| packages/geoview-core/src/core/components/common/layer-list.tsx | Remove redundant aria-disabled where native disabled is used. |
| packages/geoview-core/src/core/components/common/hooks/use-light-box.tsx | Split lightbox params into altText and returnFocusId for better semantics/focus handling. |
| packages/geoview-core/src/core/components/app-bar/buttons/version.tsx | Update IDs and focus-trap linkage; align dialog semantics. |
| packages/geoview-core/src/core/components/app-bar/app-bar.tsx | Adjust ARIA behavior based on WCAG/focus-trap mode and improve ESC handling when lightbox is open. |
| packages/geoview-core/src/core/components/app-bar/app-bar-helper.ts | Import ordering cleanup. |
| packages/geoview-core/public/locales/fr/translation.json | Add new accessibility-related strings and update legend fullscreen title string. |
| packages/geoview-core/public/locales/en/translation.json | Add new accessibility-related strings and update legend fullscreen title string. |
| docs/app/accessibility.md | Expand internal accessibility best-practices guidance (WIP). |
Comments suppressed due to low confidence (2)
packages/geoview-core/src/core/components/notifications/notifications.tsx:258
- The notifications trigger button no longer exposes open state/relationship information to AT (previous
aria-controls/aria-expandedwere removed). If this Popper is still used as a toggleable dialog/popover, consider restoringaria-expandedandaria-controls(at least whenactiveTrapGeoViewis false), similar to the conditional ARIA pattern used for AppBar panel buttons.
<IconButton
id={`${mapId}-${CONTAINER_TYPE.APP_BAR}-notifications-btn`}
aria-label={t('appbar.notifications')}
aria-haspopup="dialog"
tooltipPlacement="right"
onClick={handleOpenPopover}
className={`${interaction === 'dynamic' ? 'buttonFilled' : 'style4'} ${open ? 'active' : ''}`}
packages/geoview-core/src/core/components/app-bar/buttons/version.tsx:131
- The version trigger button no longer includes
aria-controls/aria-expandedfor the Popper dialog it toggles. This removes state/relationship cues that assistive tech can use. Consider adding them back conditionally (e.g., when not running in focus-trap/WCAG mode), aligned with the approach used for AppBar panel buttons.
<IconButton
id={`${mapId}-${CONTAINER_TYPE.APP_BAR}-version-btn`}
aria-haspopup="dialog"
aria-label={t('appbar.version')}
tooltipPlacement="right"
onClick={handleOpenPopover}
className={`${interaction === 'dynamic' ? 'buttonFilled' : 'style4'} ${open ? 'active' : ''}`}
>
You can also share your feedback on Copilot code review. Take the survey.
| const { label, ...otherProps } = props; | ||
|
|
||
| // Hooks | ||
| const switchId = useId(); // WCAG - Unique ID to associate label with switch | ||
| const theme = useTheme(); | ||
| const sxClasses = useMemo(() => getSxClasses(theme), [theme]); | ||
|
|
||
| return <MaterialFormControlLabel control={<MaterialSwitch {...otherProps} />} label={label} sx={sxClasses.formControl} />; | ||
| return ( | ||
| <MaterialFormControlLabel | ||
| htmlFor={switchId} | ||
| control={<MaterialSwitch id={switchId} {...otherProps} />} | ||
| label={label} |
There was a problem hiding this comment.
Will review next PR
| '&.buttonOutline': { | ||
| backgroundColor: 'transparent', | ||
| border: `3px solid transparent`, | ||
| color: `${geoViewColors.primary.main}`, | ||
| '&:hover, &:active, &.active': { | ||
| backgroundColor: `${geoViewColors.bgColor.dark[100]}`, | ||
| border: `3px solid ${geoViewColors.primary.light[500]}`, | ||
| color: `${geoViewColors.primary.dark[100]}`, | ||
| boxShadow: 1, | ||
| }, | ||
| '&:disabled': { | ||
| color: `${geoViewColors.bgColor.dark[450]}`, | ||
| backgroundColor: 'transparent', | ||
| }, | ||
| '&[aria-disabled="true"]': { | ||
| color: `${geoViewColors.bgColor.dark[450]}`, | ||
| backgroundColor: 'transparent', | ||
| cursor: 'not-allowed', | ||
| }, |
There was a problem hiding this comment.
Out of scope. Will review during next PR.
|
|
||
| _Work in progress_ | ||
|
|
||
| The viewer needs to be accessible for keyboard and screen reader. It's should follow WCAG 2.1 requirements: https://www.w3.org/TR/WCAG21 |
There was a problem hiding this comment.
WIP. Will review once content is closer to being finalized.
kenchase
left a comment
There was a problem hiding this comment.
@kenchase made 3 comments.
Reviewable status: 0 of 37 files reviewed, 3 unresolved discussions (waiting on jolevesq).
|
|
||
| _Work in progress_ | ||
|
|
||
| The viewer needs to be accessible for keyboard and screen reader. It's should follow WCAG 2.1 requirements: https://www.w3.org/TR/WCAG21 |
There was a problem hiding this comment.
WIP. Will review once content is closer to being finalized.
| '&.buttonOutline': { | ||
| backgroundColor: 'transparent', | ||
| border: `3px solid transparent`, | ||
| color: `${geoViewColors.primary.main}`, | ||
| '&:hover, &:active, &.active': { | ||
| backgroundColor: `${geoViewColors.bgColor.dark[100]}`, | ||
| border: `3px solid ${geoViewColors.primary.light[500]}`, | ||
| color: `${geoViewColors.primary.dark[100]}`, | ||
| boxShadow: 1, | ||
| }, | ||
| '&:disabled': { | ||
| color: `${geoViewColors.bgColor.dark[450]}`, | ||
| backgroundColor: 'transparent', | ||
| }, | ||
| '&[aria-disabled="true"]': { | ||
| color: `${geoViewColors.bgColor.dark[450]}`, | ||
| backgroundColor: 'transparent', | ||
| cursor: 'not-allowed', | ||
| }, |
There was a problem hiding this comment.
Out of scope. Will review during next PR.
| const { label, ...otherProps } = props; | ||
|
|
||
| // Hooks | ||
| const switchId = useId(); // WCAG - Unique ID to associate label with switch | ||
| const theme = useTheme(); | ||
| const sxClasses = useMemo(() => getSxClasses(theme), [theme]); | ||
|
|
||
| return <MaterialFormControlLabel control={<MaterialSwitch {...otherProps} />} label={label} sx={sxClasses.formControl} />; | ||
| return ( | ||
| <MaterialFormControlLabel | ||
| htmlFor={switchId} | ||
| control={<MaterialSwitch id={switchId} {...otherProps} />} | ||
| label={label} |
There was a problem hiding this comment.
Will review next PR
kenchase
left a comment
There was a problem hiding this comment.
@kenchase resolved 3 discussions.
Reviewable status: 0 of 37 files reviewed, all discussions resolved (waiting on jolevesq).
jolevesq
left a comment
There was a problem hiding this comment.
@jolevesq reviewed 36 files and all commit messages, and made 6 comments.
Reviewable status: 36 of 37 files reviewed, 3 unresolved discussions (waiting on kenchase).
packages/geoview-core/src/core/components/legend/legend-fullscreen.tsx line 26 at r1 (raw file):
* @property {boolean} isOpen - Controls whether the fullscreen dialog is open. * @property {() => void} onClose - Callback function invoked when the fullscreen dialog is closed. * @property {React.RefObject<HTMLButtonElement>} buttonRef - Reference to the fullscreen button for focus restoration.
FYI we do not need the the property type anymore, just e.g. buttonRef - meaning
packages/geoview-core/src/core/components/legend/legend-layer.tsx line 150 at r1 (raw file):
); // WCAG - Track layer status changes for screen reader announcements
I like the // WCAG comment everywhere we need to do stuff for WCAG!
docs/app/accessibility.md line 185 at r1 (raw file):
**What this means for users in practice** — AT users are not blocked from operating the legend (they can show/hide classes by name), but they cannot independently interpret what the map symbols look like. This is a partial conformance gap rather than a complete barrier to use. **What would unblock the fix** — if the symbology data driving each legend image is available in the layer configuration (e.g. colour hex, symbol type, size range), those values could be used to auto-generate sr-only descriptions programmatically without manual authoring per symbol. That might be worth investigating as a future enhancement.
Nice doc!
packages/geoview-core/src/core/components/export/export-modal.tsx line 358 at r1 (raw file):
{/* Format Selection Menu */} <Menu id={`${mapId}-format-selection`} open={formatMenuOpen} onClose={handleFormatMenuClose} anchorEl={formatAnchorEl}>
Should we add the export like id-export-format-selection? When we read it would be more clear...
packages/geoview-core/src/core/components/export/export-modal.tsx line 370 at r1 (raw file):
{(exportFormat === 'png' || exportFormat === 'jpeg') && ( <> <Menu id={`${mapId}-dpi-selection`} open={dpiMenuOpen} onClose={handleMenuClose} anchorEl={dpiAnchorEl}>
Same
packages/geoview-core/src/core/components/export/export-modal.tsx line 384 at r1 (raw file):
{exportFormat === 'jpeg' && ( <> <Menu id={`${mapId}-quality-selection`} open={qualityMenuOpen} onClose={handleQualityMenuClose} anchorEl={qualityAnchorEl}>
Same
Alex-NRCan
left a comment
There was a problem hiding this comment.
@Alex-NRCan reviewed 37 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on kenchase).
packages/geoview-custom-legend/src/components/layer-item.tsx line 25 at r1 (raw file):
if (!isLegendLayer(item)) return; return <LegendLayer layerPath={item.layerPath} showControls={true} mapId={mapId} containerType={CONTAINER_TYPE.APP_BAR} />;
Question: Is there a particular reason of doing the useGeoViewMapId() at this level instead of doing it in the LegendLayer component? (It'd save having to open a props on <LegendLayer>)
legend.tsx also wouldn't have to send the mapId to it.
DamonU2
left a comment
There was a problem hiding this comment.
@DamonU2 reviewed 37 files and all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on kenchase).
* documentation: update accessibility best practices md file (work in progress) * geolocator: add aria-disabled to clear filters button when no filters selected * geolocator: updated loading status region for A11Y (aria-live) (ProgressBar) * global: updates to use unique ids * global styles: add css rule for aria-disabled icon buttons (buttonOutline) * global styles: add reusable style utility for visuallyHidden * legend layer: remove truncation from legend title * legend layer: remove tooltips from non interactive elements (legend title) * legend layer: add accessible code for loading layer * legend layer ctrl: fix to use aria-disabled instead of disabled attribute on IconButtons (for consistent focus management) * legend layer ctrl: improve performance by memoizing control action handlers and reading state values imperatively from the store within handlers * legend panel: fix fullscreen icon button focus management * legend panel: fix to allow both zoom icon buttons to retain focus after being pressed * legend panel: improve aria and semantic HTML implementation * legend panel: fix images that open a lightbox to use an interactive element (button) * legend panel: fix focus management on lightbox close (returns focus to triggering item) * legend panel fullscreen: update panel title to be more descriptive (read only) * panel and other modals: improve aria and semantic HTML implementation * panel and appBar and related: updated to use consistent ID naming conventions (mapId-containerType...) * switch: make label required for accessibility * switch: make focus indicators more noticeable * switch: add unique id to associate the label to the switch * use-light-box: update to use separate props for image alt test and focus management element ID * use-light-box (global): update to set alt text to "" where descriptive alt text is unavailable
a3701b9 to
11bca6c
Compare
kenchase
left a comment
There was a problem hiding this comment.
@kenchase made 4 comments.
Reviewable status: 30 of 37 files reviewed, 4 unresolved discussions (waiting on Alex-NRCan, DamonU2, and jolevesq).
packages/geoview-core/src/core/components/export/export-modal.tsx line 358 at r1 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Should we add the export like id-export-format-selection? When we read it would be more clear...
Done.
packages/geoview-core/src/core/components/export/export-modal.tsx line 370 at r1 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Same
Done.
packages/geoview-core/src/core/components/export/export-modal.tsx line 384 at r1 (raw file):
Previously, jolevesq (Johann Levesque) wrote…
Same
Done.
packages/geoview-custom-legend/src/components/layer-item.tsx line 25 at r1 (raw file):
Previously, Alex-NRCan (Alex) wrote…
Question: Is there a particular reason of doing the useGeoViewMapId() at this level instead of doing it in the LegendLayer component? (It'd save having to open a props on <LegendLayer>)
legend.tsx also wouldn't have to send the mapId to it.
That makes sense. I've updated related files as per your comment.
jolevesq
left a comment
There was a problem hiding this comment.
@jolevesq reviewed 8 files and all commit messages, and resolved 3 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Alex-NRCan).
Description
A11Y fixes related to WCAG review of the legend panel.
Fixes #3327
Type of change
How Has This Been Tested?
Tested manually using keyboard navigation, Chrome dev tools, W3C HTML validator.
Add the URL for your deploy!
Checklist:
I have made corresponding changes to the documentationI have added tests that prove my fix is effective or that my feature worksNew and existing unit tests pass locally with my changesThis change is