Improve meaningful sequence in report/around page #5354

lucascumsille · 2025-02-05T09:50:00Z

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

This template seems very similar to the non-mobile one, I can combine them later.
#5208 (comment)

templates/web/base/report/display_tools_mobile.html is quite

codecov · 2025-02-05T10:01:49Z

Codecov Report

Attention: Patch coverage is 64.28571% with 10 lines in your changes missing coverage. Please review.

Project coverage is 82.29%. Comparing base (0df4ec8) to head (21312a1).

Files with missing lines	Patch %	Lines
web/cobrands/fixmystreet/fixmystreet.js	64.28%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5354      +/-   ##
==========================================
- Coverage   82.30%   82.29%   -0.01%     
==========================================
  Files         428      428              
  Lines       34057    34075      +18     
  Branches     5511     5511              
==========================================
+ Hits        28029    28041      +12     
- Misses       4397     4403       +6     
  Partials     1631     1631

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lucascumsille · 2025-02-05T10:21:31Z

A couple of notes from the previous PR:
#5208

if ($('#key-tools-mobile a.js-feed').length) {

This change shouldn't be made - the js-feed is still in key-tools on any page it is used (not a report page), and needs to moved from there to sub_map_links.
And I'm afraid that sub_map_links uses a background image, so you then end up with two RSS feed icons being shown.
#5208 (comment)

I solved this issue in this PR. See here:

lucascumsille · 2025-02-06T07:21:19Z

@dracos I made the changes requested in #5208 so I split the commits into 4:

1.A commit to switch from external to inline SVGs, no other changes (that would also involve e.g. fixing the issue where the Feed link is moved to sub_map_links).
2.A commit to add the close button to the small drawer (and anything else related to that, but nothing more).
3.The skip button on around page if needed.
4.Changes to have a separate key-tools for desktop than mobile (fixing the issue with placement, not working for non-staff, etc)

In the review I included the latest comments you left in #5208

lizettal · 2025-04-08T12:05:34Z

@dracos I think this is still with you for review. Could you please review this before you go on leave so we can try and get this wrapped up in the coming sprint.

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.

- 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.

dracos

Looks good, thanks for tidying and making all those changes :) I think the display_tools_mobile template can include the other SVG templates you changed the main template to use, and I had one question about when the buttons are hidden on mobile.
The browser tests fail a lot, I assume due to where things have moved to so that will need looking at, I can take a look at some of them if time later today.

dracos · 2025-04-10T10:53:27Z

web/cobrands/sass/_base.scss

+
+#key-tools {
+  html.js & {
+    display: none;


Why are the desktop buttons only hidden if JS is enabled? It means both sets of buttons are shown if JS isn't working - is there an issue with always hiding them, does something go wrong?

dracos · 2025-04-10T10:56:28Z

templates/web/base/report/display_tools_mobile.html

+        <li><form method="post" action="/report/[% problem.id %]/delete" id="remove-from-site-form">
+            <input type="hidden" name="token" value="[% csrf_token %]">
+            <button type="submit" id="key-tool-report-abuse" class="has-inline-svg has-inline-svg--column-wrap" data-confirm="[% loc('Are you sure?') %]" name="remove_from_site">
+              <svg role="presentation" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="1.5em" height="1.5em" viewBox="0 0 24 24"><path fill="none" d="M0 0h24v24H0z"/><path fill="currentColor" d="M12.866 3l9.526 16.5a1 1 0 0 1-.866 1.5H2.474a1 1 0 0 1-.866-1.5L11.134 3a1 1 0 0 1 1.732 0zM11 16v2h2v-2h-2zm0-7v5h2V9h-2z"/></svg>


This template could use the same icons/ templates I think? The contents of the SVGs look the same.

(Then, if all that's different then is the order of the text and the SVG, can flex order fix that? Then they could share more of the same template for the list items themselves.)

dracos · 2025-04-10T12:46:53Z

templates/web/base/report/display_tools_mobile.html

@@ -0,0 +1,43 @@
+<div class="shadow-wrap">


Sorry, one more thing - this file has some IDs in that are duplicating the ones in display_tools.html. I see you fixed the ones that you were then using in the JS, but the others will need fixing too - e.g. key-tool-report-abuse (not used at all, can be removed), key-tool-problems-nearby (does have some uses), and remove-from-site-form (not needed at all?)

This allows it more easily to appear in a better place on mobile from desktop.

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.

dracos · 2025-04-10T12:53:47Z

I've hopefully patched the commit so that the Cypress tests are passing again (they were getting confused now there were two key tools blocks in the source). So just the small issues above :)

lucascumsille force-pushed the 4560-meaningful-sequence-v2 branch from 7219efc to eb3bd29 Compare February 5, 2025 09:50

lucascumsille force-pushed the 4560-meaningful-sequence-v2 branch from eb3bd29 to a22092c Compare February 5, 2025 10:12

lucascumsille force-pushed the 4560-meaningful-sequence-v2 branch from 85e4c8d to 6360815 Compare February 5, 2025 10:53

lucascumsille marked this pull request as ready for review February 6, 2025 07:19

lucascumsille requested a review from dracos February 6, 2025 07:19

lucascumsille mentioned this pull request Feb 6, 2025

Improve meaningful sequence in report page #5208

Closed

dracos force-pushed the master branch from 7c6aeb4 to b2cca58 Compare February 12, 2025 14:58

lucascumsille added 2 commits April 10, 2025 11:58

Added accessibility changes to small_drawer

cf7c1bd

- 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.

dracos requested changes Apr 10, 2025

View reviewed changes

dracos force-pushed the 4560-meaningful-sequence-v2 branch from 6360815 to 47e8f90 Compare April 10, 2025 12:32

dracos reviewed Apr 10, 2025

View reviewed changes

lucascumsille added 2 commits April 10, 2025 13:52

Added key_tools for mobile.

9339c4b

This allows it more easily to appear in a better place on mobile from desktop.

dracos force-pushed the 4560-meaningful-sequence-v2 branch from 47e8f90 to 21312a1 Compare April 10, 2025 12:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve meaningful sequence in report/around page #5354

Improve meaningful sequence in report/around page #5354

lucascumsille commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

lucascumsille commented Feb 5, 2025 •

edited

Loading

lucascumsille commented Feb 6, 2025 •

edited

Loading

lizettal commented Apr 8, 2025

dracos left a comment

dracos Apr 10, 2025

dracos Apr 10, 2025

dracos Apr 10, 2025

dracos Apr 10, 2025

dracos commented Apr 10, 2025

Improve meaningful sequence in report/around page #5354

Are you sure you want to change the base?

Improve meaningful sequence in report/around page #5354

Conversation

lucascumsille commented Feb 5, 2025 • edited Loading

Preview

Preview Mobile

Pending from PR #5208

codecov bot commented Feb 5, 2025 • edited Loading

Codecov Report

lucascumsille commented Feb 5, 2025 • edited Loading

lucascumsille commented Feb 6, 2025 • edited Loading

lizettal commented Apr 8, 2025

dracos left a comment

Choose a reason for hiding this comment

dracos Apr 10, 2025

Choose a reason for hiding this comment

dracos Apr 10, 2025

Choose a reason for hiding this comment

dracos Apr 10, 2025

Choose a reason for hiding this comment

dracos Apr 10, 2025

Choose a reason for hiding this comment

dracos commented Apr 10, 2025

lucascumsille commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

lucascumsille commented Feb 5, 2025 •

edited

Loading

lucascumsille commented Feb 6, 2025 •

edited

Loading