From 3c4a186a254c42d2ffca387e4592efd3eecd2318 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Tue, 18 Dec 2018 14:40:53 +0000 Subject: [PATCH 01/11] Display the template folders nested on the move template/folder form Before, all the folders were displayed in a list which was ordered but not nested. This changes the move form to nest the template folders. --- app/main/forms.py | 36 +++++++++---- app/main/views/templates.py | 3 +- app/templates/components/radios.html | 50 ++++++++++++++++++- app/templates/views/templates/_move_to.html | 4 +- tests/app/main/views/test_template_folders.py | 10 ++-- 5 files changed, 86 insertions(+), 17 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 79a935ae5..18ab60fa7 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -742,6 +742,26 @@ class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): pass +class NestedRadioField(RadioFieldWithNoneOption): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + def children(self): + child_map = {} + child_ids = [ + folder['id'] for folder in self.all_template_folders + if folder['parent_id'] is None] + + child_map[None] = [option for idx, option in enumerate(self) if option.data in child_ids or idx == 0] + + for option in self: + child_ids = [ + folder['id'] for folder in self.all_template_folders + if folder['parent_id'] == option.data] + child_map[option.data] = [option for option in self if option.data in child_ids] + return child_map + + class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField): pass @@ -1207,7 +1227,6 @@ class TemplateAndFoldersSelectionForm(Form): self, all_template_folders, template_list, - current_folder_id, allow_adding_letter_template, allow_adding_copy_of_template, *args, @@ -1221,13 +1240,10 @@ class TemplateAndFoldersSelectionForm(Form): self.op = None self.is_move_op = self.is_add_folder_op = self.is_add_template_op = False - if current_folder_id is None: - current_folder_id = RadioFieldWithNoneOption.NONE_OPTION_VALUE - + self.move_to.all_template_folders = all_template_folders self.move_to.choices = [ (item['id'], item['name']) for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders) - if item['id'] != current_folder_id ] self.add_template_by_template_type.choices = list(filter(None, [ @@ -1262,10 +1278,12 @@ class TemplateAndFoldersSelectionForm(Form): templates_and_folders = MultiCheckboxField('Choose templates or folders', validators=[ required_for_ops('move-to-new-folder', 'move-to-existing-folder') ]) - move_to = RadioFieldWithNoneOption('Choose a folder', validators=[ - Optional(), - required_for_ops('move-to-new-folder', 'move-to-existing-folder') - ]) + move_to = NestedRadioField( + 'Choose a folder', + validators=[ + Optional(), + required_for_ops('move-to-new-folder', 'move-to-existing-folder') + ]) add_new_folder_name = StringField('Folder name', validators=[required_for_ops('add-new-folder')]) move_to_new_folder_name = StringField('Folder name', validators=[required_for_ops('move-to-new-folder')]) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 191cac718..c0617dab0 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -116,13 +116,13 @@ def choose_template(service_id, template_type='all', template_folder_id=None): all_template_folders=current_service.all_template_folders, template_list=template_list, template_type=template_type, - current_folder_id=template_folder_id, allow_adding_letter_template=current_service.has_permission('letter'), allow_adding_copy_of_template=( current_service.all_templates or len(user_api_client.get_service_ids_for_user(current_user)) > 1 ), ) + if request.method == 'POST' and templates_and_folders_form.validate_on_submit(): if not can_manage_folders(): abort(403) @@ -149,6 +149,7 @@ def choose_template(service_id, template_type='all', template_folder_id=None): template_type=template_type, search_form=SearchTemplatesForm(), templates_and_folders_form=templates_and_folders_form, + move_to_children=templates_and_folders_form.move_to.children() ) diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index 8b3ee19dc..84ac4bc2d 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -14,6 +14,43 @@ {% endcall %} {% endmacro %} + +{% macro radio_list( + options, + child_map, + disable=[], + option_hints={} +) %} + +{% endmacro %} + + +{% macro radios_nested( + field, + child_map, + hint=None, + disable=[], + option_hints={}, + hide_legend=False +) %} + {% set disable = [current_option_id] %} + {% call radios_wrapper( + field, hint, disable, option_hints, hide_legend + ) %} + {{ radio_list(child_map[None], child_map, disable) }} + {% endcall %} +{% endmacro %} + {% macro radios_wrapper(field, hint=None, disable=[], option_hints={}, hide_legend=False) %}
@@ -37,8 +74,12 @@
{% endmacro %} -{% macro radio(option, disable=[], option_hints={}, data_target=None) %} +{% macro radio(option, disable=[], option_hints={}, data_target=None, as_list_item=False) %} + {% if as_list_item %} +
  • + {% else %}
    + {% endif %} {% endif %} + {% if caller %} + {{ caller() }} + {% endif %} + {% if as_list_item %} +
    • + {% else %} + {% endif %} {% endmacro %} diff --git a/app/templates/views/templates/_move_to.html b/app/templates/views/templates/_move_to.html index 27e8f1124..02c014c09 100644 --- a/app/templates/views/templates/_move_to.html +++ b/app/templates/views/templates/_move_to.html @@ -1,4 +1,4 @@ -{% from "components/radios.html" import radios %} +{% from "components/radios.html" import radios, radios_nested %} {% from "components/page-footer.html" import page_footer %}
    @@ -6,7 +6,7 @@ {% if templates_and_folders_form.move_to.choices and template_list.templates_to_show %}
    - {{ radios(templates_and_folders_form.move_to) }} + {{ radios_nested(templates_and_folders_form.move_to, move_to_children, current_option) }} {{ page_footer('Move', button_name='operation', button_value='move-to-existing-folder') }}
    diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 993987771..513b0dd66 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -13,6 +13,7 @@ from tests.conftest import ( normalize_spaces, ) +ROOT_FOLDER_ID = '__NONE__' PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0' CHILD_FOLDER_ID = '92ee1ee0-e4ee-4dcc-b1a7-a5da9ebcfa2b' GRANDCHILD_FOLDER_ID = 'fafe723f-1d39-4a10-865f-e551e03d8886' @@ -791,10 +792,10 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi assert radios == page.select('input[name=move_to]') assert [x['value'] for x in radios] == [ - PARENT_FOLDER_ID, CHILD_FOLDER_ID, FOLDER_ONE_TWO_ID, FOLDER_TWO_ID, + ROOT_FOLDER_ID, PARENT_FOLDER_ID, CHILD_FOLDER_ID, FOLDER_ONE_TWO_ID, FOLDER_TWO_ID, ] assert [x.text.strip() for x in radio_div.select('label')] == [ - 'folder_one', 'folder_one_one', 'folder_one_two', 'folder_two', + 'All templates', 'folder_one', 'folder_one_one', 'folder_one_two', 'folder_two', ] assert set(x['value'] for x in page.find_all('button', {'name': 'operation'})) == { 'unknown', @@ -902,7 +903,7 @@ def test_should_be_able_to_move_a_sub_item( template_folder_id=PARENT_FOLDER_ID, _data={ 'operation': 'move-to-existing-folder', - 'move_to': '__NONE__', + 'move_to': ROOT_FOLDER_ID, 'templates_and_folders': [GRANDCHILD_FOLDER_ID], }, _expected_status=302, @@ -957,7 +958,7 @@ def test_should_be_able_to_move_a_sub_item( 'operation': 'add-new-template', 'templates_and_folders': [], 'move_to_new_folder_name': '', - 'move_to': '__NONE__', + 'move_to': 'ROOT_FOLDER_ID', 'add_template_by_template_type': 'email', }, # add a new template, but don't select anything @@ -1005,6 +1006,7 @@ def test_no_action_if_user_fills_in_ambiguous_fields( ] assert [ + ROOT_FOLDER_ID, FOLDER_TWO_ID, PARENT_FOLDER_ID, ] == [ From 501ec2a5347d73b634a664821e5cbf6dc70f254d Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Mon, 31 Dec 2018 11:11:16 +0000 Subject: [PATCH 02/11] Make all options descend from 'all templates' --- app/main/forms.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 18ab60fa7..dd4fe0403 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -747,18 +747,25 @@ class NestedRadioField(RadioFieldWithNoneOption): super().__init__(*args, **kwargs) def children(self): - child_map = {} - child_ids = [ - folder['id'] for folder in self.all_template_folders - if folder['parent_id'] is None] - - child_map[None] = [option for idx, option in enumerate(self) if option.data in child_ids or idx == 0] + # start map with root option as a single child entry + child_map = {None: [option for option in self + if option.data == self.NONE_OPTION_VALUE]} + # add entries for all other children for option in self: - child_ids = [ - folder['id'] for folder in self.all_template_folders - if folder['parent_id'] == option.data] - child_map[option.data] = [option for option in self if option.data in child_ids] + if option.data == self.NONE_OPTION_VALUE: + child_ids = [ + folder['id'] for folder in self.all_template_folders + if folder['parent_id'] is None] + key = self.NONE_OPTION_VALUE + else: + child_ids = [ + folder['id'] for folder in self.all_template_folders + if folder['parent_id'] == option.data] + key = option.data + + child_map[key] = [option for option in self if option.data in child_ids] + return child_map From 3cf36c592fb6ec6e4f475395cb295e509a450538 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 2 Jan 2019 17:12:18 +0000 Subject: [PATCH 03/11] Change move folder form to not have default radio btn and to show hint Updated the move folder form to add a hint for the radio button for the current folder saying 'current folder'. This hint does not get shown if you are viewing all folders (so you are not inside a folder). Also stopped a default radio button from being selected on the form. --- app/main/forms.py | 1 + app/main/views/templates.py | 4 +- app/templates/components/radios.html | 3 +- app/templates/views/templates/_move_to.html | 2 +- tests/app/main/views/test_template_folders.py | 55 +++++++++++++++++++ 5 files changed, 61 insertions(+), 4 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index dd4fe0403..c114a46fc 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1287,6 +1287,7 @@ class TemplateAndFoldersSelectionForm(Form): ]) move_to = NestedRadioField( 'Choose a folder', + default='', validators=[ Optional(), required_for_ops('move-to-new-folder', 'move-to-existing-folder') diff --git a/app/main/views/templates.py b/app/main/views/templates.py index c0617dab0..ba12244dc 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -122,6 +122,7 @@ def choose_template(service_id, template_type='all', template_folder_id=None): len(user_api_client.get_service_ids_for_user(current_user)) > 1 ), ) + option_hints = {template_folder_id: 'current folder'} if request.method == 'POST' and templates_and_folders_form.validate_on_submit(): if not can_manage_folders(): @@ -149,7 +150,8 @@ def choose_template(service_id, template_type='all', template_folder_id=None): template_type=template_type, search_form=SearchTemplatesForm(), templates_and_folders_form=templates_and_folders_form, - move_to_children=templates_and_folders_form.move_to.children() + move_to_children=templates_and_folders_form.move_to.children(), + option_hints=option_hints ) diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index 84ac4bc2d..c2b8bde5c 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -43,11 +43,10 @@ option_hints={}, hide_legend=False ) %} - {% set disable = [current_option_id] %} {% call radios_wrapper( field, hint, disable, option_hints, hide_legend ) %} - {{ radio_list(child_map[None], child_map, disable) }} + {{ radio_list(child_map[None], child_map, disable, option_hints) }} {% endcall %} {% endmacro %} diff --git a/app/templates/views/templates/_move_to.html b/app/templates/views/templates/_move_to.html index 02c014c09..543905884 100644 --- a/app/templates/views/templates/_move_to.html +++ b/app/templates/views/templates/_move_to.html @@ -6,7 +6,7 @@ {% if templates_and_folders_form.move_to.choices and template_list.templates_to_show %}
    - {{ radios_nested(templates_and_folders_form.move_to, move_to_children, current_option) }} + {{ radios_nested(templates_and_folders_form.move_to, move_to_children, option_hints=option_hints) }} {{ page_footer('Move', button_name='operation', button_value='move-to-existing-folder') }}
    diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 513b0dd66..229884dcf 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -882,6 +882,61 @@ def test_should_not_be_able_to_move_to_existing_folder_if_dont_have_permission( assert mock_move_to_template_folder.called is False +def test_move_folder_form_shows_current_folder_hint_when_in_a_folder( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, +): + service_one['permissions'] += ['edit_folders'] + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'parent_folder', 'parent_id': None}, + {'id': CHILD_FOLDER_ID, 'name': 'child_folder', 'parent_id': PARENT_FOLDER_ID}, + ] + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + template_folder_id=PARENT_FOLDER_ID, + _test_page_title=False + ) + + page.find("input", attrs={"name": "move_to", "value": PARENT_FOLDER_ID}) + + move_form_labels = page.find('div', id='move_to_folder_radios').find_all('label') + + assert len(move_form_labels) == 3 + assert normalize_spaces(move_form_labels[0].text) == 'All templates' + assert normalize_spaces(move_form_labels[1].text) == 'parent_folder current folder' + assert normalize_spaces(move_form_labels[2].text) == 'child_folder' + + +def test_move_folder_form_does_not_show_current_folder_hint_at_the_top_level( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, +): + service_one['permissions'] += ['edit_folders'] + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'parent_folder', 'parent_id': None}, + {'id': CHILD_FOLDER_ID, 'name': 'child_folder', 'parent_id': PARENT_FOLDER_ID}, + ] + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + _test_page_title=False + ) + + page.find("input", attrs={"name": "move_to", "value": PARENT_FOLDER_ID}) + + move_form_labels = page.find('div', id='move_to_folder_radios').find_all('label') + + assert len(move_form_labels) == 3 + assert normalize_spaces(move_form_labels[0].text) == 'All templates' + assert normalize_spaces(move_form_labels[1].text) == 'parent_folder' + assert normalize_spaces(move_form_labels[2].text) == 'child_folder' + + def test_should_be_able_to_move_a_sub_item( client_request, service_one, From 9bce22f76630e46a36ff06116e78055eb5af5a1f Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 3 Jan 2019 14:58:20 +0000 Subject: [PATCH 04/11] Swap order of move_to validators If Optional runs before required_for_ops, it stops the validation chain so it doesn't get to required_for_ops. The move_to field isn't required for the 'move-to-new-folder' operation, so this has been removed. This also adds comments explaining why we set default to an empty string when instantiating the move_to field. --- app/main/forms.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index c114a46fc..b3abce726 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1285,12 +1285,15 @@ class TemplateAndFoldersSelectionForm(Form): templates_and_folders = MultiCheckboxField('Choose templates or folders', validators=[ required_for_ops('move-to-new-folder', 'move-to-existing-folder') ]) + # if no default set, it is set to None, which process_data transforms to '__NONE__' + # this means '__NONE__' (self.ALL_TEMPLATES option) is selected when no form data has been submitted + # set default to empty string so process_data method doesn't perform any transformation move_to = NestedRadioField( 'Choose a folder', default='', validators=[ - Optional(), - required_for_ops('move-to-new-folder', 'move-to-existing-folder') + required_for_ops('move-to-existing-folder'), + Optional() ]) add_new_folder_name = StringField('Folder name', validators=[required_for_ops('add-new-folder')]) move_to_new_folder_name = StringField('Folder name', validators=[required_for_ops('move-to-new-folder')]) From 33800c5f27122d24127b7a4bc17a6ab05cde92dd Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 4 Jan 2019 11:56:15 +0000 Subject: [PATCH 05/11] Add styles for nested lists of radios Assumes lists that are descendants of a radio control should be indented at the same amount as their label text. --- app/assets/stylesheets/app.scss | 5 +++++ app/templates/components/radios.html | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index e80f0ec18..b30cd8e80 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -251,6 +251,11 @@ details .arrow { cursor: default; } +.multiple-choice > .panel { + border-left: none; + padding: 0 0 0 12px; +} + .heading-inline { display: inline-block; } diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index c2b8bde5c..a35f2c0fa 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -19,9 +19,10 @@ options, child_map, disable=[], - option_hints={} + option_hints={}, + top_level=False ) %} -
      + {% for option in options %} {% if child_map[option.data] %} {% call radio(option, disable, option_hints, as_list_item=True) %} @@ -46,7 +47,7 @@ {% call radios_wrapper( field, hint, disable, option_hints, hide_legend ) %} - {{ radio_list(child_map[None], child_map, disable, option_hints) }} + {{ radio_list(child_map[None], child_map, disable, option_hints, top_level=True) }} {% endcall %} {% endmacro %} From 39506c479497c8171d420fd09c907fbd6a25e585 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 4 Jan 2019 13:34:15 +0000 Subject: [PATCH 06/11] =?UTF-8?q?Rename=20=E2=80=98All=20templates?= =?UTF-8?q?=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/main/forms.py | 2 +- tests/app/main/views/test_template_folders.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index b3abce726..5c66f3a6c 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1226,7 +1226,7 @@ class TemplateAndFoldersSelectionForm(Form): """ ALL_TEMPLATES_FOLDER = { - 'name': 'All templates', + 'name': 'Templates', 'id': RadioFieldWithNoneOption.NONE_OPTION_VALUE, } diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 229884dcf..9e6d46117 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -795,7 +795,7 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi ROOT_FOLDER_ID, PARENT_FOLDER_ID, CHILD_FOLDER_ID, FOLDER_ONE_TWO_ID, FOLDER_TWO_ID, ] assert [x.text.strip() for x in radio_div.select('label')] == [ - 'All templates', 'folder_one', 'folder_one_one', 'folder_one_two', 'folder_two', + 'Templates', 'folder_one', 'folder_one_one', 'folder_one_two', 'folder_two', ] assert set(x['value'] for x in page.find_all('button', {'name': 'operation'})) == { 'unknown', @@ -905,7 +905,7 @@ def test_move_folder_form_shows_current_folder_hint_when_in_a_folder( move_form_labels = page.find('div', id='move_to_folder_radios').find_all('label') assert len(move_form_labels) == 3 - assert normalize_spaces(move_form_labels[0].text) == 'All templates' + assert normalize_spaces(move_form_labels[0].text) == 'Templates' assert normalize_spaces(move_form_labels[1].text) == 'parent_folder current folder' assert normalize_spaces(move_form_labels[2].text) == 'child_folder' @@ -932,7 +932,7 @@ def test_move_folder_form_does_not_show_current_folder_hint_at_the_top_level( move_form_labels = page.find('div', id='move_to_folder_radios').find_all('label') assert len(move_form_labels) == 3 - assert normalize_spaces(move_form_labels[0].text) == 'All templates' + assert normalize_spaces(move_form_labels[0].text) == 'Templates' assert normalize_spaces(move_form_labels[1].text) == 'parent_folder' assert normalize_spaces(move_form_labels[2].text) == 'child_folder' From 4aa4970fc92027d33154b43e575f55b4364a980a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 4 Jan 2019 13:44:52 +0000 Subject: [PATCH 07/11] Add grey lines to visually show level of nesting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uses some CSS to draw some grey lines to show which radios descend from which. I don’t feel like the intentation is enough, and it looks a bit messy because the circles of the radio buttons don’t have straight edges easily suggest visual alignment. Copies the design from conditionally revealing content in the design system: https://design-system.service.gov.uk/components/radios/#conditionally-revealing-content Implementation is done with pseudo elements, because borders can’t be positionned exactly enough. --- app/assets/stylesheets/app.scss | 5 -- app/assets/stylesheets/components/pill.scss | 4 +- .../stylesheets/components/radio-select.scss | 57 +++++++++++++++++++ app/templates/components/radios.html | 9 +-- 4 files changed, 64 insertions(+), 11 deletions(-) diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index b30cd8e80..e80f0ec18 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -251,11 +251,6 @@ details .arrow { cursor: default; } -.multiple-choice > .panel { - border-left: none; - padding: 0 0 0 12px; -} - .heading-inline { display: inline-block; } diff --git a/app/assets/stylesheets/components/pill.scss b/app/assets/stylesheets/components/pill.scss index 8258a2738..5139c9632 100644 --- a/app/assets/stylesheets/components/pill.scss +++ b/app/assets/stylesheets/components/pill.scss @@ -57,11 +57,11 @@ border: 2px solid $black; outline: 1px solid rgba($white, 0.1); position: relative; - z-index: 1000; + z-index: 10; color: $text-colour; &:focus { - z-index: 10; + z-index: 1000; outline: 3px solid $yellow; } diff --git a/app/assets/stylesheets/components/radio-select.scss b/app/assets/stylesheets/components/radio-select.scss index 59586121a..3889218ac 100644 --- a/app/assets/stylesheets/components/radio-select.scss +++ b/app/assets/stylesheets/components/radio-select.scss @@ -57,3 +57,60 @@ } } + +.radios-nested { + + margin-bottom: 10px; + + .multiple-choice { + + $circle-diameter: 39px; + $border-thickness: 4px; + $border-indent: ($circle-diameter / 2) - ($border-thickness / 2); + + float: none; + position: relative; + + &:before { + content: ""; + position: absolute; + bottom: 0; + left: $border-indent; + width: $border-thickness; + height: 100%; + background: $border-colour; + } + + label { + float: none; + } + + [type=radio]+label::before { + // To overlap the grey inset line + background: $white; + } + + ul { + // To equalise the spacing between the line and the top/bottom of + // the radio + margin-top: 5px; + margin-bottom: -5px; + } + + .block-label-hint { + &:after { + // Adds an little extra segment of line alongside the ‘current folder’ + // hint so that it extends all the way down to the next radio + content: ""; + position: absolute; + bottom: -5px; + left: $border-indent; + width: $border-thickness; + height: 25px; + background: $border-colour; + } + } + + } + +} diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index a35f2c0fa..351158acb 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -19,10 +19,9 @@ options, child_map, disable=[], - option_hints={}, - top_level=False + option_hints={} ) %} - +
        {% for option in options %} {% if child_map[option.data] %} {% call radio(option, disable, option_hints, as_list_item=True) %} @@ -47,7 +46,9 @@ {% call radios_wrapper( field, hint, disable, option_hints, hide_legend ) %} - {{ radio_list(child_map[None], child_map, disable, option_hints, top_level=True) }} +
        + {{ radio_list(child_map[None], child_map, disable, option_hints) }} +
        {% endcall %} {% endmacro %} From 3be03072a79caa3f9da6c9e5c1f6071209a68b0f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 4 Jan 2019 14:56:22 +0000 Subject: [PATCH 08/11] Reflect expanded scope of radio SASS file --- .../stylesheets/components/{radio-select.scss => radios.scss} | 0 app/assets/stylesheets/main.scss | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename app/assets/stylesheets/components/{radio-select.scss => radios.scss} (100%) diff --git a/app/assets/stylesheets/components/radio-select.scss b/app/assets/stylesheets/components/radios.scss similarity index 100% rename from app/assets/stylesheets/components/radio-select.scss rename to app/assets/stylesheets/components/radios.scss diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index f4944ecac..f8fc5787c 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -46,7 +46,7 @@ $path: '/static/images/'; @import 'components/email-message'; @import 'components/api-key'; @import 'components/vendor/previous-next-navigation'; -@import 'components/radio-select'; +@import 'components/radios'; @import 'components/pill'; @import 'components/secondary-button'; @import 'components/show-more'; From f5321346c4b03b749c4afb6476eeca4d5d7306d4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 4 Jan 2019 15:30:39 +0000 Subject: [PATCH 09/11] Add left padding to nested radios This makes them align horizontally with the label of their parent, so that the layout is a bit calmer. --- app/assets/stylesheets/components/radios.scss | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/components/radios.scss b/app/assets/stylesheets/components/radios.scss index 3889218ac..bd0de4268 100644 --- a/app/assets/stylesheets/components/radios.scss +++ b/app/assets/stylesheets/components/radios.scss @@ -67,6 +67,7 @@ $circle-diameter: 39px; $border-thickness: 4px; $border-indent: ($circle-diameter / 2) - ($border-thickness / 2); + $border-colour: $border-colour; float: none; position: relative; @@ -95,6 +96,7 @@ // the radio margin-top: 5px; margin-bottom: -5px; + padding-left: 12px; } .block-label-hint { @@ -103,7 +105,7 @@ // hint so that it extends all the way down to the next radio content: ""; position: absolute; - bottom: -5px; + top: $circle-diameter + 5px; left: $border-indent; width: $border-thickness; height: 25px; From d907fc9ce6d173383f0704d88c06468eda9b90d8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 4 Jan 2019 15:40:34 +0000 Subject: [PATCH 10/11] Update tests/app/main/views/test_template_folders.py Remove quotes around variable. Were added by mistake. Co-Authored-By: tombye --- tests/app/main/views/test_template_folders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 9e6d46117..0c6a0a51c 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -1013,7 +1013,7 @@ def test_should_be_able_to_move_a_sub_item( 'operation': 'add-new-template', 'templates_and_folders': [], 'move_to_new_folder_name': '', - 'move_to': 'ROOT_FOLDER_ID', + 'move_to': ROOT_FOLDER_ID, 'add_template_by_template_type': 'email', }, # add a new template, but don't select anything From bdc4a1056de6eaa69573c8819e6287dded42c122 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 4 Jan 2019 17:21:54 +0000 Subject: [PATCH 11/11] Add a test to ensure move_to has no default folder As requested in: https://github.com/alphagov/notifications-admin/pull/2630#pullrequestreview-189414780 --- tests/app/main/views/test_template_folders.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index 0c6a0a51c..296b3d94e 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -806,6 +806,33 @@ def test_should_show_radios_and_buttons_for_move_destination_if_correct_permissi } +def test_move_to_shouldnt_select_a_folder_by_default( + client_request, + mocker, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_has_no_jobs, + fake_uuid, + active_user_with_permissions +): + service_one['permissions'] += ['edit_folders'] + + client_request.login(active_user_with_permissions) + + FOLDER_TWO_ID = str(uuid.uuid4()) + mock_get_template_folders.return_value = [ + {'id': PARENT_FOLDER_ID, 'name': 'folder_one', 'parent_id': None}, + {'id': FOLDER_TWO_ID, 'name': 'folder_two', 'parent_id': None}, + ] + page = client_request.get( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + ) + checked_radio = page.find('input', attrs={'name': 'move_to', 'checked': 'checked'}) + assert checked_radio is None + + def test_should_be_able_to_move_to_existing_folder( client_request, service_one,