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

Remove aria-hidden from parents of #fms_pan_zoom #3751

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

zarino
Copy link
Member

@zarino zarino commented Jan 17, 2022

Since the #fms_pan_zoom map controls are keyboard-navigable (and could conceivably be used by even non-sighted users, wanting to move the map view to reveal new markers in the #map_sidebar list) we need to make sure the controls aren’t hidden inside an aria-hidden parent.

Removing aria-hidden from #map_box, however, now exposes a whole load of new elements to assistive devices, including (in order):

  1. each of the image tiles
  2. each of the pin shadows
  3. each of the pins
  4. map data attribution at the bottom
  5. each of the pan/zoom/other controls

In this commit, I’ve at least hidden the image tiles and pins in the non-JavaScript version(s) of the map. But doing the same thing in the OpenLayers JavaScript is beyond my current understanding!

Perhaps a developer can take a look?

TODO:

  • Tell OpenLayers to mark the image tiles and pin shadows as aria-hidden="true"
  • Decide whether to leave the pins accessible, or mark them as aria-hidden="true" as well (given there’s no way to skip over/past them, and there’s a list of all reports in #map_sidebar anyway).

@codecov
Copy link

codecov bot commented Jan 17, 2022

Codecov Report

Merging #3751 (36769d3) into master (38a4820) will decrease coverage by 13.38%.
The diff coverage is 100.00%.

❗ Current head 36769d3 differs from pull request most recent head 0b97d2b. Consider uploading reports for the commit 0b97d2b to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3751       +/-   ##
===========================================
- Coverage   82.32%   68.94%   -13.39%     
===========================================
  Files         352      284       -68     
  Lines       23920    18693     -5227     
  Branches     3635     3478      -157     
===========================================
- Hits        19692    12887     -6805     
- Misses       3090     4761     +1671     
+ Partials     1138     1045       -93     
Impacted Files Coverage Δ
perllib/FixMyStreet/App/Controller/Report.pm 88.23% <100.00%> (-0.85%) ⬇️
perllib/FixMyStreet/Cobrand/Buckinghamshire.pm 42.10% <100.00%> (-33.35%) ⬇️
perllib/FixMyStreet/Roles/BoroughEmails.pm 6.25% <0.00%> (-93.75%) ⬇️
perllib/FixMyStreet/Map/Bing.pm 6.66% <0.00%> (-93.34%) ⬇️
perllib/FixMyStreet/SendReport/Email/Highways.pm 3.57% <0.00%> (-92.86%) ⬇️
perllib/FixMyStreet/Map/OS/FMS.pm 9.09% <0.00%> (-90.91%) ⬇️
perllib/FixMyStreet/Map/CheshireEast.pm 9.09% <0.00%> (-90.91%) ⬇️
perllib/FixMyStreet/Cobrand/Zurich.pm 3.26% <0.00%> (-87.91%) ⬇️
perllib/FixMyStreet/Roles/Open311Alloy.pm 14.28% <0.00%> (-85.72%) ⬇️
perllib/FixMyStreet/SendReport/Zurich.pm 5.26% <0.00%> (-84.22%) ⬇️
... and 186 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38a4820...0b97d2b. Read the comment docs.

@dracos
Copy link
Member

dracos commented Feb 4, 2022

"In this commit, I’ve at least hidden the image tiles and pins in the non-JavaScript version(s) of the map" - only the tiles I think? pins is the next div on.

@dracos dracos force-pushed the societyworks/2803-aria-visible-map-controls branch from 634544b to 8cc0c81 Compare February 4, 2022 12:03
@dracos dracos changed the title [WIP] Remove aria-hidden from parents of #fms_pan_zoom Remove aria-hidden from parents of #fms_pan_zoom Feb 4, 2022
@dracos dracos marked this pull request as ready for review February 4, 2022 12:04
@zarino
Copy link
Member Author

zarino commented Feb 4, 2022

Thanks for fixing this up @dracos 👍

I’ve tried it out in Safari with VoiceOver and the map controls are all now accessible with ⌃⎇→ etc. The flow is:

  1. header
  2. navigation menu
  3. "main" element
  4. the map copyright statement (shame that comes first, but hey ho)
  5. each map control in order, starting with "pan up"
  6. "Click map to report a problem" banner
  7. "Can’t see the map, skip this step"
  8. "Get updates" link
  9. list filter form
  10. pagination
  11. each list item link in order

Happy for me to squash into one commit, with a changelog entry, and merge?

@dracos
Copy link
Member

dracos commented Feb 4, 2022

Can we hide the copyright? I'm tempted if it's first, I mean, the map is hidden after all...

@zarino zarino force-pushed the societyworks/2803-aria-visible-map-controls branch from 8cc0c81 to 98e5c2e Compare February 4, 2022 17:57
@zarino zarino requested a review from dracos February 4, 2022 17:57
@dracos dracos force-pushed the master branch 4 times, most recently from c85ba77 to 38a4820 Compare February 4, 2022 18:58
Removes `aria-hidden` from `#map_box`, but adds it back onto the map
image tiles and pins, to allow keyboard and assistive device users to
tab through the map controls without having to wade through potentially
hundreds of map markers. After the map controls, keyboard focus passes
to the links, form filters, and list of reports in `#map_sidebar`.

Fixes mysociety/societyworks#2803.
@dracos dracos force-pushed the societyworks/2803-aria-visible-map-controls branch from 98e5c2e to 0b97d2b Compare February 4, 2022 19:23
@dracos dracos merged commit 0b97d2b into master Feb 4, 2022
@github-pages github-pages bot temporarily deployed to github-pages February 4, 2022 20:03 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants