The fieldset that wraps the collapsible checkboxes
has an aria-describedby to make the summary its
accessible description.
This needs to point to the id of the summary but
the summary didn't have one.
These changes add the id and fix a fixture in the
tests for this module.
The fullscreenTable component has 2 layers to the
table you see onscreen:
1. the actual data table
2. a clone, with only the first column showing,
that sits on top so the row numbers stay in
place while you scroll
Table 1. has an id attribute on its caption. The
region wrapping it has an aria-describedby
attribute with the id as its value. This makes the
caption the description for the region.
This isn't needed for the clone and
makes the HTML invalid because ids should be
unique.
This removes the id from the cloned table.
Includes changing the code so that the radios
aren't split into two columns in the HTML present
when the page loads. This layout is now added by
the JS.
The last_dest_idx variable should always have been
tracking the last index in the source list. The
original intention, implemented incorrectly, was
to just append any items which source has no item
at that index.
A comment on the pull request for this branch
pointed out that it's not clear why the 'items'
list is deleted and then reassigned in
extend_params:
https://github.com/alphagov/notifications-admin/pull/3770#pullrequestreview-573067465
The simple reason is that we want to use
merge_jsonlike to merge params and
param_extensions (passed in as extensions) but
merge_jsonlike doesn't merge lists correctly.
I realised that if we just make merge_jsonlike
merge lists correctly, we can use it for
everything extend_params does.
This commit does that, and replaces all calls to
extend_params with merge_jsonlike.
Because extend_params is used across many form
field classes, and so many pages, I took the
following precautions after making those changes:
1. found every use of param_extensions
2. looked at the merges onto params that each would
cause and deduped them to a final list of 6(!)
3. tested pages containing fields from that list
4. added new testcases to the merge_jsonlike tests
for any merges that exist in our codebase but
not in our tests
Current behaviour is to check item-against-item
and merge based on whether items match, irrelevant
of position. This doesn't produce the results we
need for our usecases (merging data to send to
GOVUK Frontend components).
We actually want:
- items to be compared based on their position
- new primitive items at the same position to
overwrite existing ones
- dicts or lists at the same position to be merged
For example,
Starting with this list:
[{"name": "option-1", "value": "1"}]
Merging in this list:
[{"hint": {"text": "Choose one option"}}]
You currently get this:
[
{"name": "option-1", "value": "1"},
{"hint": {"text": "Choose one option"}}
]
We want to get this:
[
{
"name": "option-1", "value": "1",
"hint": {"text": "Choose one option"}
}
]
After talking with the reviewer, it was decided
that:
1. the JS could do with some comments to explain
its structure and what various functions do
better
2. some CSS selectors in the tests don't need to
be as complex and simplifying them makes the
test easier to read
Makes focus shift to the first time in the range
when you select a day.
Also rewrites the code for controlling focus so it
explains itself better, now it has different
settings.
All buttons that open or close a region of the
component should have aria-expanded attributes to
show:
- they have that control
- the state of the region
The OrganisationAgreementSignedForm class has a
bug causing it to render different HTML when the
page loads to when you subsequently refresh it.
This commit proposes a change to the extend_params
function to fix it.
extend_params, is used by the
OrganisationAgreementSignedForm, as well as all
the other WTForms field classes we added to wrap
GOVUK Frontend components. Fixing it should
therefore fix any similar bugs with them.
All of these fields send a dict of configuration
data to the GOVUK Frontend component when they
call it, at render time. This dict is 'JSON-like',
meaning it's values can be all the primitives as
well as lists and dicts. This also means it can go
quite deep.
Extending the default configuration
The classes have a default dict of this data kept
privately in the params variable. They let you
change it by passing in an argument called
param_extensions on instantiation, after that,
through an attribute of the same name and at
render time as the same argument (in templates).
The extend_params function
The param_extensions dict is used as a collection
of changes to make to the default params dict.
The changes are applied by the extend_params
function. Its code deletes part of the
param_extensions, a side effect that didn't seem a
problem because it isn't used after the function
has run.
The bug
The bug was only with the part of the HTML that
got its data from the part of the param_extensions
dict that was deleted by extend_params. The class
with the bug set param_extensions when the field
is instantiated, as part of its parent form
definition.
My guess is that param_extensions was stored in
memory, as part of the form class, and reused
when the page refreshed. At that point,
extend_params had deleted part of its data,
causing the bug.
We added an extra, hidden, <input> to our /sign-in
and /register forms to stop Chrome's form
heuristics filling the fields in wrong.
See the commit that added it:
https://github.com/alphagov/notifications-admin/commit/33b15cdec6ce4eb4ad70a5e0701a2e49466cf825
This removes it after testing with the following
Chrome/OS combinations and all working without the
hack:
- Chrome 87, Windows 7
- Chrome 86, Windows 7
- Chrome 86, Windows 10
- Chrome 85, Windows 7
- Chrome 80, Windows 10
- Chrome 81, Windows 7
- Chrome 52, Windows 7
- Chrome 68, Windows 7
- Chrome 78, Windows 8.1
- Chrome 86, Windows 8.1
These combinations were based on the most-used
versions recorded in our analytics for the last 3
months.
An accessiblity audit done as part of Notify's
service assessment raised the following problem
with our big_number component.
When you turn CSS off, the sentence in the
component is split onto separate lines.
This was because the number part is wrapped in a
<div> which browsers were interpreting as being a
separate sentence to the label.
So "1 letter", where "letter" is the label, was
seen as:
"1"
"letter"
The accessibility expert consulted on this pointed
out that this would sound confusing for users of
screen readers when moving through the document
sentence by sentence.
These changes:
- make the <div>s into <span>s which are 'phrasing
content' and so are interpreted as part of the
same sentence
- change the CSS so the number will still sit
on top of its label text
The HTML5 spec has a section on how browsers
should arrange text into paragraphs that explains
what was happening in more detail:
https://www.w3.org/TR/html52/dom.html#paragraphs
We will use this list in various views, to send
them through to the file_upload component.
These changes make it:
- into a Set so it can't be altered
- uppercase to show it is a constant
Safari has a bug where it stops input[type=file]
elements working if they don't specify the types
of file to accept (via the `accept` attribute).
It seems to just effect certain versions of Mojave
but completely blocks this action so worth fixing.
This adds a 'allowed_file_extensions' keyword
argument to the file_upload component to let you
specify a value to be passed to `accept`.
This was spotted on x-gov Slack:
https://ukgovernmentdigital.slack.com/archives/C06GCJW7R/p1607952390112800
...and StackOverflow:
https://stackoverflow.com/q/64843459/679924
Changes the selector the live search in the set
email and letter branding pages in service
settings and organisation settings. The current
one targeted the old radios HTML whereas this
version targets the same for the GOVUK Frontend
radios.
Includes a change to make these tests use the
getRadioGroup helper to reduce duplication across
the tests. This also makes a few changes to the
helper so it can produce the HTML required.
Effects all routes that use that form, or
SetLetterBranding, which inherits from it:
- /organisations/<service_id>/settings/set-letter-branding
- /organisations/<service_id>/settings/set-email-branding
- /<service_id>/service-settings/set-letter-branding
- /<service_id>/service-settings/set-email-branding
The previewPane JS used selectors that targeted
the old form of radios HTML.
The JS tests also contained selectors like this
and fragments of HTML, used for fixtures, modelled
on the old radios HTML.
Changes OrganisationCrownStatusForm.crown_status.
This also effects NewOrganisationForm, which
inherits from OrganisationCrownStatusForm.
Because of that this commit also updates the
template used for the edit org crown status page,
which uses NewOrganisationForm for its form.
Changes the OrganisationTypeField class used by
OrganisationOrganisationTypeForm.organisation_type
OrganisationTypeField is also used by the forms in
/add-service:
- CreateServiceForm
- CreateNhsServiceForm
Because of that, this commit also includes changes
to the template for that route.
Note: this also moves where OrganisationTypeField
appears in app/main/forms.py so it can use
GovukRadiosField.
Includes changing form.enabled to use
OnOffField, for consistency with other on/off
fields.
OnOffField's data is a boolean, not a string, so
some of the logic using it needed to be changed.
Uses the ServiceOnOffSettingForm.enabled.
Effects the following routes:
- /services/<service_id>/service-settings/switch-live
- /services/<service_id>/service-settings/switch-count-as-live
- /services/<service_id>/service-settings/permissions/<permission>
- /services/<service_id>/service-settings/permissions/broadcast
Includes changes to make the GOVUK radios
component render the h1 in the page. This is done
so the legend doesn't just duplicate the h1 and
is recommended in the design system guidance for
the radios component for pages that just ask you
for one piece of information.
https://design-system.service.gov.uk/components/radios/#how-it-works
Cloudfront, our CDN, sometimes decides not to gzip
assets. Because of this, we're going to gzip them
ourselves prior to upload instead.
This will involve:
1. adding gzipping to the make task that uploads
them
2. turning compression off in Cloudfront
There is already a pull request up for number 1:
https://github.com/alphagov/notifications-admin/pull/3733
Because deploying all this will, at some point,
create a state where Cloudfront is set to compress
assets that are already compressed, we need to
test that it doesn't re-compress them.
This adds a frontend build task that generates a
test asset which is:
- a copy of app/static/stylesheets/main.css
- renamed to include a MD5 SHA of its contents
- already gzipped
Once deployed, the test will be to:
1. download the asset from the live environment
2. unzip it
3. diff it against app/static/stylesheets/main.css
Our cookie banner has an accessible description
but no accessible name. It needs to have an
accessible name to distinguish it from other
elements on the page with role=region.
There's some advice in the WCAG guidance that
explains this more:
https://www.w3.org/TR/WCAG20-TECHS/ARIA11.html
This issue was raised in the accessibility review
done as part of our service assessment.
These changes borrow the approach from www.gov.uk of
moving to a label and losing the accessible
description. It also uses the 'cookie banner'
label for consistency with www.gov.uk.
Raised by the service assessment accessibility
review as a problem for the footer due to it
having a mix of text and links.
This also makes it match the latest GOVUK Frontend
style.
The checkboxes component that lets you choose
which users have permission to see the folder is
currently using the noun of 'folder' in the
summary it generates.
This is because this page users the
TemplateFolderForm class, which defaults the
permissions to 'folder'.
This updates the field_label attribute, used by
the code that generates the summary, to 'user' to
match the type of permission.
This is no longer happening with the NVDA screen
reader, which had the issue, when testing in
Outlook.
The hack added as a temporary solution has now
been removed. It went live on 19 November 2020 at
2.20pm.
Tom Byers