From 1365265fc6b0b6aba08e5eac83cb71187ff17896 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 10 Mar 2017 16:45:08 +0000 Subject: [PATCH 01/19] Make choose template a list of template names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a team has lots of templates the choose template page gets very long. It gets hard to find the template that you are looking for. Our initial reckon was that teams would not be giving their templates very useful names, and therefore a preview would be helpful. What we have found is that: - teams actually do give their templates useful names, and refer to these template names elsewhere - the previews are less useful for emails and text messages, because they have so much content (which for emails also makes it harder to `ctrl` + `f` the template name) The other problem we found was that this page presented the user with a _lot_ of options. For each template there were 4 actions, plus the click-to-preview action for letters, plus the ‘see previous version’ action for templates that had been edited multiple times. It was a very busy page. And the final problem (that we recently introduced) was that there was no way, other than the visual cues, to know whether a template was a letter, email, or text message. So this commit strips back the choose template page to be very focused on finding the right template, by only showing the template name and type. The user can then click through to a page that shows just a single template, and perform actions relevant to that template from that page. --- app/main/views/templates.py | 10 +- app/templates/views/templates/choose.html | 23 ++-- tests/app/main/views/test_send.py | 126 ---------------------- 3 files changed, 15 insertions(+), 144 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index c02ad9421..7c6cdab40 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -44,15 +44,7 @@ page_headings = { def choose_template(service_id): return render_template( 'views/templates/choose.html', - templates=[ - get_template( - template, - current_service, - letter_preview_url=url_for('.view_template', service_id=service_id, template_id=template['id']), - ) - for template in service_api_client.get_service_templates(service_id)['data'] - if should_show_template(template['template_type']) - ], + templates=service_api_client.get_service_templates(service_id)['data'] ) diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html index b20ab05fd..19bbe6ff5 100644 --- a/app/templates/views/templates/choose.html +++ b/app/templates/views/templates/choose.html @@ -1,3 +1,5 @@ +{% from "components/message-count-label.html" import message_count_label %} + {% extends "withnav_template.html" %} {% block service_page_title %} @@ -44,19 +46,22 @@ {% endif %} -
+
+ {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 70076b152..c08730192 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -664,132 +664,6 @@ def test_route_invalid_permissions( service_one) -def test_route_choose_template_manage_service_permissions( - mocker, - app_, - client, - api_user_active, - service_one, - mock_login, - mock_get_user, - mock_get_service, - mock_check_verify_code, - mock_get_service_templates, - mock_get_jobs, -): - template_id = mock_get_service_templates(service_one['id'])['data'][0]['id'] - resp = validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for( - 'main.choose_template', - service_id=service_one['id'], - ), - ['manage_users', 'manage_templates', 'manage_settings'], - api_user_active, - service_one) - page = resp.get_data(as_text=True) - assert url_for( - "main.send_messages", - service_id=service_one['id'], - template_id=template_id) not in page - assert url_for( - "main.send_test", - service_id=service_one['id'], - template_id=template_id) not in page - assert url_for( - "main.edit_service_template", - service_id=service_one['id'], - template_id=template_id) in page - - -def test_route_choose_template_send_messages_permissions( - mocker, - app_, - client, - active_user_with_permissions, - service_one, - mock_get_service, - mock_check_verify_code, - mock_get_service_templates, - mock_get_jobs, -): - template_id = None - for temp in mock_get_service_templates(service_one['id'])['data']: - if temp['template_type'] == 'sms': - template_id = temp['id'] - assert template_id - resp = validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for( - 'main.choose_template', - service_id=service_one['id'], - template_type='sms'), - ['send_texts', 'send_emails', 'send_letters'], - active_user_with_permissions, - service_one) - page = resp.get_data(as_text=True) - assert url_for( - "main.send_messages", - service_id=service_one['id'], - template_id=template_id) in page - assert url_for( - "main.edit_service_template", - service_id=service_one['id'], - template_id=template_id) not in page - - -def test_route_choose_template_manage_api_keys_permissions( - mocker, - app_, - client, - api_user_active, - service_one, - mock_get_user, - mock_get_service, - mock_check_verify_code, - mock_get_service_templates, - mock_get_jobs, -): - template_id = None - for temp in mock_get_service_templates(service_one['id'])['data']: - if temp['template_type'] == 'sms': - template_id = temp['id'] - assert template_id - resp = validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for( - 'main.choose_template', - service_id=service_one['id'], - template_type='sms'), - ['manage_api_keys'], - api_user_active, - service_one) - page = resp.get_data(as_text=True) - assert url_for( - "main.send_test", - service_id=service_one['id'], - template_id=template_id) not in page - assert url_for( - "main.edit_service_template", - service_id=service_one['id'], - template_id=template_id) not in page - page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser') - links = page.findAll('a', href=re.compile('^' + url_for( - "main.send_from_api", - service_id=service_one['id'], - template_id=template_id))) - assert len(links) == 1 - - @pytest.mark.parametrize( 'extra_args,expected_url', [ From f3c7a167df209b1a9d4493d818f1c26a9ebe1588 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 14 Mar 2017 10:46:38 +0000 Subject: [PATCH 02/19] Add find-as-you-type on the choose template page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not everyone knows how to use `ctrl` + `f`, and it’s not scoped to just the list of templates. The template you want to work with is often not the first one in the list, but ordering by created at is useful for other reasons (mainly around first time use). This commit adds a find as you type control which aims to give users a quick way of getting to the template they want to work with. --- app/assets/javascripts/liveSearch.js | 44 +++++++++++++++++++ .../stylesheets/components/live-search.scss | 16 +++++++ app/assets/stylesheets/main.scss | 1 + app/main/forms.py | 7 ++- app/main/views/templates.py | 11 ++++- app/templates/views/templates/choose.html | 12 ++++- gulpfile.babel.js | 1 + 7 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 app/assets/javascripts/liveSearch.js create mode 100644 app/assets/stylesheets/components/live-search.scss diff --git a/app/assets/javascripts/liveSearch.js b/app/assets/javascripts/liveSearch.js new file mode 100644 index 000000000..8f53f3f52 --- /dev/null +++ b/app/assets/javascripts/liveSearch.js @@ -0,0 +1,44 @@ +(function(Modules) { + "use strict"; + + let normalize = (string) => string.toLowerCase().replace(/ /g,''); + + let filter = ($searchBox, $targets) => () => { + + let query = normalize($searchBox.val()); + + $targets.each(function() { + + let content = $(this).text(); + + $(this).toggle( + normalize(content).indexOf(normalize(query)) > -1 + ); + + }); + + }; + + + Modules.LiveSearch = function() { + + this.start = function(component) { + + let $component = $(component); + + let $searchBox = $('input', $component); + + let filterFunc = filter( + $searchBox, + $($component.data('targets')) + ); + + $searchBox.on('keyup input', filterFunc); + + filterFunc(); + + }; + + }; + +})(window.GOVUK.Modules); diff --git a/app/assets/stylesheets/components/live-search.scss b/app/assets/stylesheets/components/live-search.scss new file mode 100644 index 000000000..1de4dac74 --- /dev/null +++ b/app/assets/stylesheets/components/live-search.scss @@ -0,0 +1,16 @@ +input[type=search] { + // overrides this nasty global from GOV.UK template + -moz-box-sizing: border-box; + -webkit-box-sizing: border-box; + box-sizing: border-box; +} + +.live-search { + + display: none; + + .js-enabled & { + display: block; + } + +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 56cc1caf8..f469fb257 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -58,6 +58,7 @@ $path: '/static/images/'; @import 'components/tick-cross'; @import 'components/list-entry'; @import 'components/letter'; +@import 'components/live-search'; @import 'components/vendor/breadcrumbs'; @import 'components/vendor/responsive-embed'; diff --git a/app/main/forms.py b/app/main/forms.py index 9ae203e1f..74c427370 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -22,7 +22,7 @@ from wtforms import ( FieldList, DateField, SelectField) -from wtforms.fields.html5 import EmailField, TelField +from wtforms.fields.html5 import EmailField, TelField, SearchField from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) from app.main.validators import (Blacklist, CsvFileValidator, ValidGovEmail, NoCommasInPlaceHolders, OnlyGSMCharacters) @@ -591,3 +591,8 @@ class ChooseTemplateType(Form): ('sms', 'Text message'), ('letter', 'Letter') if include_letters else None ]) + + +class SearchTemplatesForm(Form): + + search = SearchField('Search by name') diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 7c6cdab40..c78b7417c 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -13,7 +13,13 @@ from notifications_python_client.errors import HTTPError from app.main import main from app.utils import user_has_permissions, get_template, png_from_pdf -from app.main.forms import ChooseTemplateType, SMSTemplateForm, EmailTemplateForm, LetterTemplateForm +from app.main.forms import ( + ChooseTemplateType, + SMSTemplateForm, + EmailTemplateForm, + LetterTemplateForm, + SearchTemplatesForm, +) from app.main.views.send import get_example_csv_rows from app import service_api_client, current_service, template_statistics_client @@ -44,7 +50,8 @@ page_headings = { def choose_template(service_id): return render_template( 'views/templates/choose.html', - templates=service_api_client.get_service_templates(service_id)['data'] + templates=service_api_client.get_service_templates(service_id)['data'], + search_form=SearchTemplatesForm(), ) diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html index 19bbe6ff5..74d6099de 100644 --- a/app/templates/views/templates/choose.html +++ b/app/templates/views/templates/choose.html @@ -1,4 +1,5 @@ {% from "components/message-count-label.html" import message_count_label %} +{% from "components/textbox.html" import textbox %} {% extends "withnav_template.html" %} @@ -35,7 +36,7 @@ {% else %} -
+

Templates

@@ -46,7 +47,14 @@ {% endif %}
-