Improve display of folder path when deeply nested

It’s a bit rudimentary to only show the current place in the hierarchy and the parent. You lose a sense of how deep you are. But we can’t just show the full path, because it can be arbitrarily long. So what this commit does is show the full path, but truncates the display of any items. Further-up than the current folder or its parent. This also helps disambiguate between folders and templates, because folders are always shown with the folder icon. This probably won’t affect many teams, because we don’t anticipate a lot of deep nesting.
2026-04-11 12:51:05 -04:00 · 2019-01-30 10:09:04 +00:00
parent 02f7d7708a
commit 88ad982bf7
10 changed files with 125 additions and 50 deletions
--- a/app/assets/images/folder-black-bold.png
+++ b/app/assets/images/folder-black-bold.png
--- a/app/assets/images/folder-black-bold.svg
+++ b/app/assets/images/folder-black-bold.svg
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Generator: Adobe Illustrator 21.1.0, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
+<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
+	 viewBox="0 0 26 20" style="enable-background:new 0 0 26 20;" xml:space="preserve">
+<style type="text/css">
+	.st0{fill:#DEE0E2;}
+	.st1{fill:#0B0C0C;}
+</style>
+<path class="st0" d="M1.2,18.8V1.3h8.4l1.5,4h13.7v13.5H1.2z"/>
+<path class="st1" d="M8.8,2.5l0.9,2.4l0.6,1.6h13.2v11h-21v-15H8.8 M10.5,0H0v20h26V4H12L10.5,0z"/>
+</svg>
--- a/app/assets/images/folder-blue-bold-hover.png
+++ b/app/assets/images/folder-blue-bold-hover.png
--- a/app/assets/images/folder-blue-bold-hover.svg
+++ b/app/assets/images/folder-blue-bold-hover.svg
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Generator: Adobe Illustrator 21.1.0, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
+<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
+	 viewBox="0 0 26 20" style="enable-background:new 0 0 26 20;" xml:space="preserve">
+<style type="text/css">
+	.st0{fill:#DEE0E2;}
+	.st1{fill:#2B8CC4;}
+</style>
+<path class="st0" d="M1.2,18.8V1.3h8.4l1.5,4h13.7v13.5H1.2z"/>
+<path class="st1" d="M8.8,2.5l0.9,2.4l0.6,1.6h13.2v11h-21v-15H8.8 M10.5,0H0v20h26V4H12L10.5,0z"/>
+</svg>
--- a/app/assets/stylesheets/components/message.scss
+++ b/app/assets/stylesheets/components/message.scss
@@ -5,10 +5,13 @@
    margin: 0;

    a {
-      display: inline;
      margin-bottom: -$gutter;
      padding-bottom: $gutter;

+      &:hover {
+        color: $link-hover-colour;
+      }
+
      &:focus {

        // Use box shadow instead of outline to avoid buggy outline
@@ -86,23 +89,35 @@

  &-folder {

-    a:first-child {
+    display: inline-block;
+    padding-left: 40px;
+    background-image: file-url('folder-blue-bold.svg');
+    background-repeat: no-repeat;
+    background-size: auto 20px;
+    background-position: 0px 2px;

-      display: inline-block;
-      text-indent: 40px;
-      background-image: file-url('folder-blue-bold.svg');
-      background-repeat: no-repeat;
-      background-size: auto 20px;
-      background-position: 0px 2px;
+    @include ie-lte(8) {
+      background-image: file-url('folder-blue-bold.png');
+    }
+
+    &:hover {
+
+      background-image: file-url('folder-blue-bold-hover.svg');

      @include ie-lte(8) {
-        background-image: file-url('folder-blue-bold.png');
+        background-image: file-url('folder-blue-bold-hover.png');
      }

    }

  }

+  &-template {
+    a {
+      display: inline;
+    }
+  }
+
  &-empty {
    color: $secondary-text-colour;
    padding: $gutter-half 0 $gutter-one-third 0;
@@ -139,31 +154,60 @@

    display: inline-block;
    vertical-align: top;
-    padding: 5px 0 0 0;
    background-repeat: no-repeat;
-    background-size: 38px auto;
-    background-position: 0 1px;
+    background-size: auto 20px;
+    background-position: 0px 2px;
+    min-height: 30px;

  }

+  &-folder {
+
+    padding: 0 0 0 40px;
+    background-image: file-url('folder-black-bold.svg');
+
+    @include ie-lte(8) {
+      background-image: file-url('folder-black-bold.png');
+    }
+
+  }
+
+  &-folder-truncated {
+    width: 0;
+    padding: 0 0 0 30px;
+    overflow: hidden;
+  }
+
+  &-folder-root-truncated {
+    max-width: 1.4em;
+    overflow: hidden;
+    text-overflow: ellipsis;
+  }
+
  a {

-    padding-top: 5px;
    display: inline-block;
    vertical-align: top;

    &.folder-heading-folder {

-      padding: 5px 0 0 60px;
-      background-image: file-url('folder-blue.svg');
+      background-image: file-url('folder-blue-bold.svg');
      max-width: 33%;
      overflow: hidden;
      text-overflow: ellipsis;

      @include ie-lte(8) {
-        background-image: file-url('folder-blue.png');
+        background-image: file-url('folder-blue-bold.png');
      }

+      &:hover {
+        background-image: file-url('folder-blue-bold-hover.svg');
+      }
+
+    }
+
+    &:hover {
+      color: $link-hover-colour;
    }

  }
@@ -173,7 +217,7 @@
    display: inline-block;
    vertical-align: top;
    color: $secondary-text-colour;
-    padding: 5px 4px 0 5px;
+    padding: 0 4px 0 5px;
    font-weight: normal;

  }
@@ -181,7 +225,7 @@
  &-manage-link {
    display: block;
    text-align: right;
-    padding: ($gutter - 2px) 0 0 0;
+    padding: ($gutter - 7px) 0 0 0;
  }

 }
--- a/app/models/service.py
+++ b/app/models/service.py
@@ -377,14 +377,12 @@ class Service():
        if folder['id'] is None:
            return [folder]

-        return [
-            self.get_template_folder(folder['parent_id']),
-            folder,
+        return self.get_template_folder_path(folder['parent_id']) + [
+            self.get_template_folder(folder['id'])
        ]

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

--- a/app/templates/components/folder-path.html
+++ b/app/templates/components/folder-path.html
@@ -14,13 +14,18 @@
    <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>
+          {% if folder.template_type or not folder.id %}
+            <span class="folder-heading-template">{{ folder.name }}</span>
+          {% else %}
+            <span class="folder-heading-folder">{{ folder.name }}</span>
+          {% endif %}
        {% else %}
          {% if folder.id %}
-            <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type, template_folder_id=folder.id) }}" class="folder-heading-folder">{{ folder.name }}</a> {% if not loop.last %}{{ folder_path_separator() }}{% endif %}
+            <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type, template_folder_id=folder.id) }}" class="folder-heading-folder {% if loop.index < (loop.length - 1) %}folder-heading-folder-truncated{% endif %}" title="{{ folder.name }}">{{ folder.name }}</a>
          {% 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 %}
+            <a href="{{ url_for('.choose_template', service_id=service_id, template_type=template_type) }}" class="{% if loop.length > 2 %}folder-heading-folder-root-truncated{% endif %}">Templates</a>
          {% endif %}
+          {% if not loop.last %}{{ folder_path_separator() }}{% endif %}
        {% endif %}
      {% endfor %}
    </h1>
--- a/app/templates/views/templates/_template_list.html
+++ b/app/templates/views/templates/_template_list.html
@@ -21,18 +21,18 @@
            value=item.id,
          ) }}
        {% endif %}
-        <h2 class="message-name {% if item.is_folder or (item.ancestors and not item.is_folder) %}template-list-folder{% endif %}">
+        <h2 class="message-name">
          {% for ancestor in item.ancestors %}
-            <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=ancestor.id) }}">
+            <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=ancestor.id) }}" class="template-list-folder">
              {{ ancestor.name }}
            </a> <span class="message-name-separator">/</span>
          {% endfor %}
          {% if item.is_folder %}
-            <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=item.id) }}">
+            <a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=item.id) }}" class="template-list-folder">
              <span class="live-search-relevant">{{ item.name }}</span>
            </a>
          {% else %}
-            <a href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.id) }}">
+            <a href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.id) }}" class="template-list-template">
              <span class="live-search-relevant">{{ item.name }}</span>
            </a>
          {% endif %}
--- a/tests/app/main/views/test_template_folders.py
+++ b/tests/app/main/views/test_template_folders.py
@@ -124,7 +124,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
        (
            'Templates – service one – GOV.UK Notify',
            'Templates',
-            None,
+            [],
            {},
            ['Text message', 'Email', 'Letter'],
            [
@@ -172,7 +172,7 @@ 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'],
            [
@@ -201,7 +201,7 @@ 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_type': 'all'}],
            {'template_folder_id': PARENT_FOLDER_ID},
            ['Text message', 'Email', 'Letter'],
            [
@@ -227,7 +227,7 @@ 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_type': 'sms', 'template_folder_id': PARENT_FOLDER_ID},
            ['All', 'Email', 'Letter'],
            [
@@ -248,7 +248,7 @@ 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_type': 'email', 'template_folder_id': PARENT_FOLDER_ID},
            ['All', 'Text message', 'Letter'],
            [],
@@ -257,9 +257,12 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
            'There are no email templates in this folder',
        ),
        (
-            'folder_one_one – folder_one – service one – GOV.UK Notify',
-            'folder_one / folder_one_one',
-            {'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID},
+            'folder_one_one – folder_one – Templates – service one – GOV.UK Notify',
+            'Templates / folder_one / folder_one_one',
+            [
+                {'template_type': 'all'},
+                {'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID},
+            ],
            {'template_folder_id': CHILD_FOLDER_ID},
            ['Text message', 'Email', 'Letter'],
            [
@@ -279,9 +282,13 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
            None,
        ),
        (
-            '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},
+            'folder_one_one_one – folder_one_one – folder_one – Templates – service one – GOV.UK Notify',
+            'Templates / folder_one / folder_one_one / folder_one_one_one',
+            [
+                {'template_type': 'all'},
+                {'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID},
+                {'template_type': 'all', 'template_folder_id': CHILD_FOLDER_ID},
+            ],
            {'template_folder_id': GRANDCHILD_FOLDER_ID},
            ['Text message', 'Email', 'Letter'],
            [
@@ -298,7 +305,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
        (
            'folder_two – Templates – service one – GOV.UK Notify',
            'Templates / folder_two',
-            {'template_type': 'all'},
+            [{'template_type': 'all'}],
            {'template_folder_id': FOLDER_TWO_ID},
            ['Text message', 'Email', 'Letter'],
            [],
@@ -309,7 +316,7 @@ def test_post_add_template_folder_page(client_request, service_one, mocker, pare
        (
            'folder_two – Templates – service one – GOV.UK Notify',
            'Templates / folder_two',
-            {'template_type': 'sms'},
+            [{'template_type': 'sms'}],
            {'template_folder_id': FOLDER_TWO_ID, 'template_type': 'sms'},
            ['All', 'Email', 'Letter'],
            [],
@@ -368,15 +375,14 @@ def test_should_show_templates_folder_page(
    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(
+    assert len(page.select('h1 a')) == len(expected_parent_link_args)
+
+    for index, parent_link in enumerate(page.select('h1 a')):
+        assert parent_link['href'] == url_for(
            'main.choose_template',
            service_id=SERVICE_ONE_ID,
-            **expected_parent_link_args
+            **expected_parent_link_args[index]
        )
-    else:
-        assert page.select_one('h1 a') is None

    links_in_page = page.select('.pill a')

--- a/tests/app/main/views/test_templates.py
+++ b/tests/app/main/views/test_templates.py
@@ -267,7 +267,7 @@ def test_should_show_live_search_if_service_has_lots_of_folders(
    )

    count_of_templates_and_folders = len(page.select('.message-name'))
-    count_of_folders = len(page.select('.template-list-folder'))
+    count_of_folders = len(page.select('.template-list-folder:first-child'))
    count_of_templates = count_of_templates_and_folders - count_of_folders

    assert len(page.select('.live-search')) == 1