Merge pull request #3700 from alphagov/fix-template-list-item-focus-style

Fix template list item focus style

app/assets/stylesheets/components/message.scss

@@ -28,45 +28,28 @@ $govuk-checkboxes-size: 40px;
 $govuk-checkboxes-label-padding-left-right: govuk-spacing(3);
 $message-type-bottom-spacing: govuk-spacing(4);
 
-.message {
-
-  &-name {
-    margin: 0;
-
-    a {
-      margin-bottom: -1 * govuk-spacing(6);
-      padding-bottom: govuk-spacing(6);
-
-      &:hover .message-name-separator:before {
-        border-color: $link-hover-colour;
-      }
-
-      .message-name-separator {
-
-        margin-right: -2px;
-        margin-left: -2px;
-
-        &:before {
-          border-color: $link-colour;
-        }
-      }
-    }
-
-    &-separator {
-      @include separator;
-    }
+a {
 
+  &:hover .message-name-separator:before {
+    border-color: $link-hover-colour;
   }
 
-  &-type {
-    color: $govuk-secondary-text-colour;
-    margin: 0 0 $message-type-bottom-spacing 0;
-    padding-left: 0;
-    pointer-events: none;
+  .message-name-separator {
+
+    margin-right: -2px;
+    margin-left: -2px;
+
+    &:before {
+      border-color: $link-colour;
+    }
   }
 
 }
 
+.message-name-separator {
+  @include separator;
+}
+
 .template-list {
 
   &-item {
@@ -81,18 +64,20 @@ $message-type-bottom-spacing: govuk-spacing(4);
 
     &-without-ancestors {
 
-      .message-name {
+      a {
 
-        a {
+        display: block;
+
+        &:first-child {
           display: block;
+        }
 
-          &:first-child {
-            display: block;
-          }
+        &.template-list-folder:first-of-type {
+          background-position: 0 2px;
+          padding-left: 0;
+          text-indent: 35px;
 
-          &.template-list-folder:first-child {
-            background-position: 0 2px;
-            padding-left: 0;
+          @include govuk-media-query($from: tablet) {
             text-indent: 40px;
           }
 
@@ -100,6 +85,43 @@ $message-type-bottom-spacing: govuk-spacing(4);
 
       }
 
+      .template-list-template,
+      .template-list-folder {
+
+        position: relative; /* contain absolutely positioned ::before pseudo element*/
+        display: block; /* fill horizontal space to allow hint/meta below to float */
+
+        &::before {
+          content: '';
+          position: absolute;
+          left: 0px;
+          bottom: -100%; /* extend link by 100% of vertical size so it covers the hint/meta */
+          width: 100%;
+          height: 100%;
+        }
+
+        &:active,
+        &:focus {
+
+          &::before {
+            background-color: $govuk-focus-colour;
+            box-shadow: 0px -2px $govuk-focus-colour, 0px 4px $govuk-focus-text-colour;
+          }
+
+          & + .template-list-item-hint,
+          .message-type {
+            color: $govuk-text-colour;
+          }
+
+        }
+
+        & + .template-list-item-hint,
+        & + .message-type {
+          position: relative; /* needs to be non-static to have a z-index above the link :before element */
+        }
+
+      }
+
     }
 
     &-label {
@@ -124,18 +146,25 @@ $message-type-bottom-spacing: govuk-spacing(4);
 
   &-folder,
   &-template {
-    @include govuk-font($size: 24, $weight: bold, $line-height: 1.25);
+    @include govuk-font($size: 24, $weight: bold, $line-height: 1.5);
+    @include govuk-media-query($from: tablet) {
+      line-height: 1.25;
+    }
   }
 
   &-folder {
 
     display: inline;
-    padding-left: 40px;
+    padding-left: 35px;
     background-image: file-url('folder-blue-bold.svg');
     background-repeat: no-repeat;
     background-size: auto 20px;
     background-position: 0px 4px;
 
+    @include govuk-media-query($from: tablet) {
+      padding-left: 40px;
+    }
+
     &:hover {
       background-image: file-url('folder-blue-bold-hover.svg');
     }

app/templates/views/templates/_template_list.html

@@ -26,6 +26,10 @@
   <nav id="template-list" class="{{ 'govuk-!-margin-top-1' if (not show_template_nav and not show_search_box) else 'govuk-!-margin-top-6' }}">
     {% set checkboxes_data = [] %}
 
+    {% if not current_user.has_permissions('manage_templates') %}
+    <ul>
+    {% endif %}
+
     {% for item in template_list %}
 
       {% set item_link_content %}
@@ -73,17 +77,19 @@
       {% set _ = checkboxes_data.append(checkbox_config) %}
 
       {% if not current_user.has_permissions('manage_templates') %}
-        <div class="template-list-item {%- if item.ancestors %} template-list-item-hidden-by-default {%- else %} template-list-item-without-ancestors{%- endif %}">
-          <h2 class="message-name">
-            {{ item_link_content }}
-          </h2>
-          <p class="message-type govuk-!-margin-bottom-4">
+        <li class="template-list-item {%- if item.ancestors %} template-list-item-hidden-by-default {%- else %} template-list-item-without-ancestors{%- endif %}">
+          {{ item_link_content }}
+          <p class="template-list-item-hint govuk-!-margin-bottom-4">
             {{ item.hint }}
           </p>
-        </div>
+        </li>
       {% endif %}
     {% endfor %}
 
+    {% if not current_user.has_permissions('manage_templates') %}
+    </ul>
+    {% endif %}
+
     {% if current_user.has_permissions('manage_templates') %}
       {{ templates_and_folders_form.templates_and_folders(param_extensions={
         "items": checkboxes_data,

app/templates/views/templates/choose-reply.html

@@ -38,29 +38,29 @@
     {{ live_search(target_selector='#template-list .template-list-item', show=True, form=search_form) }}
 
     <nav id="template-list">
+      <ul>
       {% for item in templates_and_folders %}
-        <div class="template-list-item {% if item.ancestors %}template-list-item-hidden-by-default{% endif %} {% if not item.ancestors %}template-list-item-without-ancestors{% endif %}">
-          <h2 class="message-name">
-            {% for ancestor in item.ancestors %}
-                <a href="{{ url_for('.conversation_reply', service_id=current_service.id, notification_id=notification_id, from_folder=ancestor.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
-                {{ ancestor.name }}
-              </a> <span class="message-name-separator">/</span>
-            {% endfor %}
-            {% if item.is_folder %}
-              <a href="{{ url_for('.conversation_reply', service_id=current_service.id, notification_id=notification_id, from_folder=item.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
-                <span class="live-search-relevant">{{ item.name }}</span>
-              </a>
-            {% else %}
-              <a class="govuk-link govuk-link--no-visited-state" href="{{ url_for('.conversation_reply_with_template', service_id=current_service.id, template_id=item.id, notification_id=notification_id) }}">
-                <span class="live-search-relevant">{{ item.name }}</span>
-              </a>
-            {% endif %}
-          </h2>
-          <p class="message-type">
+        <li class="template-list-item {% if item.ancestors %}template-list-item-hidden-by-default{% endif %} {% if not item.ancestors %}template-list-item-without-ancestors{% endif %}">
+          {% for ancestor in item.ancestors %}
+              <a href="{{ url_for('.conversation_reply', service_id=current_service.id, notification_id=notification_id, from_folder=ancestor.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
+              {{ ancestor.name }}
+            </a> <span class="message-name-separator"></span>
+          {% endfor %}
+          {% if item.is_folder %}
+            <a href="{{ url_for('.conversation_reply', service_id=current_service.id, notification_id=notification_id, from_folder=item.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
+              <span class="live-search-relevant">{{ item.name }}</span>
+            </a>
+          {% else %}
+            <a class="govuk-link govuk-link--no-visited-state template-list-template" href="{{ url_for('.conversation_reply_with_template', service_id=current_service.id, template_id=item.id, notification_id=notification_id) }}">
+              <span class="live-search-relevant">{{ item.name }}</span>
+            </a>
+          {% endif %}
+          <p class="template-list-item-hint govuk-hint">
             {{ item.hint }}
           </p>
-        </div>
+        </li>
       {% endfor %}
+      </ul>
     </nav>
 
 

app/templates/views/templates/copy.html

@@ -26,37 +26,37 @@
         autofocus=True
       ) }}
       <nav id="template-list">
+        <ul>
         {% for item in services_templates_and_folders %}
-          <div class="template-list-item {% if item.ancestors %}template-list-item-hidden-by-default{% endif %} {% if not item.ancestors %}template-list-item-without-ancestors{% endif %}">
-            <h2 class="message-name">
-              {% for ancestor in item.ancestors %}
-                {% if ancestor.is_service %}
-                  <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=ancestor.service_id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
-                {% else %}
-                  <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_folder=ancestor.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
-                {% endif %}
-                  {{ ancestor.name }}
-                </a> <span class="message-name-separator"></span>
-              {% endfor %}
-              {% if item.is_service %}
-                <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=item.service_id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
-                  <span class="live-search-relevant">{{ item.name }}</span>
-                </a>
-              {% elif item.is_folder %}
-                <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=item.service_id, from_folder=item.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
-                  <span class="live-search-relevant">{{ item.name }}</span>
-                </a>
+          <li class="template-list-item {% if item.ancestors %}template-list-item-hidden-by-default{% endif %} {% if not item.ancestors %}template-list-item-without-ancestors{% endif %}">
+            {% for ancestor in item.ancestors %}
+              {% if ancestor.is_service %}
+                <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=ancestor.service_id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
               {% else %}
-                <a class="govuk-link govuk-link--no-visited-state" href="{{ url_for('.copy_template', service_id=current_service.id, template_id=item.id, from_service=item.service_id) }}">
-                  <span class="live-search-relevant">{{ item.name }}</span>
-                </a>
+                <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_folder=ancestor.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
               {% endif %}
-            </h2>
-            <p class="message-type">
+                {{ ancestor.name }}
+              </a> <span class="message-name-separator"></span>
+            {% endfor %}
+            {% if item.is_service %}
+              <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=item.service_id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
+                <span class="live-search-relevant">{{ item.name }}</span>
+              </a>
+            {% elif item.is_folder %}
+              <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=item.service_id, from_folder=item.id) }}" class="govuk-link govuk-link--no-visited-state template-list-folder">
+                <span class="live-search-relevant">{{ item.name }}</span>
+              </a>
+            {% else %}
+              <a class="govuk-link govuk-link--no-visited-state template-list-template" href="{{ url_for('.copy_template', service_id=current_service.id, template_id=item.id, from_service=item.service_id) }}">
+                <span class="live-search-relevant">{{ item.name }}</span>
+              </a>
+            {% endif %}
+            <p class="template-list-item-hint govuk-hint">
               {{ item.hint }}
             </p>
-          </div>
+          </li>
         {% endfor %}
+        </ul>
       </nav>
     {% endif %}
 

tests/app/main/views/test_templates.py

@@ -188,7 +188,7 @@ def test_should_show_page_for_choosing_a_template(
     for index, expected_link in enumerate(expected_nav_links):
         assert links_in_page[index].text.strip() == expected_link
 
-    template_links = page.select('#template-list .govuk-label a, .message-name a')
+    template_links = page.select('#template-list .govuk-label a, .template-list-item a')
 
     assert len(template_links) == len(expected_templates)