darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-09-09 08:09:49 -04:00
Code Issues Packages Projects Releases Wiki Activity

Use chevrons not slashes to separate folders

Browse Source 
It looks weird to have two different visual treatments for showing a
navigable hierarchy.

I reckon losing the slash won’t make things less folder like – Windows
for example uses chevrons as foler separators.
This commit is contained in:
Chris Hill-Scott
2019-06-24 16:02:49 +01:00
parent acd992183c
commit e731dd70d1
8 changed files with 105 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
app/assets/stylesheets/components/message.scss
View File

@@ -1,3 +1,29 @@
@mixin separator {
display: inline-block;
vertical-align: top;
width: 20px;
height: $gutter;
position: relative;
&:before {
content: "";
display: block;
position: absolute;
top: -5px;
bottom: 1px;
right: 7px;
width: 9px;
height: 9px;
margin: auto 0;
-webkit-transform: rotate(45deg);
-ms-transform: rotate(45deg);
transform: rotate(45deg);
border: solid;
border-width: 2px 2px 0 0;
border-color: $secondary-text-colour;
}
}
.message { .message {
&-name { &-name {
@@ -28,11 +54,7 @@
} }
&-separator { &-separator {
display: inline-block; @include separator;
vertical-align: top;
color: $secondary-text-colour;
padding: 0 4px 0 5px;
font-weight: normal;
} }
} }
@@ -235,13 +257,7 @@
} }
&-separator { &-separator {
@include separator;
display: inline-block;
vertical-align: top;
color: $secondary-text-colour;
padding: 0 4px 0 5px;
font-weight: normal;
} }
&-manage-link { &-manage-link {

5
app/models/service.py
View File

@@ -452,7 +452,10 @@ class Service(JSONModel):
"users_with_permission": folder["users_with_permission"] "users_with_permission": folder["users_with_permission"]
} }
while folder_attrs["parent_id"] is not None: while folder_attrs["parent_id"] is not None:
folder_attrs["name"] = parent["name"] + " / " + folder_attrs["name"] folder_attrs["name"] = [
parent["name"],
folder_attrs["name"],
]
if parent["parent_id"] is None: if parent["parent_id"] is None:
folder_attrs["parent_id"] = None folder_attrs["parent_id"] = None
else: else:

2
app/templates/components/folder-path.html
View File

@@ -79,5 +79,5 @@
{% macro folder_path_separator() %} {% macro folder_path_separator() %}
<span class="folder-heading-separator">/</span> <span class="folder-heading-separator"></span>
{% endmacro %} {% endmacro %}

19
app/templates/views/templates/_template_list.html
View File

@@ -1,6 +1,17 @@
{% from "components/checkbox.html" import unlabelled_checkbox %} {% from "components/checkbox.html" import unlabelled_checkbox %}
{% from "components/message-count-label.html" import folder_contents_count, message_count_label %} {% from "components/message-count-label.html" import folder_contents_count, message_count_label %}
{% macro format_item_name(name) -%}
{%- if name is string -%}
{{- name -}}
{%- else -%}
{%- for part in name -%}
{{- format_item_name(part) -}}
{%- if not loop.last %} <span class="message-name-separator"></span> {% endif -%}
{%- endfor -%}
{% endif %}
{%- endmacro %}
{% if template_list.template_folder_id and not template_list.templates_to_show %} {% if template_list.template_folder_id and not template_list.templates_to_show %}
<p class="template-list-empty"> <p class="template-list-empty">
{% if template_list.folder_is_empty %} {% if template_list.folder_is_empty %}
@@ -24,16 +35,16 @@
<h2 class="message-name"> <h2 class="message-name">
{% for ancestor in item.ancestors %} {% 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) }}" class="template-list-folder"> <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 }} {{- format_item_name(ancestor.name) -}}
</a> <span class="message-name-separator">/</span> </a> <span class="message-name-separator"></span>
{% endfor %} {% endfor %}
{% if item.is_folder %} {% if item.is_folder %}
<a href="{{ url_for('.choose_template', service_id=current_service.id, template_type=template_type, template_folder_id=item.id) }}" class="template-list-folder"> <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> <span class="live-search-relevant">{{ format_item_name(item.name) }}</span>
</a> </a>
{% else %} {% else %}
<a href="{{ url_for('.view_template', service_id=current_service.id, template_id=item.id) }}" class="template-list-template"> <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> <span class="live-search-relevant">{{ format_item_name(item.name) }}</span>
</a> </a>
{% endif %} {% endif %}
</h2> </h2>

2
app/templates/views/templates/copy.html
View File

@@ -30,7 +30,7 @@
<a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_folder=ancestor.id) }}" class="template-list-folder"> <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_folder=ancestor.id) }}" class="template-list-folder">
{% endif %} {% endif %}
{{ ancestor.name }} {{ ancestor.name }}
</a> <span class="message-name-separator">/</span> </a> <span class="message-name-separator"></span>
{% endfor %} {% endfor %}
{% if item.is_service %} {% if item.is_service %}
<a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=item.service_id) }}" class="template-list-folder"> <a href="{{ url_for('.choose_template_to_copy', service_id=current_service.id, from_service=item.service_id) }}" class="template-list-folder">

71
tests/app/main/views/test_template_folders.py
View File

@@ -55,11 +55,11 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
['Email', 'Text message', 'Letter'], ['Email', 'Text message', 'Letter'],
[ [
'folder_one 2 folders', 'folder_one 2 folders',
'folder_one / folder_one_one 1 template, 1 folder', 'folder_one folder_one_one 1 template, 1 folder',
'folder_one / folder_one_one / folder_one_one_one 1 template', 'folder_one folder_one_one folder_one_one_one 1 template',
'folder_one / folder_one_one / folder_one_one_one / sms_template_nested Text message template', 'folder_one folder_one_one folder_one_one_one sms_template_nested Text message template',
'folder_one / folder_one_one / letter_template_nested Letter template', 'folder_one folder_one_one letter_template_nested Letter template',
'folder_one / folder_one_two Empty', 'folder_one folder_one_two Empty',
'folder_two Empty', 'folder_two Empty',
'sms_template_one Text message template', 'sms_template_one Text message template',
'sms_template_two Text message template', 'sms_template_two Text message template',
@@ -103,9 +103,9 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
['All', 'Email', 'Letter'], ['All', 'Email', 'Letter'],
[ [
'folder_one 1 folder', 'folder_one 1 folder',
'folder_one / folder_one_one 1 folder', 'folder_one folder_one_one 1 folder',
'folder_one / folder_one_one / folder_one_one_one 1 template', 'folder_one folder_one_one folder_one_one_one 1 template',
'folder_one / folder_one_one / folder_one_one_one / sms_template_nested Text message template', 'folder_one folder_one_one folder_one_one_one sms_template_nested Text message template',
'sms_template_one Text message template', 'sms_template_one Text message template',
'sms_template_two Text message template', 'sms_template_two Text message template',
], ],
@@ -126,15 +126,15 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_one – Templates – service one – GOV.UK Notify', 'folder_one – Templates – service one – GOV.UK Notify',
'Templates / folder_one', 'Templates folder_one',
[{'template_type': 'all'}], [{'template_type': 'all'}],
{'template_folder_id': PARENT_FOLDER_ID}, {'template_folder_id': PARENT_FOLDER_ID},
['Email', 'Text message', 'Letter'], ['Email', 'Text message', 'Letter'],
[ [
'folder_one_one 1 template, 1 folder', 'folder_one_one 1 template, 1 folder',
'folder_one_one / folder_one_one_one 1 template', 'folder_one_one folder_one_one_one 1 template',
'folder_one_one / folder_one_one_one / sms_template_nested Text message template', 'folder_one_one folder_one_one_one sms_template_nested Text message template',
'folder_one_one / letter_template_nested Letter template', 'folder_one_one letter_template_nested Letter template',
'folder_one_two Empty', 'folder_one_two Empty',
], ],
[ [
@@ -152,14 +152,14 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_one – Templates – service one – GOV.UK Notify', 'folder_one – Templates – service one – GOV.UK Notify',
'Templates / folder_one', 'Templates folder_one',
[{'template_type': 'sms'}], [{'template_type': 'sms'}],
{'template_type': 'sms', 'template_folder_id': PARENT_FOLDER_ID}, {'template_type': 'sms', 'template_folder_id': PARENT_FOLDER_ID},
['All', 'Email', 'Letter'], ['All', 'Email', 'Letter'],
[ [
'folder_one_one 1 folder', 'folder_one_one 1 folder',
'folder_one_one / folder_one_one_one 1 template', 'folder_one_one folder_one_one_one 1 template',
'folder_one_one / folder_one_one_one / sms_template_nested Text message template', 'folder_one_one folder_one_one_one sms_template_nested Text message template',
], ],
[ [
'folder_one_one 1 folder', 'folder_one_one 1 folder',
@@ -173,7 +173,7 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_one – Templates – service one – GOV.UK Notify', 'folder_one – Templates – service one – GOV.UK Notify',
'Templates / folder_one', 'Templates folder_one',
[{'template_type': 'email'}], [{'template_type': 'email'}],
{'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID}, {'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID},
['All', 'Text message', 'Letter'], ['All', 'Text message', 'Letter'],
@@ -184,7 +184,7 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_one_one – folder_one – Templates – service one – GOV.UK Notify', 'folder_one_one – folder_one – Templates – service one – GOV.UK Notify',
'Templates / folder_one / folder_one_one', 'Templates folder_one folder_one_one',
[ [
{'template_type': 'all'}, {'template_type': 'all'},
{'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID}, {'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID},
@@ -193,7 +193,7 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
['Email', 'Text message', 'Letter'], ['Email', 'Text message', 'Letter'],
[ [
'folder_one_one_one 1 template', 'folder_one_one_one 1 template',
'folder_one_one_one / sms_template_nested Text message template', 'folder_one_one_one sms_template_nested Text message template',
'letter_template_nested Letter template', 'letter_template_nested Letter template',
], ],
[ [
@@ -209,7 +209,7 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_one_one_one – folder_one_one – folder_one – Templates – service one – GOV.UK Notify', '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', 'Templates folder_one folder_one_one folder_one_one_one',
[ [
{'template_type': 'all'}, {'template_type': 'all'},
{'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID}, {'template_type': 'all', 'template_folder_id': PARENT_FOLDER_ID},
@@ -230,7 +230,7 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_one_one_one – folder_one_one – folder_one – Templates – service one – GOV.UK Notify', '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', 'Templates folder_one folder_one_one folder_one_one_one',
[ [
{'template_type': 'email'}, {'template_type': 'email'},
{'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID}, {'template_type': 'email', 'template_folder_id': PARENT_FOLDER_ID},
@@ -248,7 +248,7 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_two – Templates – service one – GOV.UK Notify', 'folder_two – Templates – service one – GOV.UK Notify',
'Templates / folder_two', 'Templates folder_two',
[{'template_type': 'all'}], [{'template_type': 'all'}],
{'template_folder_id': FOLDER_TWO_ID}, {'template_folder_id': FOLDER_TWO_ID},
['Email', 'Text message', 'Letter'], ['Email', 'Text message', 'Letter'],
@@ -259,7 +259,7 @@ def _folder(name, folder_id=None, parent=None, users_with_permission=None):
), ),
( (
'folder_two – Templates – service one – GOV.UK Notify', 'folder_two – Templates – service one – GOV.UK Notify',
'Templates / folder_two', 'Templates folder_two',
[{'template_type': 'sms'}], [{'template_type': 'sms'}],
{'template_folder_id': FOLDER_TWO_ID, 'template_type': 'sms'}, {'template_folder_id': FOLDER_TWO_ID, 'template_type': 'sms'},
['All', 'Email', 'Letter'], ['All', 'Email', 'Letter'],
@@ -1405,17 +1405,17 @@ def test_show_custom_error_message(
{}, {},
[ [
['folder_A', '1 template, 2 folders'], ['folder_A', '1 template, 2 folders'],
['folder_E / folder_F / folder_G', '1 template'], ['folder_E folder_F folder_G', '1 template'],
['email_template_root', 'Email template'], ['email_template_root', 'Email template'],
], ],
[ [
['folder_A', '1 template, 2 folders'], ['folder_A', '1 template, 2 folders'],
['folder_A', '/', 'folder_C', '1 template'], ['folder_A', 'folder_C', '1 template'],
['folder_A', '/', 'folder_C', '/', 'sms_template_C', 'Text message template'], ['folder_A', 'folder_C', 'sms_template_C', 'Text message template'],
['folder_A', '/', 'folder_D', 'Empty'], ['folder_A', 'folder_D', 'Empty'],
['folder_A', '/', 'sms_template_A', 'Text message template'], ['folder_A', 'sms_template_A', 'Text message template'],
['folder_E / folder_F / folder_G', '1 template'], ['folder_E folder_F folder_G', '1 template'],
['folder_E / folder_F / folder_G', '/', 'email_template_G', 'Email template'], ['folder_E folder_F folder_G', 'email_template_G', 'Email template'],
['email_template_root', 'Email template'], ['email_template_root', 'Email template'],
], ],
None, None,
@@ -1423,12 +1423,12 @@ def test_show_custom_error_message(
( (
{'template_type': 'email'}, {'template_type': 'email'},
[ [
['folder_E / folder_F / folder_G', '1 template'], ['folder_E folder_F folder_G', '1 template'],
['email_template_root', 'Email template'], ['email_template_root', 'Email template'],
], ],
[ [
['folder_E / folder_F / folder_G', '1 template'], ['folder_E folder_F folder_G', '1 template'],
['folder_E / folder_F / folder_G', '/', 'email_template_G', 'Email template'], ['folder_E folder_F folder_G', 'email_template_G', 'Email template'],
['email_template_root', 'Email template'], ['email_template_root', 'Email template'],
], ],
None, None,
@@ -1440,9 +1440,9 @@ def test_show_custom_error_message(
], ],
[ [
['folder_A', '1 template, 1 folder'], ['folder_A', '1 template, 1 folder'],
['folder_A', '/', 'folder_C', '1 template'], ['folder_A', 'folder_C', '1 template'],
['folder_A', '/', 'folder_C', '/', 'sms_template_C', 'Text message template'], ['folder_A', 'folder_C', 'sms_template_C', 'Text message template'],
['folder_A', '/', 'sms_template_A', 'Text message template'], ['folder_A', 'sms_template_A', 'Text message template'],
], ],
None, None,
), ),
@@ -1521,7 +1521,6 @@ def test_should_filter_templates_folder_page_based_on_user_permissions(
and 'template-list-item' in tag['class'] and 'template-list-item' in tag['class']
and 'template-list-item-hidden-by-default' not in tag['class'] and 'template-list-item-hidden-by-default' not in tag['class']
)) ))
assert [ assert [
[i.strip() for i in e.text.split("\n") if i.strip()] [i.strip() for i in e.text.split("\n") if i.strip()]
for e in displayed_page_items for e in displayed_page_items

32
tests/app/main/views/test_templates.py
View File

@@ -428,7 +428,7 @@ def test_user_with_only_send_and_view_sees_letter_page(
_test_page_title=False, _test_page_title=False,
) )
assert normalize_spaces(page.select_one('h1').text) == ( assert normalize_spaces(page.select_one('h1').text) == (
'Templates / Two week reminder' 'Templates Two week reminder'
) )
assert normalize_spaces(page.select_one('title').text) == ( assert normalize_spaces(page.select_one('title').text) == (
'Two week reminder – Templates – service one – GOV.UK Notify' 'Two week reminder – Templates – service one – GOV.UK Notify'
@@ -624,7 +624,7 @@ def test_should_be_able_to_view_a_template_with_links(
) )
assert normalize_spaces(page.select_one('h1').text) == ( assert normalize_spaces(page.select_one('h1').text) == (
'Templates / Two week reminder' 'Templates Two week reminder'
) )
assert normalize_spaces(page.select_one('title').text) == ( assert normalize_spaces(page.select_one('title').text) == (
'Two week reminder – Templates – service one – GOV.UK Notify' 'Two week reminder – Templates – service one – GOV.UK Notify'
@@ -855,27 +855,27 @@ def test_choose_a_template_to_copy(
'6 templates' '6 templates'
), ),
( (
'Service 1 / sms_template_one ' 'Service 1 sms_template_one '
'Text message template' 'Text message template'
), ),
( (
'Service 1 / sms_template_two ' 'Service 1 sms_template_two '
'Text message template' 'Text message template'
), ),
( (
'Service 1 / email_template_one ' 'Service 1 email_template_one '
'Email template' 'Email template'
), ),
( (
'Service 1 / email_template_two ' 'Service 1 email_template_two '
'Email template' 'Email template'
), ),
( (
'Service 1 / letter_template_one ' 'Service 1 letter_template_one '
'Letter template' 'Letter template'
), ),
( (
'Service 1 / letter_template_two ' 'Service 1 letter_template_two '
'Letter template' 'Letter template'
), ),
( (
@@ -883,27 +883,27 @@ def test_choose_a_template_to_copy(
'6 templates' '6 templates'
), ),
( (
'Service 2 / sms_template_one ' 'Service 2 sms_template_one '
'Text message template' 'Text message template'
), ),
( (
'Service 2 / sms_template_two ' 'Service 2 sms_template_two '
'Text message template' 'Text message template'
), ),
( (
'Service 2 / email_template_one ' 'Service 2 email_template_one '
'Email template' 'Email template'
), ),
( (
'Service 2 / email_template_two ' 'Service 2 email_template_two '
'Email template' 'Email template'
), ),
( (
'Service 2 / letter_template_one ' 'Service 2 letter_template_one '
'Letter template' 'Letter template'
), ),
( (
'Service 2 / letter_template_two ' 'Service 2 letter_template_two '
'Letter template' 'Letter template'
), ),
] ]
@@ -1026,7 +1026,7 @@ def test_choose_a_template_to_copy_from_folder_within_service(
) )
assert normalize_spaces(page.select_one('.folder-heading').text) == ( assert normalize_spaces(page.select_one('.folder-heading').text) == (
'service one / Parent folder' 'service one Parent folder'
) )
breadcrumb_links = page.select('.folder-heading a') breadcrumb_links = page.select('.folder-heading a')
assert len(breadcrumb_links) == 1 assert len(breadcrumb_links) == 1
@@ -1046,7 +1046,7 @@ def test_choose_a_template_to_copy_from_folder_within_service(
'1 template' '1 template'
), ),
( (
'Child folder non-empty / Should appear in list (nested) ' 'Child folder non-empty Should appear in list (nested) '
'Text message template' 'Text message template'
), ),
( (

10
tests/app/models/test_service.py
View File

@@ -80,13 +80,13 @@ def test_get_user_template_folders_only_returns_folders_visible_to_user(
result = service.get_user_template_folders(User(active_user_with_permissions)) result = service.get_user_template_folders(User(active_user_with_permissions))
assert result == [ assert result == [
{ {
'name': "Parent 1 - invisible / 1's Visible child", 'name': ["Parent 1 - invisible", "1's Visible child"],
'id': mocker.ANY, 'id': mocker.ANY,
'parent_id': None, 'parent_id': None,
'users_with_permission': [active_user_with_permissions['id']], 'users_with_permission': [active_user_with_permissions['id']],
}, },
{ {
'name': "Parent 1 - invisible / 1's Invisible child / 1's Visible grandchild", 'name': ["Parent 1 - invisible", ["1's Invisible child", "1's Visible grandchild"]],
'id': mocker.ANY, 'id': mocker.ANY,
'parent_id': None, 'parent_id': None,
'users_with_permission': [active_user_with_permissions['id']], 'users_with_permission': [active_user_with_permissions['id']],
@@ -98,7 +98,7 @@ def test_get_user_template_folders_only_returns_folders_visible_to_user(
'users_with_permission': [active_user_with_permissions['id']], 'users_with_permission': [active_user_with_permissions['id']],
}, },
{ {
'name': "2's Invisible child / 2's Visible grandchild", 'name': ["2's Invisible child", "2's Visible grandchild"],
'id': mocker.ANY, 'id': mocker.ANY,
'parent_id': VIS_PARENT_FOLDER_ID, 'parent_id': VIS_PARENT_FOLDER_ID,
'users_with_permission': [active_user_with_permissions['id']], 'users_with_permission': [active_user_with_permissions['id']],
@@ -124,13 +124,13 @@ def test_get_template_folders_shows_user_folders_when_user_id_passed_in(
result = service.get_template_folders(user=User(active_user_with_permissions)) result = service.get_template_folders(user=User(active_user_with_permissions))
assert result == [ assert result == [
{ {
'name': "Parent 1 - invisible / 1's Visible child", 'name': ["Parent 1 - invisible", "1's Visible child"],
'id': mocker.ANY, 'id': mocker.ANY,
'parent_id': None, 'parent_id': None,
'users_with_permission': [active_user_with_permissions['id']] 'users_with_permission': [active_user_with_permissions['id']]
}, },
{ {
'name': "Parent 1 - invisible / 1's Invisible child / 1's Visible grandchild", 'name': ["Parent 1 - invisible", ["1's Invisible child", "1's Visible grandchild"]],
'id': mocker.ANY, 'id': mocker.ANY,
'parent_id': None, 'parent_id': None,
'users_with_permission': [active_user_with_permissions['id']] 'users_with_permission': [active_user_with_permissions['id']]