From 32c36bf70bc29a1d29d38ef7274c5fce49aabb20 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 19 Nov 2018 16:52:21 +0000 Subject: [PATCH] Make the empty folder state more useful MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently if there’s nothing in a folder you just get an empty page. This looks a bit broken, or like the page hasn’t finished loading. This commit adds a message to the page to show that it’s intentionally blank. The message is contextual based on type of template, because there might be templates in the current folder, even if you can’t see them at the moment (because you’re filtering). --- .../stylesheets/components/message.scss | 5 ++ app/models/service.py | 5 +- .../views/templates/_template_list.html | 90 ++++++++++--------- app/templates/views/templates/choose.html | 14 ++- tests/app/main/views/test_template_folders.py | 32 ++++++- 5 files changed, 98 insertions(+), 48 deletions(-) diff --git a/app/assets/stylesheets/components/message.scss b/app/assets/stylesheets/components/message.scss index 7b46c6cd3..ec44dd8a7 100644 --- a/app/assets/stylesheets/components/message.scss +++ b/app/assets/stylesheets/components/message.scss @@ -54,6 +54,11 @@ } + &-empty { + color: $secondary-text-colour; + padding: $gutter-half 0 $gutter-one-third 0; + } + } .folder-heading { diff --git a/app/models/service.py b/app/models/service.py index dff5916e6..7192b9e30 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -133,7 +133,10 @@ class Service(): @property def has_templates(self): - return len(self.all_templates) > 0 + return bool(self.all_templates) + + def has_folders(self): + return bool(self.all_template_folders) @property def has_multiple_template_types(self): diff --git a/app/templates/views/templates/_template_list.html b/app/templates/views/templates/_template_list.html index d92627d76..cf41d3d31 100644 --- a/app/templates/views/templates/_template_list.html +++ b/app/templates/views/templates/_template_list.html @@ -1,44 +1,50 @@ {% from "components/checkbox.html" import unlabelled_checkbox %} -{% from "components/message-count-label.html" import folder_contents_count %} +{% from "components/message-count-label.html" import folder_contents_count, message_count_label %} - +{% if service_has_templates_or_folders and not templates and not template_folders %} +

+ No {{ message_count_label(1, template_type, suffix='') }} templates in this folder +

+{% else %} + +{% endif %} diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html index 4d3e6877e..4e8145c1c 100644 --- a/app/templates/views/templates/choose.html +++ b/app/templates/views/templates/choose.html @@ -77,7 +77,12 @@ {% if can_manage_folders %}
- {% with templates=templates, template_folders=template_folders %} + {% with + templates=templates, + template_folders=template_folders, + service_has_templates_or_folders=(current_service.has_templates or current_service.has_folders), + template_type=template_type + %} {% include 'views/templates/_template_list.html' %} {% endwith %} {% with templates=templates, template_folders=template_folders, templates_and_folders_form=templates_and_folders_form %} @@ -85,7 +90,12 @@ {% endwith %}
{% else %} - {% with templates=templates, template_folders=template_folders %} + {% with + templates=templates, + template_folders=template_folders, + service_has_templates_or_folders=(current_service.has_templates or current_service.has_folders), + template_type=template_type + %} {% include 'views/templates/_template_list.html' %} {% endwith %} {% endif %} diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 506c80ef3..44ba3c543 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -8,6 +8,7 @@ from tests.conftest import SERVICE_ONE_ID, normalize_spaces PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0' CHILD_FOLDER_ID = '92ee1ee0-e4ee-4dcc-b1a7-a5da9ebcfa2b' GRANDCHILD_FOLDER_ID = 'fafe723f-1d39-4a10-865f-e551e03d8886' +FOLDER_TWO_ID = 'bbbb222b-2b22-2b22-222b-b222b22b2222' def _folder(name, folder_id=None, parent=None): @@ -105,7 +106,8 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare 'expected_parent_link_args,' 'extra_args,' 'expected_nav_links,' - 'expected_items' + 'expected_items, ' + 'expected_empty_message ' ), [ ( @@ -123,7 +125,8 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare 'email_template_two Email template', 'letter_template_one Letter template', 'letter_template_two Letter template', - ] + ], + None, ), ( 'Templates – service one – GOV.UK Notify', @@ -136,6 +139,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare 'sms_template_one Text message template', 'sms_template_two Text message template', ], + None, ), ( 'folder_one – Templates – service one – GOV.UK Notify', @@ -147,6 +151,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare 'folder_one_one 1 template, 1 folder', 'folder_one_two Empty', ], + None, ), ( 'folder_one – Templates – service one – GOV.UK Notify', @@ -157,6 +162,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare [ 'folder_one_one 1 folder', ], + None, ), ( 'folder_one – Templates – service one – GOV.UK Notify', @@ -165,6 +171,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare {'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID}, ['All', 'Text message', 'Letter'], [], + 'No email templates in this folder', ), ( 'folder_one_one – folder_one – service one – GOV.UK Notify', @@ -176,6 +183,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare 'folder_one_one_one 1 template', 'letter_template_nested Letter template', ], + None, ), ( 'folder_one_one_one – folder_one_one – service one – GOV.UK Notify', @@ -186,6 +194,16 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare [ 'sms_template_nested Text message template', ], + None, + ), + ( + 'folder_two – Templates – service one – GOV.UK Notify', + 'Templates / folder_two', + {'template_type': 'all'}, + {'template_folder_id': FOLDER_TWO_ID}, + ['Text message', 'Email', 'Letter'], + [], + 'No templates in this folder', ), ] ) @@ -202,9 +220,10 @@ def test_should_show_templates_folder_page( extra_args, expected_nav_links, expected_items, + expected_empty_message, ): mock_get_template_folders.return_value = [ - _folder('folder_two'), + _folder('folder_two', FOLDER_TWO_ID), _folder('folder_one', PARENT_FOLDER_ID), _folder('folder_one_two', parent=PARENT_FOLDER_ID), _folder('folder_one_one', CHILD_FOLDER_ID, parent=PARENT_FOLDER_ID), @@ -260,6 +279,13 @@ def test_should_show_templates_folder_page( for index, expected_item in enumerate(expected_items): assert normalize_spaces(page_items[index].text) == expected_item + if expected_empty_message: + assert normalize_spaces(page.select_one('.template-list-empty').text) == ( + expected_empty_message + ) + else: + assert not page.select('.template-list-empty') + mock_get_service_templates.assert_called_once_with(SERVICE_ONE_ID)