From d2918486e995bbff091ead53a4acabc49376d989 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Feb 2017 12:08:20 +0000 Subject: [PATCH 1/2] Add error summary to whitelist page > 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/ --- app/templates/views/api/whitelist.html | 28 +++++++++++++++++++++--- tests/app/main/views/test_api_keys.py | 30 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/app/templates/views/api/whitelist.html b/app/templates/views/api/whitelist.html index 3e7c25b6e..441c1b00c 100644 --- a/app/templates/views/api/whitelist.html +++ b/app/templates/views/api/whitelist.html @@ -1,4 +1,5 @@ {% extends "withnav_template.html" %} +{% from "components/banner.html" import banner_wrapper %} {% from "components/table.html" import list_table, field, hidden_field_heading %} {% from "components/api-key.html" import api_key %} {% from "components/page-footer.html" import page_footer %} @@ -10,9 +11,30 @@ {% block maincolumn_content %} -

- Whitelist -

+ {% if form.email_addresses.errors or form.phone_numbers.errors %} + {% call banner_wrapper(type='dangerous') %} +

+ There was a problem with your whitelist +

+

Fix these errors:

+ + {% endcall %} + {% else %} +

+ Whitelist +

+ {% endif %}

You and members of diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index 38c1100ee..a3c809e67 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/tests/app/main/views/test_api_keys.py @@ -318,3 +318,33 @@ def test_should_update_whitelist( mock_update_whitelist.assert_called_once_with(service_id, { 'email_addresses': ['test@example.com', 'test@example.com'], 'phone_numbers': ['07900900000']}) + + +def test_should_validate_whitelist_items( + logged_in_client, + mock_login, + api_user_active, + mock_get_service, + mock_has_permissions, + mock_update_whitelist +): + + response = logged_in_client.post( + url_for('main.whitelist', service_id=str(uuid.uuid4())), + data=OrderedDict([ + ('email_addresses-1', 'abc'), + ('phone_numbers-0', '123') + ]) + ) + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == 'There was a problem with your whitelist' + jump_links = page.select('.banner-dangerous a') + + assert jump_links[0].string.strip() == 'Enter valid email addresses' + assert jump_links[0]['href'] == '#email_addresses' + + assert jump_links[1].string.strip() == 'Enter valid phone numbers' + assert jump_links[1]['href'] == '#phone_numbers' + + mock_update_whitelist.assert_not_called() From cd7cb8884cac9e56cb66efeaa85dd9ced92a5ddb Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Feb 2017 12:11:32 +0000 Subject: [PATCH 2/2] Send focus to error summary before single error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > 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. --- app/assets/javascripts/main.js | 2 ++ app/assets/stylesheets/components/banner.scss | 4 ++++ app/templates/components/banner.html | 8 +++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 24fe38039..a6153dcbc 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -5,3 +5,5 @@ $(() => GOVUK.modules.start()); $(() => new GOVUK.SelectionButtons('.block-label input, .sms-message-option input')); $(() => $('.error-message').eq(0).parent('label').next('input').trigger('focus')); + +$(() => $('.banner-dangerous').eq(0).trigger('focus')); diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index 5c40f11ec..9aded8655 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -54,6 +54,10 @@ margin: 15px 0; text-align: left; + &:focus { + outline: 3px solid $yellow; + } + .button { @include button($error-colour); margin-top: 10px; diff --git a/app/templates/components/banner.html b/app/templates/components/banner.html index d008b5b57..a21984b25 100644 --- a/app/templates/components/banner.html +++ b/app/templates/components/banner.html @@ -1,5 +1,11 @@ {% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None) %} -

+
{% if subhead -%} {{ subhead }}  {%- endif -%}