Merge pull request #2701 from alphagov/folder-path

Improve display of folder path when deeply nested

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>

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>

app/assets/images/folder-blue.svg

@@ -1 +0,0 @@
-<svg xmlns="http://www.w3.org/2000/svg" width="26" height="20" viewBox="0 0 26 20"><path fill="#dee0e2" d="M1 19.1V1h8.8l1.5 4H25v14.1z"/><path d="M9.2 1.9l1 2.8.5 1.2h13.4v12.2H1.9V1.9h7.3M10.5 0H0v20h26V4H12l-1.5-4z" fill="#005ea5"/></svg>

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
@@ -76,6 +79,13 @@
           &:first-child {
             display: block;
           }
+
+          &.template-list-folder:first-child {
+            background-position: 0 2px;
+            padding-left: 0;
+            text-indent: 40px;
+          }
+
         }
 
       }
@@ -86,23 +96,35 @@
 
   &-folder {
 
-    a:first-child {
+    display: inline;
+    padding-left: 40px;
+    background-image: file-url('folder-blue-bold.svg');
+    background-repeat: no-repeat;
+    background-size: auto 20px;
+    background-position: 0px 4px;
 
-      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;
@@ -128,42 +150,74 @@
     margin-top: 18px;
   }
 
-  overflow: hidden;
-  text-overflow: ellipsis;
-  white-space: nowrap;
-
-
   a,
   &-folder,
   &-subfolder {
 
-    display: inline-block;
+    display: inline;
     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 4px;
+    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;
+    white-space: nowrap;
+    overflow: hidden;
+  }
+
+  &-folder-root-truncated {
+    max-width: 1.4em;
+    white-space: nowrap;
+    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');
-      max-width: 33%;
+      display: inline;
+      background-image: file-url('folder-blue-bold.svg');
+      background-position: 0px 4px;
       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');
+      }
+
+    }
+
+    &.folder-heading-folder-truncated {
+      display: inline-block;
+      background-position: 0px 2px;
+    }
+
+    &:hover {
+      color: $link-hover-colour;
     }
 
   }
@@ -173,7 +227,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 +235,7 @@
   &-manage-link {
     display: block;
     text-align: right;
-    padding: ($gutter - 2px) 0 0 0;
+    padding: ($gutter - 7px) 0 0 0;
   }
 
 }

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,
         ]
 

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>

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 %}

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')
 

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