Merge pull request #2505 from alphagov/style-folder

Style folder hierarchy in headings
2026-08-12 18:08:57 -04:00 · 2018-11-19 15:21:57 +00:00
parent 754ae743ca d3e7557058
commit c9908befc2
9 changed files with 234 additions and 51 deletions
--- a/app/assets/stylesheets/components/message.scss
+++ b/app/assets/stylesheets/components/message.scss
@@ -55,3 +55,65 @@
  }

 }
+
+.folder-heading {
+
+  .column-main>.grid-row:first-child &.heading-medium {
+    margin-top: 18px;
+  }
+
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+
+
+  a:first-child,
+  &-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 {
+
+    padding-top: 5px;
+    display: inline-block;
+    vertical-align: top;
+
+    &:first-child {
+      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;
+
+  }
+
+
+}
--- 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,22 @@ 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 [
+            self.get_template_folder(folder['parent_id']),
+            folder,
+        ]

-        return list(reversed(path))
+    def get_template_path(self, template):
+        return [
+            self.get_template_folder(template['folder']),
+            template,
+        ]

    def get_template_folders_and_templates(self, template_type, template_folder_id):
        return (
--- a/app/templates/components/folder-path.html
+++ b/app/templates/components/folder-path.html
@@ -0,0 +1,51 @@
+{% macro folder_path(
+  folders,
+  service_id,
+  template_type,
+  fallback_page_title=None,
+  show_fallback_page_title=False,
+  link_current_item=False
+) %}
+  {% if show_fallback_page_title %}
+    <h1 class="heading-large">
+      {{ fallback_page_title }}
+    </h1>
+  {% else %}
+    <h1 class="heading-medium folder-heading">
+      {% for folder in folders %}
+        {% if loop.last and not link_current_item %}
+          <span class="folder-heading-folder">{{ folder.name }}</span>
+        {% else %}
+          {% if folder.id %}
+            <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type, template_folder_id=folder.id) }}">{{ folder.name }}</a> {% if not loop.last %}{{ folder_path_separator() }}{% endif %}
+          {% else %}
+            <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type) }}">Templates</a> {% if not loop.last %}{{ folder_path_separator() }}{% endif %}
+          {% endif %}
+        {% endif %}
+      {% endfor %}
+    </h1>
+  {% 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() %}
+  <span class="folder-heading-separator">/</span>
+{% endmacro %}
--- 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 %}
@@ -44,29 +46,21 @@

  {% else %}

-    <div class="grid-row bottom-gutter-2-3">
+    <div class="grid-row {% if current_service.all_template_folders %}bottom-gutter-1-2{% else %}bottom-gutter-2-3{% endif %}">
      <div class="column-two-thirds">
-        <h1 class="heading-large">
-          {% if template_folder_path %}
-            <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type) }}">{{ page_title }}</a>
-          {% else %}
-            {{ page_title }}
-          {% endif %}
-          {% for folder in template_folder_path %}
-            /
-            {% if loop.last %}
-              {{ folder.name }}
-            {% else %}
-              <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=folder.id) }}">{{ folder.name }}</a>
-            {% endif %}
-          {% endfor %}
-        </h1>
+        {{ 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
+        ) }}
      </div>
      {% if current_user.has_permissions('manage_templates') %}
        <div class="column-one-third">
          <a href="{{ url_for('.add_template_by_type', service_id=current_service.id, template_folder_id=current_template_folder_id) }}" class="button align-with-heading">Add new template</a>

-        {% if can_manage_folders and template_folder_path %}
+        {% if can_manage_folders and current_template_folder_id %}
          <a href="{{ url_for('.manage_template_folder', service_id=current_service.id, template_folder_id=current_template_folder_id) }}" class="align-with-heading">Manage</a>
        {% endif %}
        </div>
--- a/app/templates/views/templates/manage-template-folder.html
+++ b/app/templates/views/templates/manage-template-folder.html
@@ -1,26 +1,25 @@
 {% extends "withnav_template.html" %}
+{% from "components/folder-path.html" import folder_path, page_title_folder_path %}
 {% from "components/textbox.html" import textbox %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/form.html" import form_wrapper %}

 {% block service_page_title %}
-Templates
-{% for folder in template_folder_path %}
-  /
-    {{ folder.name }}
-{% endfor %}
-  - Manage folder
+  {{ page_title_folder_path(template_folder_path) }}
 {% endblock %}

 {% block maincolumn_content %}

-<h1 class="heading-large">
-  <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type) }}">Templates</a>
-  {% for folder in template_folder_path %}
-    /
-      <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=folder.id) }}">{{ folder.name }}</a>
-  {% endfor %}
-</h1>
+  <div class="grid-row bottom-gutter-1-2">
+    <div class="column-whole">
+      {{ folder_path(
+        folders=template_folder_path,
+        service_id=current_service.id,
+        template_type='all',
+        link_current_item=True
+      ) }}
+    </div>
+  </div>

  {% if not delete_folder %}
    {% call form_wrapper() %}
@@ -33,7 +32,6 @@ Templates
          template_folder_id=template_folder_id
        ),
        delete_link_text="Delete this folder") }}
-
    {% endcall %}
  {% else %}
    <a href="{{url_for('.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)}}">Back to manage folder page</a>
--- a/app/templates/views/templates/template.html
+++ b/app/templates/views/templates/template.html
@@ -1,5 +1,6 @@
 {% extends "withnav_template.html" %}
 {% from "components/banner.html" import banner_wrapper %}
+{% from "components/folder-path.html" import folder_path %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/textbox.html" import textbox %}
 {% from "components/api-key.html" import api_key %}
@@ -28,7 +29,17 @@
      {% endcall %}
    </div>
  {% else %}
-    <h1 class="heading-large">{{ template.name }}</h1>
+    <div class="grid-row bottom-gutter-1-2">
+      <div class="column-whole">
+        {{ folder_path(
+          folders=current_service.get_template_path(template._template),
+          service_id=current_service.id,
+          template_type='all',
+          fallback_page_title=template.name,
+          show_fallback_page_title=not current_service.all_template_folders
+        ) }}
+      </div>
+    </div>
  {% endif %}

  <div class="grid-row">
--- a/tests/init.py
+++ b/tests/init.py
@@ -229,6 +229,7 @@ def template_json(service_id,
        'reply_to': reply_to,
        'reply_to_text': reply_to_text,
        'is_precompiled_letter': is_precompiled_letter,
+        'folder': None,
    }
    if content is None:
        template['content'] = "template content"
--- 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},
@@ -322,7 +361,11 @@ def test_get_manage_folder_page(client_request, service_one, mock_get_template_f
    page = client_request.get(
        'main.manage_template_folder',
        service_id=service_one['id'],
-        template_folder_id=folder_id
+        template_folder_id=folder_id,
+        _test_page_title=False,
+    )
+    assert normalize_spaces(page.select_one('title').text) == (
+        'folder_two – Templates – service one – GOV.UK Notify'
    )
    assert page.select_one('input[name=name]') is not None
    delete_link = page.find('a', string="Delete this folder")
@@ -392,7 +435,8 @@ def test_delete_template_folder_should_request_confirmation(
    )
    page = client_request.get(
        'main.delete_template_folder', service_id=service_one['id'],
-        template_folder_id=folder_id
+        template_folder_id=folder_id,
+        _test_page_title=False,
    )
    assert normalize_spaces(page.select('.banner-dangerous')[0].text) == (
        'Are you sure you want to delete the ‘sacrifice’ folder? '
--- a/tests/app/main/views/test_templates.py
+++ b/tests/app/main/views/test_templates.py
@@ -492,6 +492,7 @@ def test_user_with_only_send_and_view_redirected_to_one_off(
 def test_user_with_only_send_and_view_sees_letter_page(
    client_request,
    mock_get_service_templates,
+    mock_get_template_folders,
    mock_get_service_letter_template,
    single_letter_contact_block,
    mock_has_jobs,
@@ -536,6 +537,7 @@ def test_user_with_only_send_and_view_sees_letter_page(
 def test_should_be_able_to_view_a_template_with_links(
    client,
    mock_get_service_template,
+    mock_get_template_folders,
    active_user_with_permissions,
    single_letter_contact_block,
    mocker,
@@ -575,6 +577,7 @@ def test_should_be_able_to_view_a_template_with_links(
 def test_should_show_template_id_on_template_page(
    logged_in_client,
    mock_get_service_template,
+    mock_get_template_folders,
    service_one,
    fake_uuid,
 ):
@@ -595,6 +598,7 @@ def test_should_show_sms_template_with_downgraded_unicode_characters(
    mocker,
    service_one,
    single_letter_contact_block,
+    mock_get_template_folders,
    fake_uuid,
 ):
    msg = 'here:\tare some “fancy quotes” and zero\u200Bwidth\u200Bspaces'
@@ -618,6 +622,7 @@ def test_should_show_sms_template_with_downgraded_unicode_characters(
 def test_should_let_letter_contact_block_be_changed_for_the_template(
    mocker,
    mock_get_service_letter_template,
+    mock_get_template_folders,
    no_letter_contact_blocks,
    client_request,
    service_one,
@@ -1264,6 +1269,7 @@ def test_should_redirect_when_saving_a_template_email(
 def test_should_show_delete_template_page_with_time_block(
    client_request,
    mock_get_service_template,
+    mock_get_template_folders,
    mocker,
    fake_uuid
 ):
@@ -1294,6 +1300,7 @@ def test_should_show_delete_template_page_with_time_block(
 def test_should_show_delete_template_page_with_time_block_for_empty_notification(
    client_request,
    mock_get_service_template,
+    mock_get_template_folders,
    mocker,
    fake_uuid
 ):
@@ -1323,6 +1330,7 @@ def test_should_show_delete_template_page_with_time_block_for_empty_notification
 def test_should_show_delete_template_page_with_never_used_block(
    client_request,
    mock_get_service_template,
+    mock_get_template_folders,
    fake_uuid,
    mocker,
 ):
@@ -1390,6 +1398,7 @@ def test_should_show_page_for_a_deleted_template(
    api_user_active,
    mock_login,
    mock_get_service,
+    mock_get_template_folders,
    mock_get_deleted_template,
    single_letter_contact_block,
    mock_get_user,
@@ -1430,6 +1439,7 @@ def test_route_permissions(
    api_user_active,
    service_one,
    mock_get_service_template,
+    mock_get_template_folders,
    mock_get_template_statistics_for_template,
    fake_uuid,
 ):
@@ -1713,6 +1723,7 @@ def test_should_show_message_before_redacting_template(
 def test_should_show_redact_template(
    client_request,
    mock_get_service_template,
+    mock_get_template_folders,
    mock_redact_template,
    single_letter_contact_block,
    service_one,
@@ -1737,6 +1748,7 @@ def test_should_show_hint_once_template_redacted(
    client_request,
    mocker,
    service_one,
+    mock_get_template_folders,
    fake_uuid,
 ):

@@ -1756,6 +1768,7 @@ def test_should_not_show_redaction_stuff_for_letters(
    mocker,
    fake_uuid,
    mock_get_service_letter_template,
+    mock_get_template_folders,
    single_letter_contact_block,
 ):