From d49622c5dc4c7c9108667a8bb9c0c4aacdd00280 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 16 Nov 2018 13:41:55 +0000 Subject: [PATCH] Style folder hierarchy in headings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the display of folders in the `

` look like the prototype. It alters the behaviour we’ve initially built here by only ever showing a maximum of two levels of hierarchy (the current folders and its parent). --- .../stylesheets/components/message.scss | 56 +++++++++++++++++++ app/models/service.py | 23 ++++---- app/templates/components/folder-path.html | 50 +++++++++++++++++ app/templates/views/templates/choose.html | 34 +++++------ tests/app/main/views/test_template_folders.py | 47 ++++++++++++++-- 5 files changed, 176 insertions(+), 34 deletions(-) create mode 100644 app/templates/components/folder-path.html diff --git a/app/assets/stylesheets/components/message.scss b/app/assets/stylesheets/components/message.scss index 55267d554..2e9b52180 100644 --- a/app/assets/stylesheets/components/message.scss +++ b/app/assets/stylesheets/components/message.scss @@ -55,3 +55,59 @@ } } + +.folder-heading { + + .column-main>.grid-row:first-child &.heading-medium { + margin-top: 18px; + } + + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + + + a, + &-folder { + + display: inline-block; + vertical-align: top; + padding: 5px 0 0 60px; + background-repeat: no-repeat; + background-size: 38px auto; + background-position: 0 1px; + + } + + a { + + background-image: file-url('folder-blue.svg'); + max-width: 33%; + overflow: hidden; + text-overflow: ellipsis; + + } + + &-folder { + + padding-left: 0; + + &:first-child { + background-image: file-url('folder-black.svg'); + padding-left: $gutter * 2; + } + + } + + &-separator { + + display: inline-block; + vertical-align: top; + color: $secondary-text-colour; + padding: 5px 4px 0 5px; + font-weight: normal; + + } + + +} diff --git a/app/models/service.py b/app/models/service.py index 32de656ba..d91a12e5e 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -316,6 +316,12 @@ class Service(): ] def get_template_folder(self, folder_id): + if folder_id is None: + return { + 'id': None, + 'name': 'Templates', + 'parent_id': None, + } return self._get_by_id(self.all_template_folders, folder_id) def is_folder_visible(self, template_folder_id, template_type='all'): @@ -335,19 +341,16 @@ class Service(): return False def get_template_folder_path(self, template_folder_id): - if template_folder_id is None: - return [] - id_to_folder = {folder['id']: folder for folder in self.all_template_folders} + folder = self.get_template_folder(template_folder_id) - folder = id_to_folder[template_folder_id] - path = [folder] + if folder['id'] is None: + return [folder] - while folder['parent_id']: - folder = id_to_folder[folder['parent_id']] - path.append(folder) - - return list(reversed(path)) + return [ + self.get_template_folder(folder['parent_id']), + folder, + ] def get_template_folders_and_templates(self, template_type, template_folder_id): return ( diff --git a/app/templates/components/folder-path.html b/app/templates/components/folder-path.html new file mode 100644 index 000000000..ef747046b --- /dev/null +++ b/app/templates/components/folder-path.html @@ -0,0 +1,50 @@ +{% macro folder_path( + folders, + service_id, + template_type, + fallback_page_title=None, + show_fallback_page_title=False +) %} + {% if show_fallback_page_title %} +

+ {{ fallback_page_title }} +

+ {% else %} +

+ {% for folder in folders %} + {% if loop.last %} + {{ folder.name }} + {% else %} + {% if folder.id %} + {{ folder.name }} {{ folder_path_separator() }} + {% else %} + Templates {{ folder_path_separator() }} + {% endif %} + {% endif %} + {% endfor %} +

+ {% endif %} +{% endmacro %} + + +{% macro page_title_folder_path( + folders, + fallback_page_title=None, + show_fallback_page_title=False +) %} + {% if show_fallback_page_title %} + {{ fallback_page_title }} + {% else %} + {% for folder in folders|reverse %} + {{ folder.name }} + {% if not loop.last %} + – + {% endif %} + {% endfor %} + {% endif %} +{% endmacro %} + + +{% macro folder_path_separator() %} + / +{% endmacro %} diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html index c3e519c6d..9b5e8fe6c 100644 --- a/app/templates/views/templates/choose.html +++ b/app/templates/views/templates/choose.html @@ -1,3 +1,4 @@ +{% from "components/folder-path.html" import folder_path, page_title_folder_path %} {% from "components/pill.html" import pill %} {% from "components/message-count-label.html" import message_count_label %} {% from "components/textbox.html" import textbox %} @@ -8,10 +9,11 @@ {% set page_title = 'Templates' %} {% block service_page_title %} - {{ page_title }} - {% for folder in template_folder_path %} - / {{ folder.name }} - {% endfor %} + {{ page_title_folder_path( + template_folder_path, + fallback_page_title=page_title, + show_fallback_page_title=not current_service.all_template_folders + ) }} {% endblock %} {% block maincolumn_content %} @@ -46,27 +48,19 @@
-

- {% if template_folder_path %} - {{ page_title }} - {% else %} - {{ page_title }} - {% endif %} - {% for folder in template_folder_path %} - / - {% if loop.last %} - {{ folder.name }} - {% else %} - {{ folder.name }} - {% endif %} - {% endfor %} -

+ {{ folder_path( + folders=template_folder_path, + service_id=current_service.id, + template_type=template_type, + fallback_page_title=page_title, + show_fallback_page_title=not current_service.all_template_folders + ) }}
{% if current_user.has_permissions('manage_templates') %}
Add new template - {% if can_manage_folders and template_folder_path %} + {% if can_manage_folders and current_template_folder_id %} Manage {% endif %}
diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 8fe03eb91..f1dd42de0 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -99,10 +99,19 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare @pytest.mark.parametrize( - 'expected_page_title, extra_args, expected_nav_links, expected_items', + ( + 'expected_title_tag,' + 'expected_page_title,' + 'expected_parent_link_args,' + 'extra_args,' + 'expected_nav_links,' + 'expected_items' + ), [ ( + 'Templates – service one – GOV.UK Notify', 'Templates', + None, {}, ['Text message', 'Email', 'Letter'], [ @@ -117,7 +126,9 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare ] ), ( + 'Templates – service one – GOV.UK Notify', 'Templates', + None, {'template_type': 'sms'}, ['All', 'Email', 'Letter'], [ @@ -127,7 +138,9 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare ], ), ( + 'folder_one – Templates – service one – GOV.UK Notify', 'Templates / folder_one', + {'template_type': 'all'}, {'template_folder_id': PARENT_FOLDER_ID}, ['Text message', 'Email', 'Letter'], [ @@ -136,7 +149,9 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare ], ), ( + 'folder_one – Templates – service one – GOV.UK Notify', 'Templates / folder_one', + {'template_type': 'sms'}, {'template_type': 'sms', 'template_folder_id': PARENT_FOLDER_ID}, ['All', 'Email', 'Letter'], [ @@ -144,13 +159,17 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare ], ), ( + 'folder_one – Templates – service one – GOV.UK Notify', 'Templates / folder_one', + {'template_type': 'email'}, {'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID}, ['All', 'Text message', 'Letter'], [], ), ( - 'Templates / folder_one / folder_one_one', + 'folder_one_one – folder_one – service one – GOV.UK Notify', + 'folder_one / folder_one_one', + {'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID}, {'template_folder_id': CHILD_FOLDER_ID}, ['Text message', 'Email', 'Letter'], [ @@ -159,7 +178,9 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare ], ), ( - 'Templates / folder_one / folder_one_one / folder_one_one_one', + 'folder_one_one_one – folder_one_one – service one – GOV.UK Notify', + 'folder_one_one / folder_one_one_one', + {'template_type': 'all', 'template_folder_id': CHILD_FOLDER_ID}, {'template_folder_id': GRANDCHILD_FOLDER_ID}, ['Text message', 'Email', 'Letter'], [ @@ -175,7 +196,9 @@ def test_should_show_templates_folder_page( service_one, mocker, fake_uuid, + expected_title_tag, expected_page_title, + expected_parent_link_args, extra_args, expected_nav_links, expected_items, @@ -206,11 +229,23 @@ def test_should_show_templates_folder_page( page = client_request.get( 'main.choose_template', service_id=SERVICE_ONE_ID, + _test_page_title=False, **extra_args ) + assert normalize_spaces(page.select_one('title').text) == expected_title_tag assert normalize_spaces(page.select_one('h1').text) == expected_page_title + if expected_parent_link_args: + assert len(page.select('h1 a')) == 1 + assert page.select_one('h1 a')['href'] == url_for( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + **expected_parent_link_args + ) + else: + assert page.select_one('h1 a') is None + links_in_page = page.select('.pill a') assert len(links_in_page) == len(expected_nav_links) @@ -312,7 +347,11 @@ def test_can_create_email_template_with_parent_folder( data['parent_folder_id']) -def test_get_manage_folder_page(client_request, service_one, mock_get_template_folders): +def test_get_manage_folder_page( + client_request, + service_one, + mock_get_template_folders, +): folder_id = str(uuid.uuid4()) mock_get_template_folders.return_value = [ {'id': folder_id, 'name': 'folder_two', 'parent_id': None},