In HTML you generally can’t nest an inline level element inside a block
level one, if you want your HTML to validate.
There were a couple of places where we were using a `<span>` as a
containing element:
- inside every table cell (think we inherited this from Digital
Marketplace)
- in the ‘pill’ navigation component for the selected tab
This meant that when we put components like big number inside these,
the resulting HTML was invalid, because big number is built with a bunch
of `<div>`s, which are block level.
This commit removes the use of a `<span>` tag in these places, and
replaces it with a `<div>`. Nesting block level elements in fine in
HTML.
Currently it’s not possible for a screen reader user to know which
financial year they’re looking at. From the accessibility report:
> The financial year links are contained in a navigation region -
> tabbing or arrowing through only reads out the links, not the main
> information of "2016 to 2017 financial year" - that information is
> vital for understanding the page content.
This problem also applies to other pages which use the `pill` component,
which is effectively tabbed navigation (that reloads the page rather
than showing or hiding content on the page).
There are specific ARIA attributes that can be used to mark up a
navigation as being tabbed. This commit:
- adds those attributes
- makes the selected ‘tab’ visible to screenreaders and keyboard
focusable
- adds a visual focus indicator to the selected tab
- adds `id`s to the parts of the page that are controlled by the tabs so
that they are labelled as such
This also means changing the pill component from being a `<nav>` to a
`<ul>` because `tablist` is not a valid `role` for a `nav`.
Mostly follows the example here:
http://accessibility.athena-ict.com/aria/examples/tabpanel2.shtml
It’s invalid HTML to have multiple labels nested within each other. This
was happening by accident because WTForms tries to be clever – when you
put `{{ field.label }}` in a template it prints a `<label>` tag for you,
not just the text of the label. But we put our own `<label>` tags in the
HTML to have more control of them.
This commit stops WTForms being so clever.
The HTML validator picks up this error in our code:
> Self-closing syntax (/>) used on a non-void HTML element. Ignoring
> the slash and treating as a start tag.
We do our own server-side validation of things like email address. We
don’t want the browser also trying to do it based on the input type of a
form field. It’s bad because the browser validation message comes up as
a nasty little tooltip under the field.
The heading structure of most pages is incorrect (`<h2>` followed by
`<h1>`). The `<h1>` indicates the main purpose of the page, the service
name (currently the first `<h2>`) doesn't need to be a heading.
In pages specific to a service (e.g. dashboard and sub pages) the title
needs to distinguish which service it applies to. This is mainly to give
context to screen reader users who could be managing multiple services.
Implementing this uses template inheritance:
`page_title` includes `per_page_title` includes `service_page_title`
‘GOV.UK Notify’ is inserted into every page title.
Pages that set `service_page_title` get the service name inserted too.
This is mainly for the ‘check’ page where we show users the contents of
their spreadsheet.
> The structure of the table means that the first cell is treated as a
> column header, so moving through columns in row 2 for example
> associates the data 2 with 1 (column header) - this has no logical
> meaning
> If both sections of the page have errors and the page is submitted,
> focus moves to the mobile numbers section so screen reader users may
> not be aware of preceding errors - focus should move to a dedicated
> error summary at the top of the page.
Right now we use Javascript to focus the first error on a page (if any
errors are found). This commit adds more JS to then focus the error
summary, if there is one on the page. So this is where the focus will
rest.
It also makes some modifications to the ‘dangerous’ banner to make it
focusable, and to visually indicate that it is focused.
> If both sections of the page have errors and the page is submitted,
> focus moves to the mobile numbers section so screen reader users may
> not be aware of preceding errors - focus should move to a dedicated
> error summary at the top of the page.
This commit adds the dedicate error summary at the top of the page,
following the GOV.UK Elements style from:
http://govuk-elements.herokuapp.com/errors/
Currently screen reader users would click the ‘Copy API key’ button but
not get any feedback. This commit adds an ARIA attribute which tells the
screenreader to announce any changes in to the content of the element
(eg when it changes from showing the API to showing the text ‘API key
copied to clipboard’).
The accessibility audit raised the issue that screen reader users could
miss the table of data on the preview page, because it’s after the
submit/back buttons.
> The back button is before the table of error messages - a screen
> reader user might read the initial error summary then get to the back
> button and not realise the error detail are later in the sequence.
> The send and back buttons are before the table of field values - a
> screen reader user might read the template details ror summary then
> get to the buttons and not realise the field details are later in the
> sequence.
This commit add a skip link to navigate the users directly to the table,
which:
- allows them to skip past a lot of content which they might already
have read
- makes them aware that the table exists
It’s added:
– as the first thing after the `<h1>` when there are no errors with the
file
- as the last thing in the error summary when there are errors with the
file
The link is hidden from those interacting with the site visually.
We’ve seen services that have a lot of activity on their dashboard
miss the usage feature because the link gets pushed a long way down the
page.
We don’t want to move it up the page, because for most users the
templates/jobs stuff is more useful. And we don’t want to remove it
because it’s a useful part of the onboarding to understand the Notify
proposition.
So this commit adds it as a link to the nav, to make users more aware of
it, and as a quick way of getting into it for the small subset of users
who will care about it (a lot).
Order of the links is determined by what’s likely to be most useful for
first time users surfing the nav. Usage is more interesting than
settings, but less interesting than team members.
Matches what we do on the usage page. No need to see months in the
future because there’s no way you’ll have sent any messages in those
months, unless you’re Marty McFly.
The previous, weekly activity breakdown was what we reckoned might be
useful. But now that we have people using the platform it feels like
aggregating a service’s usage by month is:
- matches the timeframe users report on within their organisation
- is consistent with the usage page
And like the usage page this commit also limits the page to only show
one financial year’s worth of data at once (rather than data for all
time).
This commit also makes some changes to the jobs view code so that our
aggregation of failure states is consistent between the dashboard pages
and the jobs pages.
Right now we tell people that the usage page is for the current
financial year. This is a lie – it’s for all time.
So this commit calls through to the API to get the stats for (by
default) the current financial year.
We already do this for the monthly breakdown, this just does the same
thing for the yearly totals.
It also adds navigation to show the data for other financial years:
- previous so you can go back and see your usage and verify that the
bill you’re about to pay is correct
- next so that you can check what your SMS allowance is going to be
before you actually get into it
`severe` can mean one of three things:
- `yes` – user has told us this is an emergency
- `no` – user has told us this isn’t an emergency
- Anything else – user hasn’t been asked the question or has
hacked/mangled the URL
This commit adds some stricter sanitisation of the `severe` query
parameter and does so up front, rather than spreading it across multiple
functions.
`replace` doesn’t convert a time from one timezone to another. It just
changes the label that says what timezone a time is in 😬
`.localize` is how we handle these kind of issues in the API (see
d0b467b2fb/app/utils.py (L42-L44) )
So this commit changes the calculation to use `.localize`, and makes the
tests timezone aware to check we’re doing this right.
Using and/or over any/all has a couple of advantages:
- it's a bit quicker
- it won't evaluate the second half at all if the first half fails – if
it is in business hours, and convert_to_boolean would raise, with your
use of all we'd throw a 500, whereas if we had or, business_hours
would trip and we'd skip over the second half without worrying about
exceptions
any and all are designed for use with variable length args eg
`any(x for x in thing())`
If you report a problem we want to be able to get back to you to find
out more information, or to update you on the status of a fix. So it
shouldn’t be possible to report a problem without providing an email
address.
This commit makes `email_address` a required field when `ticket_type` is
problem.
This requires a bit of fiddling with the tests which weren’t expecting
to have to provide an email address. So the tests now either:
- pass an email address
- check for an error when they don’t pass an email address
This is a real edge case, but it seems worth handling.
How you’d get to this case:
- it’s 5:29pm and you start to describe the problem you’re having
- it’s 5:31pm and you click ‘submit’
- you’re redirected to the triage page because we’re now out of hours
- you click ‘this is a serious problem’
What would be bad thing to happen:
- you’re back on the message page and all the stuff you’ve written is
gone
What would be a good thing to happen:
- we save the message in a session so that you can check it again before
sending it
Generally I prefer confirmation pages to the flash message thing
(they’re harder to miss). So this commit adds one.
It also adds some logic to this page, so that, depending what the user
has told us about the thing they’ve submitted, we can tell them how
quickly to expect a response.
TL;DR, as much as possible we should work out how to prioritise tickets
and not put that burden on the user. However, there are some cases where
we can’t.
In business hours all tickets are high priority, ie we will at least
acknowledge them within 30 mins.
If we are not in business hours then we need to know if a ticket is
serious enough to get someone out of bed. Only the user can tell us
this, but we can give them some examples to help them decide.
In addition, out-of-hours tickets are only a priority if the user has
live services. Normally we can determine this and do the
priority-setting in the background.
If they can’t log in then we can’t determine what services they have. So
in this case they will need to use the emergency email address, which
only users with live services will have.
The logic for this gets fairly complex. It might be to easier to
understand what’s going on by walking through the test cases, which are
a bit more declarative.
N.B. Deskpro’s ‘urgency’ is descending, eg 10 is the most urgent and 1
is the least.
