Skip to content
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

Address accessibility issues #11064

Merged
merged 11 commits into from Oct 19, 2021
Merged

Address accessibility issues #11064

merged 11 commits into from Oct 19, 2021

Conversation

avpeery
Copy link
Contributor

@avpeery avpeery commented Sep 29, 2021

Addresses accessibility milestone issues: #11036, #11034, #11032, and #11031.

  1. [accessibility] Consider conveying only one of aria-label and title in controls #11036 Removes title in preference for only using aria-label in controls. Some screen-readers will announce both title and aria-label. Docs on aria-label, accessibility concerns with using title attribute.

  2. [accessibility] Consider replacing aria-pressed with aria-expanded on the attribution control #11034 Aria-pressed on attribution control is replaced with aria-expanded. Aria-expanded indicates that a grouping of other elements is displayed ("expanded") or hidden ("collapsed"). Here is the difference between aria-pressed and aria-expanded

  3. [accessibility] Interactive markers should convey the aria-expanded state of the controlled popup #11032 Interactive markers with popups should also use aria-expanded to reveal if the state of the marker is expanded or collapsed.

  4. [accessibility] Interactive markers should be role="button" #11031 role="button" should be set on interactive markers (aka markers with popups). Setting role="button" reveals that the element is clickable to screen readers. Docs on role button

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: Improvements to accessibility: conveying only aria-label in controls, replaced aria-pressed with aria-expanded in the attribution control, interactive markers with popups express an aria-expanded state, and interactive markers have the role "button".

@avpeery avpeery added the skip changelog Used for PRs that do not need a changelog entry label Sep 30, 2021
@avpeery avpeery marked this pull request as ready for review October 12, 2021 17:19
@avpeery avpeery added bug 🐞 and removed skip changelog Used for PRs that do not need a changelog entry labels Oct 12, 2021
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks great. The only reservation I have is removing the title attribute — I'm wondering if this also removes the tooltip that's shown when hovering the control, and if so, whether we should try the suggestion in the reported issue instead (where title and aria-label are put on different nested elements) so that we keep the current behavior.

@avpeery avpeery requested a review from mourner October 19, 2021 18:19
@avpeery
Copy link
Contributor Author

avpeery commented Oct 19, 2021

The only reservation I have is removing the title attribute — I'm wondering if this also removes the tooltip that's shown when hovering the control, and if so, whether we should try the suggestion in the reported issue instead (where title and aria-label are put on different nested elements) so that we keep the current behavior.

@mourner I checked it out and it did remove the native tooltips. I used the nested element as suggested to add the title attribute, and tooltips now work as expected. Thanks!

@avpeery avpeery requested a review from ansis October 19, 2021 22:09
@avpeery avpeery merged commit ab6203a into main Oct 19, 2021
@avpeery avpeery deleted the avpeery/accessibility branch October 19, 2021 22:23
katydecorah pushed a commit that referenced this pull request Oct 20, 2021
* main:
  Add touch pan blocker to gesture handling for touch devices (#11116)
  Address accessibility issues (#11064)
  add support for non-mercator projections (#11124)
  Image fallback expressions within paint properties (#11049)
  Replaces EPSG:4326 with OGC:CRS84 in GL JS `LngLat` doc (#11072)
  Add globe view support to heatmap shaders (#11120)
  Exclude flaky test (#11118)
  consistify YOUR_MAPBOX_ACCESS_TOKEN as placeholder string (#11113)
  Allow adding multiple layers to `map.on()` event handler (h/t @omerbn) (#11114)
  render-test-flakiness:clear worker storage (#11111)
  upgrade to supercluster v7.1.4, earcut v2.2.3, vt-pbf v3.1.3, geojson-rewind v0.5.1 (#11110)
  Added v1.13.2 changelog entry (#11108)
  One weird JSON.parse() trick (#11098)
  Fixed doc usage of map.getCenter (#11093)
  s̶y̶m̶b̶o̶l̶-̶c̶l̶i̶p̶ dynamic-filtering with `pitch` and `distance-from-camera` expressions (#10795)
  Update link to transpiling guide (#11096)
  Cherry pick 2.5.1 changelog (#11099)
  Fix an iOS15 issue where Safari tab bar interrupts panning (#11084)
  Fix conditional check for isFullscreen to accommodate Safari (#11086)
  Render tests for #11041 (#11070)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants