-
-
Notifications
You must be signed in to change notification settings - Fork 241
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
Improve meaningful sequence in report/around page #5354
base: master
Are you sure you want to change the base?
Conversation
7219efc
to
eb3bd29
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5354 +/- ##
==========================================
+ Coverage 82.42% 85.05% +2.63%
==========================================
Files 416 373 -43
Lines 32925 27642 -5283
Branches 5289 5289
==========================================
- Hits 27137 23510 -3627
+ Misses 4225 2570 -1655
+ Partials 1563 1562 -1 ☔ View full report in Codecov by Sentry. |
- This change will make it easier to control the icon size by passing it as an argument. - It will also minimise issues with passing the contrast test, where currently the sprite has a fixed colour that doesn't necessarily pass the contrast test, and it's quite hard to read against light backgrounds. Now, the icons use currentColor, so it inherits the value of the parent.
eb3bd29
to
a22092c
Compare
A couple of notes from the previous PR:
I solved this issue in this PR. See here: ![]() |
- Added aria-expanded. - Increase visibility of the drawer on mobile by moving it to the top of the screen. - Added close button to drawer. - If drawer is opened using keyboard then the focus jumps to the first element inside the drawer.
This commit should minimise the confusion for assistive device users. Currently, the way components are presented and the focus order is not linear; after they have travelled through the map, they are presented with the 'Can't use the map?' link and then go directly to the #key-tools component sitting at the bottom of the page, skipping the filters and report list. To respect the page sequence, the user should go directly to the report filter and then to the report list. The downside of this approach is that users who want to use the #key-tools would have to jump through a whole list of reports before being able to get to the #key-tools. The solution in this commit is to add a link right after the 'Can't use the map?' that gives the user some context about the mismatch between the reading sequence and also the opportunity to skip the #key-tools in case they just want to jump the next element which would be the report filters.
85e4c8d
to
6360815
Compare
@dracos I made the changes requested in #5208 so I split the commits into 4:
In the review I included the latest comments you left in #5208 |
Fixes: https://github.com/mysociety/societyworks/issues/4560
Replaces: #5208
Unlike the previous PR#5208 I thought that a better approach to include the icons would be to use HTML templates instead of copy and pasting the SVG. See commit here
Preview
Screen.Recording.2025-02-06.at.07.01.49.mov
Preview Mobile
Screen.Recording.2025-02-06.at.07.03.19.mov
Pending from PR #5208
templates/web/base/report/display_tools_mobile.html
is quite