From edd2a04c7a7efcc22c34f1f40cc85466148d27f9 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 4 Nov 2021 11:28:31 +0000
Subject: [PATCH 1/3] Make autofocus work on all form elements which ask for it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In 674c27a693e901a9faa90b4f5e271fbb595bbcea we updated the autofocus
Javascript to be compatible with GOV.UK Frontend textboxes, which have
the `data-module` attribute set on the `input` element, rather than on
a wrapper element.

However we still have some `<textarea>`s and `<input>`s which haven’t
moved to GOV.UK Frontend and therefore aren’t getting picked up by the
Javascript which is supposed to focus them.

This commit makes the Javascript work with both kinds of textbox, which
is needed until we move entirely to GOV.UK Frontend.
---
 app/assets/javascripts/autofocus.js | 11 ++++++++++-
 tests/javascripts/autofocus.test.js | 14 +++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/autofocus.js b/app/assets/javascripts/autofocus.js
index 97267b32e..b688a9f20 100644
--- a/app/assets/javascripts/autofocus.js
+++ b/app/assets/javascripts/autofocus.js
@@ -10,7 +10,16 @@
       // is still where users intend to start
       if (($(window).scrollTop() > 0) && !forceFocus) { return; }
 
-      $component.filter('input, textarea, select').eq(0).trigger('focus');
+      // See if the component itself is something we want to send focus to
+      var target = $component.filter('input, textarea, select');
+
+      // Otherwise look inside the component to see if there are any elements
+      // we want to send focus to
+      if (target.length === 0) {
+        target = $('input, textarea, select', $component);
+      }
+
+      target.eq(0).trigger('focus');
 
     };
   };
diff --git a/tests/javascripts/autofocus.test.js b/tests/javascripts/autofocus.test.js
index 04b207ca5..3617ca1d9 100644
--- a/tests/javascripts/autofocus.test.js
+++ b/tests/javascripts/autofocus.test.js
@@ -20,7 +20,7 @@ describe('Autofocus', () => {
 
     // set up DOM
     document.body.innerHTML =
-      `<div>
+      `<div id="wrapper">
         <label class="form-label" for="search">
           ${labelText}
         </label>
@@ -50,6 +50,18 @@ describe('Autofocus', () => {
 
   });
 
+  test('is focused when attribute is set on outer element', () => {
+
+    document.getElementById('search').removeAttribute('data-module');
+    document.getElementById('wrapper').setAttribute('data-module', 'autofocus');
+
+    // start module
+    window.GOVUK.modules.start();
+
+    expect(focusHandler).toHaveBeenCalled();
+
+  });
+
   test('is not focused if the window has scrolled', () => {
 
     // mock the window being scrolled 25px

From e3089af1effdb3679503cb0a05ec101e70904f01 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 4 Nov 2021 11:28:31 +0000
Subject: [PATCH 2/3] Add test coverage for configuration of autofocus

This commits adds test coverage for ther HTML in several of the forms
which had broken autofocus.

It means that if we make changes to the HTML which triggers autofocus in
the future it should be more obvious that something is depending on the
attributes being added/removed.
---
 .../test_organisation_invites.py              | 45 +++++++++++++++++++
 tests/app/main/views/test_manage_users.py     | 43 ++++++++++++++++++
 tests/app/main/views/test_send.py             |  8 +++-
 tests/app/main/views/test_tour.py             | 30 +++++++++++++
 4 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/tests/app/main/views/organisations/test_organisation_invites.py b/tests/app/main/views/organisations/test_organisation_invites.py
index b07d6ad5c..87ca79bf6 100644
--- a/tests/app/main/views/organisations/test_organisation_invites.py
+++ b/tests/app/main/views/organisations/test_organisation_invites.py
@@ -33,6 +33,51 @@ def test_view_team_members(
     ) == 'Cancel invitation'
 
 
+@pytest.mark.parametrize('number_of_users', (
+    pytest.param(7, marks=pytest.mark.xfail),
+    pytest.param(8),
+))
+def test_should_show_live_search_if_more_than_7_users(
+    client_request,
+    mocker,
+    mock_get_organisation,
+    active_user_with_permissions,
+    number_of_users,
+):
+    mocker.patch(
+        'app.models.user.OrganisationInvitedUsers.client_method',
+        return_value=[],
+    )
+    mocker.patch(
+        'app.models.user.OrganisationUsers.client_method',
+        return_value=[active_user_with_permissions] * number_of_users,
+    )
+
+    page = client_request.get(
+        '.manage_org_users',
+        org_id=ORGANISATION_ID,
+    )
+
+    assert page.select_one('div[data-module=live-search]')['data-targets'] == (
+        ".user-list-item"
+    )
+    assert len(page.select('.user-list-item')) == number_of_users
+
+    textbox = page.select_one('[data-module=autofocus] .govuk-input')
+    assert 'value' not in textbox
+    assert textbox['name'] == 'search'
+    # data-module=autofocus is set on a containing element so it
+    # shouldn’t also be set on the textbox itself
+    assert 'data-module' not in textbox
+    assert not page.select_one('[data-force-focus]')
+    assert textbox['class'] == [
+        'govuk-input', 'govuk-!-width-full',
+    ]
+    assert normalize_spaces(
+        page.select_one('label[for=search]').text
+    ) == 'Search by name or email address'
+
+
 def test_invite_org_user(
     client_request,
     mocker,
diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py
index 4fb368642..14005bc28 100644
--- a/tests/app/main/views/test_manage_users.py
+++ b/tests/app/main/views/test_manage_users.py
@@ -151,6 +151,49 @@ def test_should_show_overview_page(
     mock_get_users.assert_called_once_with(SERVICE_ONE_ID)
 
 
+@pytest.mark.parametrize('number_of_users', (
+    pytest.param(7, marks=pytest.mark.xfail),
+    pytest.param(8),
+))
+def test_should_show_live_search_if_more_than_7_users(
+    client_request,
+    mocker,
+    mock_get_invites_for_service,
+    mock_get_template_folders,
+    mock_has_no_jobs,
+    active_user_with_permissions,
+    active_user_view_permissions,
+    number_of_users,
+):
+    mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+    mocker.patch('app.models.user.InvitedUsers.client_method', return_value=[])
+    mocker.patch(
+        'app.models.user.Users.client_method',
+        return_value=[active_user_with_permissions] * number_of_users
+    )
+
+    page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
+
+    assert page.select_one('div[data-module=live-search]')['data-targets'] == (
+        ".user-list-item"
+    )
+    assert len(page.select('.user-list-item')) == number_of_users
+
+    textbox = page.select_one('[data-module=autofocus] .govuk-input')
+    assert 'value' not in textbox
+    assert textbox['name'] == 'search'
+    # data-module=autofocus is set on a containing element so it
+    # shouldn’t also be set on the textbox itself
+    assert 'data-module' not in textbox
+    assert not page.select_one('[data-force-focus]')
+    assert textbox['class'] == [
+        'govuk-input', 'govuk-!-width-full',
+    ]
+    assert normalize_spaces(
+        page.select_one('label[for=search]').text
+    ) == 'Search by name or email address'
+
+
 def test_should_show_caseworker_on_overview_page(
     client_request,
     mocker,
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index 4e7f6c32f..bf9e3a094 100644
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -2253,8 +2253,14 @@ def test_send_one_off_letter_address_shows_form(
 
     form = page.select_one('form')
 
+    assert form['data-module'] == 'autofocus'
+    assert form['data-force-focus'] == 'True'
+
     assert form.select_one('label').text.strip() == 'Address'
-    assert form.select_one('textarea').attrs['name'] == 'address'
+    assert form.select_one('textarea')['name'] == 'address'
+    assert form.select_one('textarea')['data-module'] == 'enhanced-textbox'
+    assert form.select_one('textarea')['data-highlight-placeholders'] == 'false'
+    assert form.select_one('textarea')['rows'] == '4'
 
     upload_link = form.select_one('a')
 
diff --git a/tests/app/main/views/test_tour.py b/tests/app/main/views/test_tour.py
index 5453bb475..643f74e41 100644
--- a/tests/app/main/views/test_tour.py
+++ b/tests/app/main/views/test_tour.py
@@ -164,6 +164,36 @@ def test_should_200_for_get_tour_step(
     )
 
 
+def test_should_show_empty_text_box(
+    client_request,
+    mock_get_service_template_with_multiple_placeholders,
+    service_one,
+    fake_uuid,
+):
+    with client_request.session_transaction() as session:
+        session['placeholders'] = {'phone number': '07700 900762'}
+
+    page = client_request.get(
+        'main.tour_step',
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+        step_index=1
+    )
+
+    textbox = page.select_one('[data-module=autofocus][data-force-focus=True] .govuk-input')
+    assert 'value' not in textbox
+    assert textbox['name'] == 'placeholder_value'
+    assert textbox['class'] == [
+        'govuk-input', 'govuk-!-width-full',
+    ]
+    # data-module=autofocus is set on a containing element so it
+    # shouldn’t also be set on the textbox itself
+    assert 'data-module' not in textbox
+    assert normalize_spaces(
+        page.select_one('label[for=placeholder_value]').text
+    ) == 'one'
+
+
 def test_should_prefill_answers_for_get_tour_step(
     client_request,
     mock_get_service_template_with_multiple_placeholders,

From 538bc63f9802f3f7017bad93503b10b87fcb3eda Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 4 Nov 2021 11:52:26 +0000
Subject: [PATCH 3/3] Autofocus old and new error messages

Previously all our error messages had the class `error-message`.

Where we are using the components from GOV.UK Frontend they have the
class `govuk-error-message`.

This makes the code which jumps focus to the first error work in both
cases.
---
 app/assets/javascripts/main.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js
index c255d9652..a6cb2ccba 100644
--- a/app/assets/javascripts/main.js
+++ b/app/assets/javascripts/main.js
@@ -17,7 +17,7 @@ showHideContent.init();
 
 $(() => GOVUK.modules.start());
 
-$(() => $('.error-message').eq(0).parent('label').next('input').trigger('focus'));
+$(() => $('.error-message, .govuk-error-message').eq(0).parent('label').next('input').trigger('focus'));
 
 $(() => $('.banner-dangerous').eq(0).trigger('focus'));