Tracking upstream accessibility issues

Malvoz commented 3 years ago

Using code from Leaflet (and plugins) means this map component is sometimes inheriting accessibility issues.

We do have comments inline in code where fixes have been made, sometimes with links for upstream issues. Collecting them here in one spot helps get an overview of the state of accessibility in this component. This issue is to track upstream issues for discussion/fixes.

🔗 Map component

[x] Should be a programmatically identifiable element.
- Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/151/commits/7c86ddcbb8b39328b90930ffa1c5992f891f3df7.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7193.
[ ] ~~The controls container should come before other author provided content such as popups in the DOM (i.e. .leaflet-control-container should come before .leaflet-map-pane).~~ tab navigation for markers is problematic (see upstream issue below for explanation).
- Issue: https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/535.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7479.
[ ] Should have resizable text (relative font-size units).
- Upstream: https://github.com/Leaflet/Leaflet/issues/7548.

🔗 Controls in general

[ ] <a role="button"> should behave like <button>.
[x] Should be at least 44 by 44 CSS pixels.
- Fixed in #223.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7549.
[x] Should always show a focus outline on keyboard navigation.
- Fixed in mapml.css#L314-L321.
- Upstream:
- https://github.com/Leaflet/Leaflet/pull/7259.
- https://github.com/Leaflet/Leaflet/pull/7996.
[x] Should not have a focus outline unless actually focused.
- Fixed in mapml.css#L322-L326.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7499.
[ ] In many cases controls have both a title and an aria-label with the same value, which causes double speak in some screen readers. The title could be moved to the control's child icon element, which should then be set to aria-hidden="true" to preserve the tooltip-on-hover functionality while preventing double speak.
- Upstream Leaflet: TODO.
[ ] Focus should not be shifted to the map after controls have been interacted with.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7947.

🔗 Zoom controls

[x] Characters + and - should be explicitly hidden from ATs.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7433.
[x] Should convey the disabled state to ATs when min/max zoom has been reached.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7211.

🔗 Layer control

[x] Should have role="button".
- Upstream: https://github.com/Leaflet/Leaflet/issues/7498.
[ ] Should convey the aria-expanded state.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7526.
[ ] The layer menu's container should be conveyed as a group (of related user interfaces).
- Upstream: https://github.com/Leaflet/Leaflet/issues/7530.
[ ] Esc key should close the layer menu.
- Upstream Leaflet: TODO.
[ ] Focus should be delegated to the layer menu (e.g. to a tabindex="-1" on container, or first layer item) after activating the layer control, at least for keyboard users.
- Upstream Leaflet: TODO.
  🔗 Fullscreen control
[x] Should have role="button".

🔗 Markers

[x] Interactive markers should have an accessible name and role="button".
- Fixed (all features have a default accessible name of "Feature", including markers).
- Upstream: https://github.com/Leaflet/Leaflet/issues/7428.
[x] Interactive markers should convey the aria-expanded state of popups.
- Fixed.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7527.
[ ] Markers that don't do anything (i.e. don't bind to a popup?) should not be focusable by default.
- Upstream Leaflet: TODO.
[ ] Should be at least 44 by 44 CSS pixels (relates to https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/215).
- Upstream: https://github.com/Leaflet/Leaflet/issues/7549.
[x] ~~Markers (and other vector features) should be panned into view on focus.~~
- No longer needed in this component, see https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/482#issuecomment-998669122).
- Upstream: https://github.com/Leaflet/Leaflet/issues/8040.

🔗 Popups

[x] Popup content isn't announced by screen readers when opened. Focus should be delegated to the popup after it's been opened.
- Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/292 (focus is sent to .mapml-popup-content).
- Upstream: https://github.com/Leaflet/Leaflet/issues/2199 (describes a different problem, but proposes a shared solution).
[x] .leaflet-popup should have role="dialog".
- Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/468/commits/96127831d3f9ecb1b64405c3033efe5d52f4e4af (applied to .mapml-popup-content).
- Upstream: https://github.com/Leaflet/Leaflet/issues/7968.
[x] Popup's Close button should have an accessible name and role="button".
- Upstream: https://github.com/Leaflet/Leaflet/issues/7399.
[x] Character × in the popup's Close button should be explicitly hidden from ATs.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7472.
[x] Popup's Close button (and controls in mapml-focus-buttons) should be at least 44 by 44 CSS pixels (relates to https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/215).
- Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/468.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7549.
[x] Popup's Close button should have sufficient contrast ratio.
- Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/349.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7539.
[x] When focus is inside of a popup, pressing Esc should close the popup.
- Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/292.
- Upstream: https://github.com/Leaflet/Leaflet/issues/6551.
[ ] When a popup has been closed focus should not be sent to the <body> element, rather the previously focused element (e.g. marker) should be focused (if not available then the map container should be focused).
- [x] Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/292 for Esc key.
- [ ] Closing popups using the × close button should send focus to the marker/feature
- Upstream Leaflet: TODO.

[x] Menu items should have role="button".
- Fixed in 5d076fe.
- Upstream: https://github.com/aratcliffe/Leaflet.contextmenu/issues/136.
[x] When the context menu has been opened focus should be sent to (or into) it.
- Partially fixed in 95ecbcb (does not work in Chrome).
  - Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/523.
- Upstream: https://github.com/aratcliffe/Leaflet.contextmenu/issues/135.
[ ] When the context menu has been closed focus should be sent to the element that was focused before opening the context menu (if not available then the map container should be focused).
- Upstream: https://github.com/aratcliffe/Leaflet.contextmenu/issues/137.

🔗 Attribution

[x] Should have a legible font-size.
- Fixed in mapml.css#L31-L36.
- Upstream: https://github.com/Leaflet/Leaflet/pull/7346.
[ ] Should have resizable text (relative font-size units).
- Upstream: https://github.com/Leaflet/Leaflet/issues/7548.
[x] Links should have sufficient contrast ratio.
- Fixed in mapml.css#L112.
- Upstream: https://github.com/Leaflet/Leaflet/issues/6742.
[x] Links should easily be distinguishable from surrounding text (e.g. by preserving the default text-decoration).
- Fixed in mapml.css#L113.
- Upstream: https://github.com/Leaflet/Leaflet/issues/6742.
[x] The container element for the attribution should have sufficient contrast against underlying graphics.
- Fixed in mapml.css#L105-L108.
- Upstream: https://github.com/Leaflet/Leaflet/issues/7538.
[x] The container element for the attribution should provide contextual information, e.g. role="group" aria-label="Map data attribution" (or a control to toggle attribution should be labelled, per https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/216).
- Fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/598.
- Upstream Leaflet: TODO.
[ ] The vertical lines (separators) should be aria-hidden="true".
- May be fixed in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/642.
- Upstream: https://github.com/Leaflet/Leaflet/pull/7969.

🔗 Tooltips

[x] Tooltips aren't announced by screen readers.
- A fix is not desirable/necessary in this polyfill: features provide the accessible name, if this was enabled it'd cause double speak!
- Upstream: https://github.com/Leaflet/Leaflet/issues/8112

prushforth commented 3 years ago

Regarding:

Should be a programmatically identifiable element. Fixed in 7c86ddc. Upstream: Leaflet/Leaflet#7193.

Shouldn't this be on the mapml-viewer / map element? The leaflet-container is in shadow DOM.

Malvoz commented 3 years ago

Shouldn't this be on the mapml-viewer / map element?

The leaflet container has tabindex=0 so giving that element the accessible name means it can be announced to the user as it is focused when sequentially navigated to.

prushforth commented 3 years ago

I'm just surprised that shadow DOM is traversable by AT, but probably because I am not too familiar with how it works and how it fails.

Malvoz commented 3 years ago

@prushforth ATs construct an accessibility tree from the rendered DOM (and may be affected by CSS, e.g. display: none would hide the element and all descendants from the accessibility tree).

I've updated the list btw!

Maps4HTML / MapML.js