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.
Based on these comments on the associated pull
request:
- add area/areas condition to the array used to
build the label prefix
e2af2f63a4 (r55831534)
- use a for loop instead of while when looping
through nodes
e2af2f63a4 (r55831693)
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.
@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
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.
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.
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.
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.
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.
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.
Resolves: https://github.com/alphagov/notifications-admin/pull/3980#discussion_r692919874
Previously it was unclear what kinds of areas this method returned,
and whether there would be any duplicates (due to the hierarchy of
areas we work with). This clarifies that.
In addition, the areas returned may not overlap with the custom one
[1], so we should reword to avoid falsely implying that. We could do
the overlap check as part of the method as an alternative, but that
would create extra work when calculating the ratio of intersection.
We could always add "overlapping areas" as a complementary method to
this one in future.
[1]: https://github.com/alphagov/notifications-admin/pull/3980#discussion_r692919874
Previously we were passing a flag to the API which handled this. Now
we are doing it at the time of clicking the link, not at the time of
storing the new password. We don’t need to update the timestamp twice,
so this commit removes the code which tells the API to do it.
Accepting an invite means that you’ve just clicked a link in your email
inbox. This shows that you have access to your email.
We can make a record of this, thereby extending the time before we ask
you to revalidate your email address.
When someone uses a fresh password reset link they have proved that they
have access to their inbox.
At the moment, when revalidating a user’s email address we wait until
after they’ve put in the 2FA code before updating the timestamp which
records when they last validated their email address[1].
We can’t think of a good reason that we need the extra assurance of a
valid 2FA code to assert that the user has access to their email –
they’ve done that just by clicking the link. When the user clicks the
link we already update their failed login count before they 2fa. Think
it makes sense to handle `email_access_validated_at` then too.
As a bonus, the functional tests never go as far as getting a 2FA code
after a password reset[2], so the functional test user never gets its
timestamp updated. This causes the functional tests start failing after
90 days. By moving the update to this point we ensure that the
functional tests will keep passing indefinitely.
1. This code in the API (91542ad33e/app/dao/users_dao.py (L131))
which is called by this code in the admin app (9ba37249a4/app/utils/login.py (L26))
2. 5837eb01dc/tests/functional/preview_and_dev/test_email_auth.py (L43-L46)
We had an audit in February of this year but did
not update the accessibility statement to reflect
the issues identified as fixed or to include new
issues it produced.
Some of the dates for fixed have also not been
updated for a long time.
This adds those changes, with placeholders for
dates assigned to each issue.
This content is now ready for review. The dates
will be assigned when that is complete.
Theoretically the maximum expiry time of a broadcast should be 24 hours.
If it goes over 24 hours there can be problems.
However we want to make it more conservative to mitigate two potential
issues:
1. The CBC has a repetition period (eg 60 seconds) and a count (eg
1,440). If these were slightly innaccurate or generous it could take
us over 24 hours. For this reason we should give ourselves half an
hour of buffer.
2. It’s possibly that the CBC could interpret a UTC time as BST or vice
versa. Until we’re sure that it’s using UTC everywhere, we need to
remove another whole hour as buffer.
In total this means we remove 1 hour 30 minutes from 24 hours, giving an
expiry time of 22 hours 30 minutes.
While testing alerts on these channels the MNOs sometimes need to
restart their CBCs to make sure everything is failing over properly.
If the CBC does not come back up, for whatever reason, then we are left
in a state where the alert can’t be cancelled.
To minimise the impact to the public in this scenario we should keep the
expiry time at 4 hours for alerts sent on test channels. We recently
increased it back up to 24 hours for all channels, so this in effect is
reverting that change for channels that won’t be used in a real
emergency.
