From 388edeef5dee21d079364169f7ce5b3a59b8bf5a Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 4 Aug 2021 09:49:52 +0100 Subject: [PATCH 01/17] Accessibility fixes for map Makes the controls and links inside it match GOVUK Frontend styles and: - gives the map container a focus style matching that for GOVUK Frontend text inputs - makes it all work in high contrast modes (on Windows and Firefox) Note: the focus style for the container is applied with :focus-visible so only appears when it gets focus directly, not when it does due to child elements (like the controls or links) getting focused. Browsers without support for :focus-visible get the same styling for all forms of focus. --- app/assets/stylesheets/map.scss | 148 ++++++++++++++++++ .../partials/area-map-stylesheets.html | 2 +- gulpfile.js | 5 +- 3 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 app/assets/stylesheets/map.scss diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss new file mode 100644 index 000000000..b512a719e --- /dev/null +++ b/app/assets/stylesheets/map.scss @@ -0,0 +1,148 @@ +@import 'leaflet/dist/leaflet'; + +// Styles to make the leaflet map match GOVUK (accessibility-focused) styling + +@import "settings/all"; +@import "helpers/all"; + +// Reset focus colour to latest version, to match the rest of the app +$govuk-focus-colour: #FFDD00; +$zoom-button-colour: govuk-colour("white"); +$zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); + +// The focus style is a 2-colour outline, made to match the GOVUK text input focus style. +// When colours are overridden, for example when users have a dark mode, box-shadows disappear, +// so the outline which is left as a single colour (defined by the OS, to replace 'transparent'). +// +// Leaflet adds focus styles with JS, through inline styles. Because of their higher precedence +// we need to mark our overrides with !important. +// +// This also uses :focus-visible to stop it showing focus when you click zoom in/out. +// This approach, taken from MDN, allows it to fall back to :focus for browsers without support: +// https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible#selectively_showing_the_focus_indicator +.leaflet-container { + + &:focus { + box-shadow: 0px 0px 0 3px $govuk-focus-text-colour, 0 0 0 6px $govuk-focus-colour; + outline: solid 3px transparent !important; // sass-lint:disable-line no-important + } + + &:focus:not(:focus-visible) { + box-shadow: none; + } + + &:focus-visible { + box-shadow: 0px 0px 0 3px $govuk-focus-text-colour, 0 0 0 6px $govuk-focus-colour; + outline: solid 3px transparent !important; // sass-lint:disable-line no-important + + // You only see the outline in forced colour mode which doesn't have enough contrast with the + // map so this creates some space between them to mimic the two-colour default styling + @media (forced-colors: active) { + outline-offset: 3px; + } + } +} + +// Overrides for zoom controls to make them match GOVUK buttons +// https://design-system.service.gov.uk/components/button/ +// +// GOVUK buttons have an invisible outline, used for high contrast modes. Our buttons are cropped by +// their container so we use a border instead. +// +// Also introduces a black divider between the buttons, made with a pseudo element, appearing on +// focus to mimic the 2-colour style GOVUK Frontend buttons and links have +.leaflet-bar a { + + // Allow it to contain the absolutely positioned divider bar we show between buttons + // when one is focused + &:last-child { + position: relative; + } + + // Hover style is darker background + &:hover { + background-color: $zoom-button-hover-colour; + } + + // Styles that apply if focused with or without the :hover or :active state + &:focus { + // When colours are overridden, for example when users have a dark mode, + // backgrounds and box-shadows disappear, so we need to ensure there's a + // transparent outline which will be set to a visible colour by the OS. + border: solid 2px transparent; + box-sizing: border-box; // make sure height includes the border + line-height: 26px; // subtract the border from the height (normally 30px) + + // The inline display box for the button text overlaps the button edge. + // This is only visible in high contrast mode because of the background colour being set. + @media (forced-colors: active) { + overflow: hidden; + } + } + + // Focused style sets the button to the focus colour + &:focus:not(:active):not(:hover) { + color: $govuk-focus-text-colour; + background-color: $govuk-focus-colour; + } + + // Retain the space between buttons now we're using the border in the focus style + &:first-child:focus { + margin-bottom: 1px; + } + + // 3px divider between buttons as a version of the underline from the GOVUK focus style + &:last-child:focus:not(:active):not(:hover):before, + &:first-child:focus:not(:active):not(:hover) + a:before { + content: ""; + position: absolute; + height: 2px; + background: $govuk-focus-text-colour; + + @media (forced-colors: active) { + background: canvasText; + } + } + + // Styles for the divider, specifc to the second button being focused + &:last-child:focus:not(:active):not(:hover):before { + left: -2px; // Subtract 2px left border + top: -3px; // Subtract 2px top border and position so it overlaps space above button + width: calc(100% + 4px); // Subtract borders + } + + // Styles for the divider, specifc to the first button being focused + &:first-child:focus:not(:active):not(:hover) + a:before { + left: 0px; // No borders on parent of :before when not focused + top: -1px; // No borders so just overlap space above parent button + width: 100%; + } +} + +// Extra block to override specific LeafletJS rounded-corners when buttons are focused +.leaflet-touch .leaflet-bar a { + &:first-child:focus, + &:last-child:focus { + border-radius: initial; + } +} + +// Map attribution links +.leaflet-control-attribution { + & a { + &:focus { + // When colours are overridden, for example when users have a dark mode, + // backgrounds and box-shadows disappear, so we need to ensure there's a + // transparent outline which will be set to a visible colour. + outline: $govuk-focus-width solid transparent !important; // sass-lint:disable-line no-important + color: $govuk-focus-text-colour; + background: $govuk-focus-colour; + box-shadow: 0 -2px $govuk-focus-colour, 0 4px $govuk-focus-text-colour; + } + + // use an extra prefix class to override LeafletJS CSS + .leaflet-bottom &:hover { + text-decoration: none; + } + } +} diff --git a/app/templates/views/broadcast/partials/area-map-stylesheets.html b/app/templates/views/broadcast/partials/area-map-stylesheets.html index e007724a1..84e8c32a6 100644 --- a/app/templates/views/broadcast/partials/area-map-stylesheets.html +++ b/app/templates/views/broadcast/partials/area-map-stylesheets.html @@ -1 +1 @@ - + diff --git a/gulpfile.js b/gulpfile.js index 9dbd998ee..4412a1ddd 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -202,8 +202,8 @@ const javascripts = () => { const sass = () => { return src([ paths.src + '/stylesheets/main*.scss', - paths.src + '/stylesheets/print.scss', - paths.npm + '/leaflet/dist/leaflet.css' + paths.src + '/stylesheets/map.scss', + paths.src + '/stylesheets/print.scss' ]) .pipe(plugins.prettyerror()) .pipe(plugins.sass({ @@ -212,6 +212,7 @@ const sass = () => { paths.npm + 'govuk-elements-sass/public/sass/', paths.toolkit + 'stylesheets/', paths.govuk_frontend, + paths.npm ] })) .pipe(plugins.cssUrlAdjuster({ From f70dcc8329a8ba8f020fe35fd688f45a7aa57a3c Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 11 Aug 2021 16:39:58 +0100 Subject: [PATCH 02/17] Remove rounded corners from map controls Also includes a quick fix for high contrast mode. The buttons are separated by a horizontal line by default, styled as a border above the 2nd button. Both buttons use the border when focused in high contrast mode to provide a form of outline. This takes away the line dividing them, because you can only have one border. The buttons are still separated by a 1px margin so this just sets a background colour on the container to simulate the divider. --- app/assets/stylesheets/map.scss | 136 ++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 58 deletions(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index b512a719e..d42e464bd 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -51,72 +51,92 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // // Also introduces a black divider between the buttons, made with a pseudo element, appearing on // focus to mimic the 2-colour style GOVUK Frontend buttons and links have -.leaflet-bar a { +.leaflet-bar { + border-radius: 0; // Remove rounded corners - // Allow it to contain the absolutely positioned divider bar we show between buttons - // when one is focused - &:last-child { - position: relative; + @media (forced-colors: active) { + background: canvasText; } - // Hover style is darker background - &:hover { - background-color: $zoom-button-hover-colour; - } + a { - // Styles that apply if focused with or without the :hover or :active state - &:focus { - // When colours are overridden, for example when users have a dark mode, - // backgrounds and box-shadows disappear, so we need to ensure there's a - // transparent outline which will be set to a visible colour by the OS. - border: solid 2px transparent; - box-sizing: border-box; // make sure height includes the border - line-height: 26px; // subtract the border from the height (normally 30px) + &:first-child { + // Remove rounded corners + border-top-left-radius: 0; + border-top-right-radius: 0; + } - // The inline display box for the button text overlaps the button edge. - // This is only visible in high contrast mode because of the background colour being set. - @media (forced-colors: active) { - overflow: hidden; + &:last-child { + // Allow it to contain the absolutely positioned divider bar we show between buttons + // when one is focused + position: relative; + + // Remove rounded corners + border-bottom-left-radius: 0; + border-bottom-right-radius: 0; + } + + // Hover style is darker background + &:hover { + background-color: $zoom-button-hover-colour; + } + + // Styles that apply if focused with or without the :hover or :active state + &:focus { + // When colours are overridden, for example when users have a dark mode, + // backgrounds and box-shadows disappear, so we need to ensure there's a + // outline so it can be set to a visible colour by the OS. + border: solid 2px $govuk-focus-colour; + box-sizing: border-box; // make sure height includes the border + line-height: 26px; // subtract the border from the height (normally 30px) + + // The inline display box for the button text overlaps the button edge. + // This is only visible in high contrast mode because of the background colour being set. + @media (forced-colors: active) { + overflow: hidden; + } + } + + // Focused style sets the button to the focus colour + &:focus:not(:active):not(:hover) { + color: $govuk-focus-text-colour; + background-color: $govuk-focus-colour; + } + + // Retain the space between buttons now we're using the border in the focus style + &:first-child:focus { + margin-bottom: 1px; + } + + // 3px divider between buttons as a version of the underline from the GOVUK focus style + &:last-child:focus:not(:active):not(:hover):before, + &:first-child:focus:not(:active):not(:hover) + a:before { + content: ""; + position: absolute; + height: 2px; + background: $govuk-focus-text-colour; + + // hide for focus:hover state + @media (forced-colors: active) { + display: none; + } + } + + // Styles for the divider, specifc to the second button being focused + &:last-child:focus:not(:active):not(:hover):before { + left: -2px; // Subtract 2px left border + top: -3px; // Subtract 2px top border and position so it overlaps space above button + width: calc(100% + 4px); // Subtract borders + } + + // Styles for the divider, specifc to the first button being focused + &:first-child:focus:not(:active):not(:hover) + a:before { + left: 0px; // No borders on parent of :before when not focused + top: -1px; // No borders so just overlap space above parent button + width: 100%; } } - // Focused style sets the button to the focus colour - &:focus:not(:active):not(:hover) { - color: $govuk-focus-text-colour; - background-color: $govuk-focus-colour; - } - - // Retain the space between buttons now we're using the border in the focus style - &:first-child:focus { - margin-bottom: 1px; - } - - // 3px divider between buttons as a version of the underline from the GOVUK focus style - &:last-child:focus:not(:active):not(:hover):before, - &:first-child:focus:not(:active):not(:hover) + a:before { - content: ""; - position: absolute; - height: 2px; - background: $govuk-focus-text-colour; - - @media (forced-colors: active) { - background: canvasText; - } - } - - // Styles for the divider, specifc to the second button being focused - &:last-child:focus:not(:active):not(:hover):before { - left: -2px; // Subtract 2px left border - top: -3px; // Subtract 2px top border and position so it overlaps space above button - width: calc(100% + 4px); // Subtract borders - } - - // Styles for the divider, specifc to the first button being focused - &:first-child:focus:not(:active):not(:hover) + a:before { - left: 0px; // No borders on parent of :before when not focused - top: -1px; // No borders so just overlap space above parent button - width: 100%; - } } // Extra block to override specific LeafletJS rounded-corners when buttons are focused From 8827b8802e420405b45decb7d4ed1b938f0540fa Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 25 Aug 2021 20:39:26 +0100 Subject: [PATCH 03/17] Update comments about high contrast styles To address the points in this comment on the associated pull request: https://github.com/alphagov/notifications-admin/pull/3996/files/04de4ed865cd34caae31f7fef5f41730df8ec072#diff-6c10fd1eff57bed9c68dbad652255da627ac922a2f8aabf7b17a02a5dff9d099 --- app/assets/stylesheets/map.scss | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index d42e464bd..66da5291a 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -10,9 +10,10 @@ $govuk-focus-colour: #FFDD00; $zoom-button-colour: govuk-colour("white"); $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); -// The focus style is a 2-colour outline, made to match the GOVUK text input focus style. -// When colours are overridden, for example when users have a dark mode, box-shadows disappear, -// so the outline which is left as a single colour (defined by the OS, to replace 'transparent'). +// The focus style is a 2-colour outline, made to match the GOVUK text input focus style, using +// box-shadow. When colours are overridden, for example when users have Windows high contrast mode +// on, box-shadows disappear, so the outline which is left as a single colour (defined by the OS, +// to replace 'transparent'). // // Leaflet adds focus styles with JS, through inline styles. Because of their higher precedence // we need to mark our overrides with !important. @@ -83,9 +84,10 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // Styles that apply if focused with or without the :hover or :active state &:focus { - // When colours are overridden, for example when users have a dark mode, - // backgrounds and box-shadows disappear, so we need to ensure there's a - // outline so it can be set to a visible colour by the OS. + // Button focus is shown by a change in background colour. + // When colours are overridden, for example when users have Windows high + // contrast mode on, backgrounds disappear, so we need to ensure there's a + // visible outline so it can be set to a colour by the OS. border: solid 2px $govuk-focus-colour; box-sizing: border-box; // make sure height includes the border line-height: 26px; // subtract the border from the height (normally 30px) @@ -151,9 +153,10 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); .leaflet-control-attribution { & a { &:focus { - // When colours are overridden, for example when users have a dark mode, - // backgrounds and box-shadows disappear, so we need to ensure there's a - // transparent outline which will be set to a visible colour. + // Link focus is shown by a change in background colour. + // When colours are overridden, for example when users have Windows high + // contrast mode on, backgrounds disappear, so we need to ensure there's a + // visible outline so it can be set to a colour by the OS. outline: $govuk-focus-width solid transparent !important; // sass-lint:disable-line no-important color: $govuk-focus-text-colour; background: $govuk-focus-colour; From 4950b08d71dd0588fc75ca9334a8c8e5cec5e3f5 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 26 Aug 2021 11:51:44 +0100 Subject: [PATCH 04/17] Fix CSS that removes rounded corners on controls Users on devices/browsers that support touch have the rounded corners styles applied by this selector: .leaflet-touch .leaflet-bar a:first-child, .leaflet-touch .leaflet-bar a:last-child It has a higher precedence than the existing selector we use to override it so our overrides are ignored: .leaflet-bar a:first-child, .leaflet-bar a:last-child This changes the selector for our block of styles to include one matching the existing styles above so devices/browsers also get our styles. Note 1: this actually means some styles were no longer needed so this also removes them. Note 2: oddly, this was spotted in Firefox when displaying high contrast mode on desktop. So not a touch device but leaflet's JS is marking it as such. --- app/assets/stylesheets/map.scss | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index 66da5291a..c766fade3 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -61,20 +61,10 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); a { - &:first-child { - // Remove rounded corners - border-top-left-radius: 0; - border-top-right-radius: 0; - } - &:last-child { // Allow it to contain the absolutely positioned divider bar we show between buttons // when one is focused position: relative; - - // Remove rounded corners - border-bottom-left-radius: 0; - border-bottom-right-radius: 0; } // Hover style is darker background @@ -142,11 +132,21 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); } // Extra block to override specific LeafletJS rounded-corners when buttons are focused +.leaflet-bar a, .leaflet-touch .leaflet-bar a { - &:first-child:focus, - &:last-child:focus { - border-radius: initial; + + &:first-child { + // Remove rounded corners + border-top-left-radius: 0; + border-top-right-radius: 0; } + + &:last-child { + // Remove rounded corners + border-bottom-left-radius: 0; + border-bottom-right-radius: 0; + } + } // Map attribution links From 83bf78156bf1835ba4ce646a75a5b0008efb5cbb Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 26 Aug 2021 13:46:46 +0100 Subject: [PATCH 05/17] Rewrite comment on part of the focus style The original comment doesn't describe what the code does. Raised in this comment on the associated pull request: https://github.com/alphagov/notifications-admin/pull/3996#discussion_r692093945 --- app/assets/stylesheets/map.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index c766fade3..56c58f510 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -108,7 +108,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); height: 2px; background: $govuk-focus-text-colour; - // hide for focus:hover state + // hide in forced-color mode, where the normal focus state the divider is part of does not show @media (forced-colors: active) { display: none; } From 542be8832d42f8c44a63d071f13501a515590e4d Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 26 Aug 2021 15:42:12 +0100 Subject: [PATCH 06/17] Turn outlines off on map control buttons Leaflet does this anyway when they're focused (through JS) but we found holding shift when on a focused button, which you do when tabbing backwards, turns this off for some reason so you see the outline the browser applies by default. This turns all outlines off to stop that happening. Worth nothing that focus is indicated by: - a change of background colour instead when tabbing - a border in forced-color mode ...so the outline is not needed. --- app/assets/stylesheets/map.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index 56c58f510..49147fc6a 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -61,6 +61,9 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); a { + // Outlines don't work because of the buttons being so close together so turn off + outline: none; + &:last-child { // Allow it to contain the absolutely positioned divider bar we show between buttons // when one is focused From a3854e49b6bcc25c5effc5ac0f0ac31835628727 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 26 Aug 2021 16:35:13 +0100 Subject: [PATCH 07/17] Fix issue with jumping buttons The content of the map buttons jumped on Chrome and Safari when focused. It turns out this was because I was testing in Firefox which Leaflet had identified as having touch capability (and so added the .leaflet-touch class). Leaflet makes the buttons 30px rather than 26px for touch-capable devices/browsers so the jumping was down to the line-height being set for the wrong container height. This adds styles to give a different line-height when touch is available, to match the Leaflet styles. --- app/assets/stylesheets/map.scss | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index 49147fc6a..4defbef45 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -83,7 +83,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // visible outline so it can be set to a colour by the OS. border: solid 2px $govuk-focus-colour; box-sizing: border-box; // make sure height includes the border - line-height: 26px; // subtract the border from the height (normally 30px) + line-height: 22px; // subtract the border from the height (normally 26px) // The inline display box for the button text overlaps the button edge. // This is only visible in high contrast mode because of the background colour being set. @@ -152,6 +152,11 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); } +// buttons are 30px for touch-capable devices/browsers +.leaflet-touch .leaflet-bar a:focus { + line-height: 26px; // subtract the border from the height (normally 30px) +} + // Map attribution links .leaflet-control-attribution { & a { From 23f0bb096eaad083b405b5783aa27f153af4eefe Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 27 Aug 2021 11:18:56 +0100 Subject: [PATCH 08/17] Add accessible name and description to map The map is already in the tabbing order, so can be moved to by tabbing and by programs like screen readers or speech recognition, but it doesn't have an accessible name so when assistive tech' that requires this for identification gets the contents read out instead, which is confusing. This adds an accessible name, via aria-label, made out of the areas the alert targets. This also adds some help text, explaining how to use the map via aria-describedby. This is a pretty common pattern and is used in native UI like selectboxes where a range of commands are available to control the UI 'widget'. Using aria-describedby means the help text is not used every time the widget is focused but is available if the user gets stuck. For example, Voiceover announces it if the widget is focused but not interacted with for a period of time, or when a shortcut key is pressed. Finally, I also added a role of 'region' because when I tested with the NVDA screen reader, the accessible name wasn't announced but this fixed that. I think it's because the div isn't being recognised as having a role without it being set explicitly and is therefore ignored. --- .../partials/area-map-javascripts.html | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/app/templates/views/broadcast/partials/area-map-javascripts.html b/app/templates/views/broadcast/partials/area-map-javascripts.html index 5a401b8fd..75c2f5475 100644 --- a/app/templates/views/broadcast/partials/area-map-javascripts.html +++ b/app/templates/views/broadcast/partials/area-map-javascripts.html @@ -29,16 +29,51 @@ var mapElement = document.getElementById('area-list-map'); + var mapContainer = mapElement.parentNode; + var grandparent = mapContainer.parentNode; + var isInDetails = grandparent.className.indexOf('govuk-details') > -1; + var areas = document.querySelectorAll('.area-list > .area-list-item'); + var description = document.createElement('p'); + var descriptionText = 'Use the arrow keys to move the map. Use the buttons to zoom the map in or out'; + var labelPrefix = 'Map of the United Kingdom, showing the areas for'; + var trimRegExp = /^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g; + var details; + var label; + + function getStringFromAreas (areas) { + var areasLen = areas.length; + var areaStrings = []; + var idx; + + if (areasLen === 1) { return areas[0].textContent.replace(trimRegExp, ''); } + + for (idx = 0; idx < areasLen; idx++) { + areaStrings.push(areas[idx].textContent.replace(trimRegExp, '')); + } + + // always join last 2 areas with 'and', the rest with commas + return areaStrings.slice(0, areasLen - 1).join(', ') + ' and ' + areaStrings[areasLen - 1]; + }; + + description.appendChild( + document.createTextNode(descriptionText) + ); + description.setAttribute('id', 'area-list-map__description'); + description.className = 'govuk-visually-hidden'; + mapContainer.insertBefore(description, mapElement.nextSibling); // if element is inside a details element then to make the map render correctly we open the details element // and set up the map before closing the details map after - grandparent = mapElement.parentElement.parentElement; - if (grandparent.className.split(/\s+/).indexOf('govuk-details') > -1) { + if (isInDetails) { details = grandparent; details.open = true; } + label = labelPrefix + " " + getStringFromAreas(areas); mapElement.style.height = Math.max(320, window.innerHeight - mapElement.offsetTop - 100) + 'px'; + mapElement.setAttribute('role', 'region'); + mapElement.setAttribute('aria-label', label); + mapElement.setAttribute('aria-describedby', 'area-list-map__description'); var mymap = L.map( 'area-list-map', @@ -58,7 +93,7 @@ ); // if element is inside a details element then close the details element - if (details) { + if (isInDetails) { details.open = false; } From e2af2f63a41e48ce51fc4be3beb5893049e5d3b6 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 1 Sep 2021 20:26:12 +0100 Subject: [PATCH 09/17] Fix issues with JS added to map code This addresses the following issues with the JS: 1. fix string listing the alert areas so their 'remove' text isn't included 2. use `!==` instead of `>` for index comparison 3. put code for adding the label and description into separate functions Those issues are (matching the list above): 1. https://github.com/alphagov/notifications-admin/pull/3996#discussion_r699235376 2. https://github.com/alphagov/notifications-admin/pull/3996#discussion_r699220672 3. https://github.com/alphagov/notifications-admin/pull/3996#discussion_r699230038 --- .../partials/area-map-javascripts.html | 94 +++++++++++++------ 1 file changed, 63 insertions(+), 31 deletions(-) diff --git a/app/templates/views/broadcast/partials/area-map-javascripts.html b/app/templates/views/broadcast/partials/area-map-javascripts.html index 75c2f5475..79eabbde6 100644 --- a/app/templates/views/broadcast/partials/area-map-javascripts.html +++ b/app/templates/views/broadcast/partials/area-map-javascripts.html @@ -29,39 +29,13 @@ var mapElement = document.getElementById('area-list-map'); - var mapContainer = mapElement.parentNode; - var grandparent = mapContainer.parentNode; - var isInDetails = grandparent.className.indexOf('govuk-details') > -1; + var grandparent = mapElement.parentNode.parentNode; + var isInDetails = grandparent.className.indexOf('govuk-details') !== -1; var areas = document.querySelectorAll('.area-list > .area-list-item'); - var description = document.createElement('p'); - var descriptionText = 'Use the arrow keys to move the map. Use the buttons to zoom the map in or out'; - var labelPrefix = 'Map of the United Kingdom, showing the areas for'; var trimRegExp = /^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g; var details; var label; - function getStringFromAreas (areas) { - var areasLen = areas.length; - var areaStrings = []; - var idx; - - if (areasLen === 1) { return areas[0].textContent.replace(trimRegExp, ''); } - - for (idx = 0; idx < areasLen; idx++) { - areaStrings.push(areas[idx].textContent.replace(trimRegExp, '')); - } - - // always join last 2 areas with 'and', the rest with commas - return areaStrings.slice(0, areasLen - 1).join(', ') + ' and ' + areaStrings[areasLen - 1]; - }; - - description.appendChild( - document.createTextNode(descriptionText) - ); - description.setAttribute('id', 'area-list-map__description'); - description.className = 'govuk-visually-hidden'; - mapContainer.insertBefore(description, mapElement.nextSibling); - // if element is inside a details element then to make the map render correctly we open the details element // and set up the map before closing the details map after if (isInDetails) { @@ -69,11 +43,69 @@ details.open = true; } - label = labelPrefix + " " + getStringFromAreas(areas); + function addAriaLabel (mapElement) { + + function getLabelPrefix (areas) { + var labelPrefix = ['Map of the United Kingdom, showing the ', ' for']; + + if (areas.length === 1) { + return labelPrefix[0] + 'area' + labelPrefix[1]; + } else { + return labelPrefix[0] + 'areas' + labelPrefix[1]; + } + }; + + function getStringOfAreas (areas) { + var areasLen = areas.length; + var areaStrings = []; + var idx; + + function getAreaName (area) { + var areaString = ''; + var childNodesLen = area.childNodes.length; + var childNode; + + while (childNodesLen--) { + childNode = area.childNodes[childNodesLen]; + if (childNode.nodeType === 3) { areaString += childNode.nodeValue; } // only use text nodes + } + + return areaString.replace(trimRegExp, '') + }; + + if (areasLen === 1) { return getAreaName(areas[0]); } + + for (idx = 0; idx < areasLen; idx++) { + areaStrings.push(getAreaName(areas[idx])); + } + + // always join last 2 areas with 'and', the rest with commas + return areaStrings.slice(0, areasLen - 1).join(', ') + ' and ' + areaStrings[areasLen - 1]; + }; + + label = getLabelPrefix(areas) + " " + getStringOfAreas(areas); + mapElement.setAttribute('aria-label', label); + }; + + function addAriaDescription (mapElement) { + + var mapContainer = mapElement.parentNode; + var description = document.createElement('p'); + var descriptionText = 'Use the arrow keys to move the map. Use the buttons to zoom the map in or out'; + + description.appendChild( + document.createTextNode(descriptionText) + ); + description.setAttribute('id', 'area-list-map__description'); + description.className = 'govuk-visually-hidden'; + mapContainer.insertBefore(description, mapElement.nextSibling); + mapElement.setAttribute('aria-describedby', 'area-list-map__description'); + }; + mapElement.style.height = Math.max(320, window.innerHeight - mapElement.offsetTop - 100) + 'px'; mapElement.setAttribute('role', 'region'); - mapElement.setAttribute('aria-label', label); - mapElement.setAttribute('aria-describedby', 'area-list-map__description'); + addAriaLabel(mapElement); + addAriaDescription(mapElement); var mymap = L.map( 'area-list-map', From f52dd23e354a8a784e89041f895f00ba97f0fc3b Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 1 Sep 2021 20:38:06 +0100 Subject: [PATCH 10/17] Correct term in map CSS comment Co-authored-by: Ben Thorner --- app/assets/stylesheets/map.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index 4defbef45..c42e7bc0e 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -121,7 +121,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); &:last-child:focus:not(:active):not(:hover):before { left: -2px; // Subtract 2px left border top: -3px; // Subtract 2px top border and position so it overlaps space above button - width: calc(100% + 4px); // Subtract borders + width: calc(100% + 4px); // Compensate for borders } // Styles for the divider, specifc to the first button being focused From 9c9e7a7c61dda066d709f594afb763671238c2fd Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 2 Sep 2021 11:11:03 +0100 Subject: [PATCH 11/17] Fix code for map focus style @benthorner pointed out a few things about these styles that could do with changes: - the outline-offset will only appear when the outline does, which is in forced-color mode when the browser assigns a colour to it, so it doesn't need to be assigned in a media query targeting forced-color mode - the `&:focus:not(:focus-visible)` selector stops the focus styles showing in scenarios where: 1. the browser supports :focus-visible 2. focus comes from something other than tabbing to the map ...so we don't need to target :focus-visible specifically. This applies changes to these styles to remove those not needed and move some to a better place. Related to this comment on the associated pull request: https://github.com/alphagov/notifications-admin/pull/3996#discussion_r699246969 --- app/assets/stylesheets/map.scss | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index c42e7bc0e..78aa580e9 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -26,21 +26,16 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); &:focus { box-shadow: 0px 0px 0 3px $govuk-focus-text-colour, 0 0 0 6px $govuk-focus-colour; outline: solid 3px transparent !important; // sass-lint:disable-line no-important - } - - &:focus:not(:focus-visible) { - box-shadow: none; - } - - &:focus-visible { - box-shadow: 0px 0px 0 3px $govuk-focus-text-colour, 0 0 0 6px $govuk-focus-colour; - outline: solid 3px transparent !important; // sass-lint:disable-line no-important - // You only see the outline in forced colour mode which doesn't have enough contrast with the // map so this creates some space between them to mimic the two-colour default styling - @media (forced-colors: active) { - outline-offset: 3px; - } + outline-offset: 3px; + } + + // The map should only recieve focus from being tabbed to. This stops the focus state coming + // from other sources (like clicks on child elements). + &:focus:not(:focus-visible) { + box-shadow: none; + outline: none; } } From ee3e3c6c90d6b9e10957792f45a10061a941bacd Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 2 Sep 2021 11:59:39 +0100 Subject: [PATCH 12/17] Change how line between map buttons is styled There is a slight variance in how the line between the map buttons is rendered when in forced-colors mode and when not. This is not helped by it alternately being rendered as either: - a gap between buttons, showing the container colour - a border-bottom on the first button This attempts to flatten these styles so it is only styled as a gap between the buttons so changes to how its colour renders in different modes can just be dealt with on the container. --- app/assets/stylesheets/map.scss | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index 78aa580e9..e226cd521 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -49,6 +49,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // focus to mimic the 2-colour style GOVUK Frontend buttons and links have .leaflet-bar { border-radius: 0; // Remove rounded corners + background: #CCCCCC; @media (forced-colors: active) { background: canvasText; @@ -59,6 +60,11 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // Outlines don't work because of the buttons being so close together so turn off outline: none; + &:first-child { + border-bottom: none; + margin-bottom: 1px; + } + &:last-child { // Allow it to contain the absolutely positioned divider bar we show between buttons // when one is focused @@ -93,11 +99,6 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); background-color: $govuk-focus-colour; } - // Retain the space between buttons now we're using the border in the focus style - &:first-child:focus { - margin-bottom: 1px; - } - // 3px divider between buttons as a version of the underline from the GOVUK focus style &:last-child:focus:not(:active):not(:hover):before, &:first-child:focus:not(:active):not(:hover) + a:before { From 6de836b2f213d3688cf8ab3d89a2726d78a0eaa2 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 2 Sep 2021 12:26:55 +0100 Subject: [PATCH 13/17] Rewrite map CSS comments --- app/assets/stylesheets/map.scss | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index e226cd521..f0e8ad502 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -10,15 +10,14 @@ $govuk-focus-colour: #FFDD00; $zoom-button-colour: govuk-colour("white"); $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); -// The focus style is a 2-colour outline, made to match the GOVUK text input focus style, using +// The map focus style is a 2-colour outline, made to match the GOVUK text input focus style, using // box-shadow. When colours are overridden, for example when users have Windows high contrast mode -// on, box-shadows disappear, so the outline which is left as a single colour (defined by the OS, -// to replace 'transparent'). +// on, box-shadows disappear, so the outline is left as a single colour (defined by the OS, to +// replace 'transparent') which we offset from the map to mimic the 2-colour version. // // Leaflet adds focus styles with JS, through inline styles. Because of their higher precedence // we need to mark our overrides with !important. // -// This also uses :focus-visible to stop it showing focus when you click zoom in/out. // This approach, taken from MDN, allows it to fall back to :focus for browsers without support: // https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible#selectively_showing_the_focus_indicator .leaflet-container { @@ -26,8 +25,6 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); &:focus { box-shadow: 0px 0px 0 3px $govuk-focus-text-colour, 0 0 0 6px $govuk-focus-colour; outline: solid 3px transparent !important; // sass-lint:disable-line no-important - // You only see the outline in forced colour mode which doesn't have enough contrast with the - // map so this creates some space between them to mimic the two-colour default styling outline-offset: 3px; } @@ -43,10 +40,11 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // https://design-system.service.gov.uk/components/button/ // // GOVUK buttons have an invisible outline, used for high contrast modes. Our buttons are cropped by -// their container so we use a border instead. +// their container so we use their border instead. // -// Also introduces a black divider between the buttons, made with a pseudo element, appearing on -// focus to mimic the 2-colour style GOVUK Frontend buttons and links have +// Also introduces a thick black divider between the buttons, made with a pseudo element, appearing +// on focus. Together with the yellow background, this copies the 2-colour style GOVUK Frontend +// buttons and links have. .leaflet-bar { border-radius: 0; // Remove rounded corners background: #CCCCCC; @@ -66,7 +64,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); } &:last-child { - // Allow it to contain the absolutely positioned divider bar we show between buttons + // Allow it to contain the absolutely positioned divider we show between buttons // when one is focused position: relative; } @@ -99,7 +97,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); background-color: $govuk-focus-colour; } - // 3px divider between buttons as a version of the underline from the GOVUK focus style + // Shared styles for the divider between buttons that appears when either are focused &:last-child:focus:not(:active):not(:hover):before, &:first-child:focus:not(:active):not(:hover) + a:before { content: ""; From 7c2f4adfd5433a171654af62ed279d11a62503dc Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 2 Sep 2021 13:52:37 +0100 Subject: [PATCH 14/17] Refactor JS Based on these comments on the associated pull request: - add area/areas condition to the array used to build the label prefix https://github.com/alphagov/notifications-admin/commit/e2af2f63a41e48ce51fc4be3beb5893049e5d3b6#r55831534 - use a for loop instead of while when looping through nodes https://github.com/alphagov/notifications-admin/commit/e2af2f63a41e48ce51fc4be3beb5893049e5d3b6#r55831693 --- .../partials/area-map-javascripts.html | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/app/templates/views/broadcast/partials/area-map-javascripts.html b/app/templates/views/broadcast/partials/area-map-javascripts.html index 79eabbde6..fdb7af5f7 100644 --- a/app/templates/views/broadcast/partials/area-map-javascripts.html +++ b/app/templates/views/broadcast/partials/area-map-javascripts.html @@ -46,13 +46,11 @@ function addAriaLabel (mapElement) { function getLabelPrefix (areas) { - var labelPrefix = ['Map of the United Kingdom, showing the ', ' for']; - - if (areas.length === 1) { - return labelPrefix[0] + 'area' + labelPrefix[1]; - } else { - return labelPrefix[0] + 'areas' + labelPrefix[1]; - } + return [ + 'Map of the United Kingdom, showing the ', + (areas.length === 1) ? 'area' : 'areas', + ' for' + ].join(''); }; function getStringOfAreas (areas) { @@ -63,10 +61,11 @@ function getAreaName (area) { var areaString = ''; var childNodesLen = area.childNodes.length; + var idx; var childNode; - while (childNodesLen--) { - childNode = area.childNodes[childNodesLen]; + for (idx = 0; idx < childNodesLen; idx++) { + childNode = area.childNodes[idx]; if (childNode.nodeType === 3) { areaString += childNode.nodeValue; } // only use text nodes } From c1c80802e21448ba2ae6089093d9a00cd11d14ab Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 3 Sep 2021 10:55:06 +0100 Subject: [PATCH 15/17] Give outline reset same precedence as for focus The CSS that cancelled outline on focus events not fired by the :focus-visible heuristic is being overridden by the higher precedence of the outline style for :focus, due to its use of !important. This adds !important to the cancelling CSS. This brings that block up to the same level as that for :focus, meaning the :focus-visible styles will win because they sit lower in the stylesheet. --- app/assets/stylesheets/map.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index f0e8ad502..b03dabeb8 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -32,7 +32,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // from other sources (like clicks on child elements). &:focus:not(:focus-visible) { box-shadow: none; - outline: none; + outline: none !important; // sass-lint:disable-line no-important } } From 87674aef085ca7cf9df75d412076a57781ade36b Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 3 Sep 2021 11:54:06 +0100 Subject: [PATCH 16/17] Fixes for the comments in the map CSS Based on a range of comments made in the associated pull request: - https://github.com/alphagov/notifications-admin/pull/3996/files/7c2f4adfd5433a171654af62ed279d11a62503dc#r701234728 - https://github.com/alphagov/notifications-admin/pull/3996/files/7c2f4adfd5433a171654af62ed279d11a62503dc#r701235330 - https://github.com/alphagov/notifications-admin/pull/3996/files/7c2f4adfd5433a171654af62ed279d11a62503dc#r701235586 - https://github.com/alphagov/notifications-admin/pull/3996/files/7c2f4adfd5433a171654af62ed279d11a62503dc#r701242035 - https://github.com/alphagov/notifications-admin/pull/3996/files/7c2f4adfd5433a171654af62ed279d11a62503dc#r701241659 Co-authored-by: Ben Thorner --- app/assets/stylesheets/map.scss | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index b03dabeb8..64bbe7fd6 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -39,7 +39,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // Overrides for zoom controls to make them match GOVUK buttons // https://design-system.service.gov.uk/components/button/ // -// GOVUK buttons have an invisible outline, used for high contrast modes. Our buttons are cropped by +// GOVUK buttons usually have an outline that shows up in high contrast modes. Our buttons are cropped by // their container so we use their border instead. // // Also introduces a thick black divider between the buttons, made with a pseudo element, appearing @@ -47,7 +47,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // buttons and links have. .leaflet-bar { border-radius: 0; // Remove rounded corners - background: #CCCCCC; + background: #CCCCCC; // Grey background to give a thin divider between buttons when they're not in focus @media (forced-colors: active) { background: canvasText; @@ -60,7 +60,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); &:first-child { border-bottom: none; - margin-bottom: 1px; + margin-bottom: 1px; // Thin divider to distinguish buttons } &:last-child { @@ -114,14 +114,14 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); // Styles for the divider, specifc to the second button being focused &:last-child:focus:not(:active):not(:hover):before { left: -2px; // Subtract 2px left border - top: -3px; // Subtract 2px top border and position so it overlaps space above button + top: -3px; // Shift divider up 1px so it overlaps buttons equally, and another 2px to compensate for the reduced height of the last button (due to the border) width: calc(100% + 4px); // Compensate for borders } // Styles for the divider, specifc to the first button being focused &:first-child:focus:not(:active):not(:hover) + a:before { left: 0px; // No borders on parent of :before when not focused - top: -1px; // No borders so just overlap space above parent button + top: -1px; // Shift divider up 1px so it overlaps buttons equally width: 100%; } } From c6ecbf647a475f520868c97f905fa60b4ddbb288 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 3 Sep 2021 12:30:29 +0100 Subject: [PATCH 17/17] Change comment about turning part of focus style off Co-authored-by: Ben Thorner --- app/assets/stylesheets/map.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/map.scss b/app/assets/stylesheets/map.scss index 64bbe7fd6..6fd80c125 100644 --- a/app/assets/stylesheets/map.scss +++ b/app/assets/stylesheets/map.scss @@ -105,7 +105,7 @@ $zoom-button-hover-colour: govuk-shade($zoom-button-colour, 10%); height: 2px; background: $govuk-focus-text-colour; - // hide in forced-color mode, where the normal focus state the divider is part of does not show + // Hide in forced-color mode, since the button outlines suffice @media (forced-colors: active) { display: none; }