From 9d1a7904a87bf7e1c37c0ee3acbe3a1de8ccb39c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 21 May 2019 15:44:27 +0100 Subject: [PATCH] =?UTF-8?q?Fix=20duplicated=20H1=20on=20=E2=80=98New=20let?= =?UTF-8?q?ter=20branding=E2=80=99=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For accessibility reasons a page should have one (and only one) H1. This commit fixes an instance where the H1 was duplicated as a result of the work done to componentize our page headings. It also adds an extra check to `client_request` so that we don’t introduce pages with multiple or no H1s in the future. --- app/templates/components/folder-path.html | 7 ++++--- .../letter-branding/manage-letter-branding.html | 1 - app/templates/views/templates/choose-reply.html | 2 +- tests/app/main/views/test_send.py | 12 ++++++------ tests/conftest.py | 3 +++ 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/templates/components/folder-path.html b/app/templates/components/folder-path.html index 9c6e86065..b183c84ea 100644 --- a/app/templates/components/folder-path.html +++ b/app/templates/components/folder-path.html @@ -3,9 +3,10 @@ service_id, template_type, current_user, - link_current_item=False + link_current_item=False, + root_element='h1' ) %} -

+ <{{ root_element }} class="heading-medium folder-heading"> {% for folder in folders %} {% if loop.last and not link_current_item %} {% if folder.template_type or not folder.id %} @@ -26,7 +27,7 @@ {% if not loop.last %}{{ folder_path_separator() }}{% endif %} {% endif %} {% endfor %} -

+ {% endmacro %} diff --git a/app/templates/views/letter-branding/manage-letter-branding.html b/app/templates/views/letter-branding/manage-letter-branding.html index 2ece938e6..772bdb129 100644 --- a/app/templates/views/letter-branding/manage-letter-branding.html +++ b/app/templates/views/letter-branding/manage-letter-branding.html @@ -11,7 +11,6 @@ {% block platform_admin_content %} -

{{ '{} letter branding'.format('Update' if is_update else 'Add')}}

{{ page_header( '{} letter branding'.format('Update' if is_update else 'Add'), back_link=url_for('main.letter_branding') diff --git a/app/templates/views/templates/choose-reply.html b/app/templates/views/templates/choose-reply.html index caf762338..c9946b26e 100644 --- a/app/templates/views/templates/choose-reply.html +++ b/app/templates/views/templates/choose-reply.html @@ -14,7 +14,7 @@

Choose a template

- {{ folder_path(template_folder_path, current_service.id, template_type, current_user) }} + {{ folder_path(template_folder_path, current_service.id, template_type, current_user, root_element='h2') }}
{% if not templates_and_folders.templates_to_show %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 331d667b2..5aa7a764d 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -998,7 +998,7 @@ def test_send_test_step_redirects_if_session_not_setup( ): mocker.patch('app.user_api_client.get_user', return_value=user(fake_uuid)) template_mock(mocker) - mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=9) with client_request.session_transaction() as session: assert 'recipient' not in session @@ -1109,7 +1109,7 @@ def test_send_one_off_or_test_has_correct_page_titles( ): mocker.patch('app.user_api_client.get_user', return_value=user(fake_uuid)) template_mock(mocker) - mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=9) response = logged_in_client.get( partial_url(service_id=service_one['id'], template_id=fake_uuid, step_index=0), @@ -1224,7 +1224,7 @@ def test_send_one_off_has_skip_link( ): mocker.patch('app.user_api_client.get_user', return_value=user(fake_uuid)) template_mock(mocker) - mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=9) page = client_request.get( 'main.send_one_off_step', @@ -1261,7 +1261,7 @@ def test_send_one_off_has_sticky_header_for_email_and_letter( expected_sticky, ): template_mock(mocker) - mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=9) page = client_request.get( 'main.send_one_off_step', @@ -1774,7 +1774,7 @@ def test_send_test_caches_page_count( fake_uuid, ): - mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=99) + mocker.patch('app.main.views.send.get_page_count_for_letter', return_value=9) logged_in_client.get( url_for( @@ -1785,7 +1785,7 @@ def test_send_test_caches_page_count( follow_redirects=True, ) with logged_in_client.session_transaction() as session: - assert session['send_test_letter_page_count'] == 99 + assert session['send_test_letter_page_count'] == 9 def test_send_test_indicates_optional_address_columns( diff --git a/tests/conftest.py b/tests/conftest.py index b46b7d3b8..84a9d5da4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2908,6 +2908,9 @@ def client_request( assert resp.location == _expected_redirect page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser') if _test_page_title: + count_of_h1s = len(page.select('h1')) + if count_of_h1s != 1: + raise AssertionError('Page should have one H1 ({} found)'.format(count_of_h1s)) page_title, h1 = ( normalize_spaces(page.find(selector).text) for selector in ('title', 'h1') )