From 187e23315cf77eeed635c389ff9f0007a1316aa2 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 26 Feb 2020 16:58:33 +0000 Subject: [PATCH 01/13] Bring in Jinja and Sass for checkboxes component --- app/assets/stylesheets/govuk-frontend/_all.scss | 1 + gulpfile.js | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/govuk-frontend/_all.scss b/app/assets/stylesheets/govuk-frontend/_all.scss index 9b8755367..6776fb95a 100644 --- a/app/assets/stylesheets/govuk-frontend/_all.scss +++ b/app/assets/stylesheets/govuk-frontend/_all.scss @@ -26,6 +26,7 @@ $govuk-assets-path: "/static/"; @import 'components/button/_button'; @import 'components/details/_details'; @import 'components/radios/_radios'; +@import 'components/checkboxes/_checkboxes'; @import "utilities/all"; @import "overrides/all"; diff --git a/gulpfile.js b/gulpfile.js index fc9fab7a6..69aa425a6 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -65,7 +65,12 @@ const copy = { 'footer', 'back-link', 'details', - 'button' + 'button', + 'error-message', + 'fieldset', + 'hint', + 'label', + 'checkboxes' ]; let done = 0; From 012d5767d78e728ed9c2ed9f046efdca162d2ea4 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 2 Apr 2020 18:06:01 +0100 Subject: [PATCH 02/13] Add govukCheckboxesField govukCheckboxesField subclasses SelectMultipleField and overwrites how it renders HTML to let us use the GOVUK Checkboxes component while retaining all the functionality of WTForms fields. Based on work on github.com/richardjpope/recourse: https://github.com/richardjpope/recourse/blob/master/recourse/forms.py#L6 --- app/main/forms.py | 64 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 0312537ef..0eaf82278 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -3,7 +3,7 @@ from datetime import datetime, timedelta from itertools import chain import pytz -from flask import request +from flask import Markup, render_template, request from flask_login import current_user from flask_wtf import FlaskForm as Form from flask_wtf.file import FileAllowed @@ -487,12 +487,66 @@ class RegisterUserFromOrgInviteForm(StripWhitespaceForm): auth_type = HiddenField('auth_type', validators=[DataRequired()]) -PermissionsAbstract = type("PermissionsAbstract", (StripWhitespaceForm,), { - permission: BooleanField(label) for permission, label in permissions -}) +# based on work done by @richardjpope: https://github.com/richardjpope/recourse/blob/master/recourse/forms.py#L6 +class govukCheckboxesField(SelectMultipleField): + + def __init__(self, label='', validators=None, param_extensions=None, **kwargs): + super(govukCheckboxesField, self).__init__(label, validators, **kwargs) + # default choices to a single True Boolean + if 'choices' not in kwargs: + self.choices = [('y', label)] + self.param_extensions = param_extensions + + # self.__call__ renders the HTML for the field by: + # 1. delegating to self.meta.render_field which + # 2. calls field.widget + # this bypasses that by making self.widget a method with the same interface as widget.__call__ + def widget(self, field, **kwargs): + items = [] + + # error messages + error_message = None + if field.errors: + error_message = {"text": " ".join(field.errors).strip()} + + # convert options to ones govuk understands + for option in field: + items.append({ + "name": option.name, + "id": option.id, + "text": option.label.text, + "value": option.data, + "checked": option.checked + }) + + params = { + 'idPrefix': field.id, + 'name': field.name, + 'errorMessage': error_message, + 'items': items + } + + # add HTML to group items if more than one + if len(items) > 1: + params.update({ + "fieldset": { + "attributes": {"id": field.name}, + "legend": { + "text": field.label.text, + "classes": "govuk-fieldset__legend--s" + } + } + }) + + # extend default params with any sent in + if self.param_extensions: + params.update(self.param_extensions) + + return Markup( + render_template('vendor/govuk-frontend/components/checkboxes/template.njk', params=params)) -class PermissionsForm(PermissionsAbstract): +class PermissionsForm(StripWhitespaceForm): def __init__(self, all_template_folders=None, *args, **kwargs): super().__init__(*args, **kwargs) self.folder_permissions.choices = [] From 06027b447c932da6b0782ee4048c4bb936d0567e Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 23 Apr 2020 12:12:55 +0100 Subject: [PATCH 03/13] Add mixin & field to make collapsible checkboxes Allows checkboxes to be collapsed so they take up less space in the page. The collapsed state includes a live summary tracking which of them are selected. Includes changes to the JS for collapsible checkboxes to make it work with the GOVUK Checkboxes component HTML. --- .../javascripts/collapsibleCheckboxes.js | 14 +++++---- app/main/forms.py | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/collapsibleCheckboxes.js b/app/assets/javascripts/collapsibleCheckboxes.js index 11c14a2e4..0333b8dc3 100644 --- a/app/assets/javascripts/collapsibleCheckboxes.js +++ b/app/assets/javascripts/collapsibleCheckboxes.js @@ -5,7 +5,7 @@ function Summary (module) { this.module = module; - this.$el = module.$formGroup.find('.selection-summary'); + this.$el = module.$formGroup.find('.selection-summary').first(); this.fieldLabel = module.fieldLabel; this.total = module.total; this.addContent(); @@ -25,6 +25,7 @@ if (this.fieldLabel === 'folder') { this.$text.addClass('selection-summary__text--folders'); } this.$el.append(this.$text); + this.module.$formGroup.find('.govuk-hint').remove(); }; Summary.prototype.update = function(selection) { let template; @@ -86,12 +87,13 @@ .focus(); }; CollapsibleCheckboxes.prototype.start = function(component) { - this.$formGroup = $(component); - this.$fieldset = this.$formGroup.find('fieldset'); + this.$component = $(component); + this.$formGroup = this.$component.find('.govuk-form-group').first(); + this.$fieldset = this.$formGroup.find('fieldset').first(); this.$checkboxes = this.$fieldset.find('input[type=checkbox]'); - this.fieldLabel = this.$formGroup.data('fieldLabel'); + this.fieldLabel = this.$component.data('fieldLabel'); this.total = this.$checkboxes.length; - this.legendText = this.$fieldset.find('legend').text().trim(); + this.legendText = this.$fieldset.find('legend').first().text().trim(); this.expanded = false; this.addHeadingHideLegend(); @@ -113,7 +115,7 @@ }; CollapsibleCheckboxes.prototype.getSelection = function() { return this.$checkboxes.filter(':checked').length; }; CollapsibleCheckboxes.prototype.addHeadingHideLegend = function() { - const headingLevel = this.$formGroup.data('heading-level') || '2'; + const headingLevel = this.$component.data('heading-level') || '2'; this.$heading = $(`${this.legendText}`); this.$fieldset.before(this.$heading); diff --git a/app/main/forms.py b/app/main/forms.py index 0eaf82278..332739f62 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -546,6 +546,36 @@ class govukCheckboxesField(SelectMultipleField): render_template('vendor/govuk-frontend/components/checkboxes/template.njk', params=params)) +# Extends fields using the govukCheckboxesField interface to wrap their render in HTML needed by the collapsible JS +class govukCollapsibleCheckboxesMixin: + def __init__(self, label='', validators=None, field_label='', param_extensions=None, **kwargs): + + self.field_label = field_label + + def widget(self, field, **kwargs): + + # add a blank hint to act as an ARIA live-region + if self.param_extensions is not None: + self.param_extensions.update( + {"hint": {"html": "
"}}) + else: + self.param_extensions = \ + {"hint": {"html": "
"}} + + # wrap the checkboxes HTML in the HTML needed by the collapisble JS + return Markup( + f'
' + f' {super(govukCollapsibleCheckboxesMixin, self).widget(field, **kwargs)}' + f'
' + ) + + +class govukCollapsibleCheckboxesField(govukCollapsibleCheckboxesMixin, govukCheckboxesField): + pass + + class PermissionsForm(StripWhitespaceForm): def __init__(self, all_template_folders=None, *args, **kwargs): super().__init__(*args, **kwargs) From efc4aff4fe67aecf92d426b08b443894611190e2 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 23 Apr 2020 12:15:24 +0100 Subject: [PATCH 04/13] Add govukCollapsibleNestedCheckboxesField Includes: 1. changes to make NestedFieldMixin work with new fields and CSS for nested checkboxes 2. adds custom version of GOVUK checkboxes component to allow us to: - add classes to elements currently inaccessible - wrap the checkboxes in a list - add child checkboxes to each checkbox (making tree structures possible through recursion Change 2. should be pushed upstream to the GOVUK Design System as a proposal for changes to the GOVUK Checkboxes component. --- .../stylesheets/components/checkboxes.scss | 28 ++++ app/main/forms.py | 89 ++++++++++-- .../forms/fields/checkboxes/macro.njk | 4 + .../forms/fields/checkboxes/template.njk | 134 ++++++++++++++++++ 4 files changed, 241 insertions(+), 14 deletions(-) create mode 100644 app/templates/forms/fields/checkboxes/macro.njk create mode 100644 app/templates/forms/fields/checkboxes/template.njk diff --git a/app/assets/stylesheets/components/checkboxes.scss b/app/assets/stylesheets/components/checkboxes.scss index 3986c65bf..094c37d7a 100644 --- a/app/assets/stylesheets/components/checkboxes.scss +++ b/app/assets/stylesheets/components/checkboxes.scss @@ -1,3 +1,7 @@ +// Taken from https://github.com/alphagov/govuk-frontend/blob/v2.13.0/src/components/checkboxes/_checkboxes.scss +$govuk-touch-target-size: 44px; +$govuk-checkboxes-size: 40px; + .selection-summary { .selection-summary__text { @@ -65,6 +69,30 @@ } +.govuk-form-group--nested { + + $border-thickness: $govuk-touch-target-size - $govuk-checkboxes-size; + $border-indent: $govuk-touch-target-size / 2; + + position: relative; + + // To equalise the spacing between the line and the top/bottom of + // the radio + margin-top: govuk-spacing(1) + ($border-thickness / 2); + margin-bottom: govuk-spacing(1) * -1; + padding-left: govuk-spacing(2) + 2; + + &:before { + content: ""; + position: absolute; + bottom: 0; + left: $border-indent * -1; + width: $border-thickness; + height: 100%; + background: $govuk-border-colour; + } +} + .selection-content { margin-bottom: govuk-spacing(4); diff --git a/app/main/forms.py b/app/main/forms.py index 332739f62..86db50f91 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -16,6 +16,7 @@ from notifications_utils.recipients import ( normalise_phone_number, validate_phone_number, ) +from werkzeug.utils import cached_property from wtforms import ( BooleanField, DateField, @@ -317,13 +318,16 @@ class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): class NestedFieldMixin: + def children(self): + # 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: + # assign all options with a NONE_OPTION_VALUE (not always None) to the None key if option.data == self.NONE_OPTION_VALUE: child_ids = [ folder['id'] for folder in self.all_template_folders @@ -339,6 +343,47 @@ class NestedFieldMixin: return child_map + # to be used as the only version of .children once radios are converted + @cached_property + def _children(self): + return self.children() + + def get_items_from_options(self, field): + items = [] + + for option in self._children[None]: + item = self.get_item_from_option(option) + if option.data in self._children: + item['children'] = self.render_children(field.name, option.label.text, self._children[option.data]) + items.append(item) + + return items + + def render_children(self, name, label, options): + params = { + "name": name, + "fieldset": { + "legend": { + "text": label, + "classes": "govuk-visually-hidden" + } + }, + "formGroup": { + "classes": "govuk-form-group--nested" + }, + "asList": True, + "items": [] + } + for option in options: + item = self.get_item_from_option(option) + + if len(self._children[option.data]): + item['children'] = self.render_children(name, option.label.text, self._children[option.data]) + + params['items'].append(item) + + return render_template('forms/fields/checkboxes/template.njk', params=params) + class NestedRadioField(RadioFieldWithNoneOption, NestedFieldMixin): pass @@ -490,6 +535,8 @@ class RegisterUserFromOrgInviteForm(StripWhitespaceForm): # based on work done by @richardjpope: https://github.com/richardjpope/recourse/blob/master/recourse/forms.py#L6 class govukCheckboxesField(SelectMultipleField): + render_as_list = False + def __init__(self, label='', validators=None, param_extensions=None, **kwargs): super(govukCheckboxesField, self).__init__(label, validators, **kwargs) # default choices to a single True Boolean @@ -497,30 +544,34 @@ class govukCheckboxesField(SelectMultipleField): self.choices = [('y', label)] self.param_extensions = param_extensions + def get_item_from_option(self, option): + return { + "name": option.name, + "id": option.id, + "text": option.label.text, + "value": str(option.data), # to protect against non-string types like uuids + "checked": option.checked + } + + def get_items_from_options(self, field): + return [self.get_item_from_option(option) for option in field] + # self.__call__ renders the HTML for the field by: # 1. delegating to self.meta.render_field which # 2. calls field.widget # this bypasses that by making self.widget a method with the same interface as widget.__call__ def widget(self, field, **kwargs): - items = [] # error messages error_message = None if field.errors: error_message = {"text": " ".join(field.errors).strip()} - # convert options to ones govuk understands - for option in field: - items.append({ - "name": option.name, - "id": option.id, - "text": option.label.text, - "value": option.data, - "checked": option.checked - }) + # returns either a list or a hierarchy of lists + # depending on how get_items_from_options is implemented + items = self.get_items_from_options(field) params = { - 'idPrefix': field.id, 'name': field.name, 'errorMessage': error_message, 'items': items @@ -535,21 +586,23 @@ class govukCheckboxesField(SelectMultipleField): "text": field.label.text, "classes": "govuk-fieldset__legend--s" } - } + }, + "asList": self.render_as_list }) # extend default params with any sent in - if self.param_extensions: + if self.param_extensions.keys(): params.update(self.param_extensions) return Markup( - render_template('vendor/govuk-frontend/components/checkboxes/template.njk', params=params)) + render_template('forms/fields/checkboxes/macro.njk', params=params)) # Extends fields using the govukCheckboxesField interface to wrap their render in HTML needed by the collapsible JS class govukCollapsibleCheckboxesMixin: def __init__(self, label='', validators=None, field_label='', param_extensions=None, **kwargs): + super(govukCollapsibleCheckboxesMixin, self).__init__(label, validators, param_extensions, **kwargs) self.field_label = field_label def widget(self, field, **kwargs): @@ -576,6 +629,14 @@ class govukCollapsibleCheckboxesField(govukCollapsibleCheckboxesMixin, govukChec pass +# govukCollapsibleCheckboxesMixin adds an ARIA live-region to the hint and wraps the render in HTML needed by the +# collapsible JS +# NestedFieldMixin puts the items into a tree hierarchy, pre-rendering the sub-trees of the top-level items +class govukCollapsibleNestedCheckboxesField(govukCollapsibleCheckboxesMixin, NestedFieldMixin, govukCheckboxesField): + NONE_OPTION_VALUE = None + render_as_list = True + + class PermissionsForm(StripWhitespaceForm): def __init__(self, all_template_folders=None, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/app/templates/forms/fields/checkboxes/macro.njk b/app/templates/forms/fields/checkboxes/macro.njk new file mode 100644 index 000000000..c91452053 --- /dev/null +++ b/app/templates/forms/fields/checkboxes/macro.njk @@ -0,0 +1,4 @@ +{%- macro govukCheckboxes(params) %} + {%- include "./template.njk" -%} +{%- endmacro %} +{{ govukCheckboxes(params) }} diff --git a/app/templates/forms/fields/checkboxes/template.njk b/app/templates/forms/fields/checkboxes/template.njk new file mode 100644 index 000000000..2ac9cf68b --- /dev/null +++ b/app/templates/forms/fields/checkboxes/template.njk @@ -0,0 +1,134 @@ +{% from "components/error-message/macro.njk" import govukErrorMessage -%} +{% from "components/fieldset/macro.njk" import govukFieldset %} +{% from "components/hint/macro.njk" import govukHint %} +{% from "components/label/macro.njk" import govukLabel %} + + +{#- Copied from https://github.com/alphagov/govuk-frontend/blob/v2.13.0/src/components/checkboxes/template.njk + Changes: + - `classes` option added to `item` allow custom classes on the `.govuk-checkboxes__item` element + - `classes` option added to `item.hint` allow custom classes on the `.govuk-hint` element (added to GOVUK Frontend in v3.5.0 - remove when we update) + - `asList` option added the root `params` object to allow setting of the `.govuk-checkboxes` and `.govuk-checkboxes__item` element types + - `children` option added to `item` allowing the sending in of prerendered child checkboxes (allowing the creation of tree structures through recursion) -#} +{#- If an id 'prefix' is not passed, fall back to using the name attribute + instead. We need this for error messages and hints as well -#} +{% set idPrefix = params.idPrefix if params.idPrefix else params.name %} + +{#- a record of other elements that we need to associate with the input using + aria-describedby – for example hints or error messages -#} +{% set describedBy = params.describedBy if params.describedBy else "" %} +{% if params.fieldset.describedBy %} + {% set describedBy = params.fieldset.describedBy %} +{% endif %} + +{#- set the types of element used for the checkboxes and their group based on + whether asList is set -#} +{% if params.asList %} + {% set groupElement = 'ul' %} + {% set groupItemElement = 'li' %} +{% else %} + {% set groupElement = 'div' %} + {% set groupItemElement = 'div' %} +{% endif %} + +{% set isConditional = false %} +{% for item in params.items %} + {% if item.conditional %} + {% set isConditional = true %} + {% endif %} +{% endfor %} + +{#- fieldset is false by default -#} +{% set hasFieldset = true if params.fieldset else false %} + +{#- Capture the HTML so we can optionally nest it in a fieldset -#} +{% set innerHtml %} +{% if params.hint %} + {% set hintId = idPrefix + '-hint' %} + {% set describedBy = describedBy + ' ' + hintId if describedBy else hintId %} + {{ govukHint({ + id: hintId, + classes: params.hint.classes, + attributes: params.hint.attributes, + html: params.hint.html, + text: params.hint.text + }) | indent(2) | trim }} +{% endif %} +{% if params.errorMessage %} + {% set errorId = idPrefix + '-error' %} + {% set describedBy = describedBy + ' ' + errorId if describedBy else errorId %} + {{ govukErrorMessage({ + id: errorId, + classes: params.errorMessage.classes, + attributes: params.errorMessage.attributes, + html: params.errorMessage.html, + text: params.errorMessage.text, + visuallyHiddenText: params.errorMessage.visuallyHiddenText + }) | indent(2) | trim }} +{% endif %} + <{{ groupElement }} class="govuk-checkboxes {%- if params.classes %} {{ params.classes }}{% endif %}" + {%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %} + {%- if isConditional %} data-module="checkboxes"{% endif -%}> + {% for item in params.items %} + {% set id = item.id if item.id else idPrefix + "-" + loop.index %} + {% set name = item.name if item.name else params.name %} + {% set conditionalId = "conditional-" + id %} + {% set hasHint = true if item.hint.text or item.hint.html %} + {% set itemHintId = id + "-item-hint" if hasHint else "" %} + {% set itemDescribedBy = describedBy if not hasFieldset else "" %} + {% set itemDescribedBy = (itemDescribedBy + " " + itemHintId) | trim %} + <{{ groupItemElement }} class="govuk-checkboxes__item {%- if item.classes %} {{ item.classes }}{% endif %}"> + + {{ govukLabel({ + html: item.html, + text: item.text, + classes: 'govuk-checkboxes__label' + (' ' + item.label.classes if item.label.classes), + attributes: item.label.attributes, + for: id + }) | indent(6) | trim }} + {%- if hasHint %} + {{ govukHint({ + id: itemHintId, + classes: 'govuk-checkboxes__hint' + (' ' + item.hint.classes if item.hint.classes), + attributes: item.hint.attributes, + html: item.hint.html, + text: item.hint.text + }) | indent(6) | trim }} + {%- endif %} + {%- if item.children %} + {{ item.children | safe }} + {%- endif %} + {% if params.asList and item.conditional %} +
+ {{ item.conditional.html | safe }} +
+ {% endif %} + + {% if not params.asList and item.conditional %} +
+ {{ item.conditional.html | safe }} +
+ {% endif %} + {% endfor %} + +{% endset -%} + +
+{% if params.fieldset %} + {% call govukFieldset({ + describedBy: describedBy, + classes: params.fieldset.classes, + attributes: params.fieldset.attributes, + legend: params.fieldset.legend + }) %} + {{ innerHtml | trim | safe }} + {% endcall %} +{% else %} + {{ innerHtml | trim | safe }} +{% endif %} +
From 691034ef85f100af776a3fe7befe1a85d9afb996 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Tue, 28 Apr 2020 17:20:09 +0100 Subject: [PATCH 05/13] Fix nested checkboxes with single top-level node Nested checkboxes with a single top-level node will only have one item in their `items` list. This is because the other choices are children of that list item. This means we need to check the `choices` attribute, which lists all the checkboxes, to see if they should be marked as a group (by being wrapped in a `
`) or not. --- app/main/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/forms.py b/app/main/forms.py index 86db50f91..19b96da17 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -578,7 +578,7 @@ class govukCheckboxesField(SelectMultipleField): } # add HTML to group items if more than one - if len(items) > 1: + if len(field.choices) > 1: params.update({ "fieldset": { "attributes": {"id": field.name}, From b5f3d1e6e6c338cb230a4ee36af29e77beee4f76 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 7 May 2020 15:30:20 +0100 Subject: [PATCH 06/13] Add govukCheckboxField for single checkboxes Single checkboxes are distinct because: - they don't need to be wrapped in a `
` - they are a subclass of BooleanField so their data is either True or False --- app/main/forms.py | 99 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 81 insertions(+), 18 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 19b96da17..6681f9b7e 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -532,6 +532,73 @@ class RegisterUserFromOrgInviteForm(StripWhitespaceForm): auth_type = HiddenField('auth_type', validators=[DataRequired()]) +class govukCheckboxField(BooleanField): + + def __init__(self, label='', validators=None, param_extensions=None, **kwargs): + super(govukCheckboxField, self).__init__(label, validators, false_values=None, **kwargs) + self.param_extensions = param_extensions + + def extend_params(self, params, extensions): + items = None + param_items = len(params['items']) if 'items' in params else 0 + + # split items off from params to make it a pure dict + if 'items' in extensions: + items = extensions['items'] + del extensions['items'] + + # merge dicts + params.update(extensions) + + # merge items + if items: + if 'items' not in params: + params['items'] = items + else: + for idx, _item in enumerate(items): + if idx >= param_items: + params['items'].append(items[idx]) + else: + params['items'][idx].update(items[idx]) + + # self.__call__ renders the HTML for the field by: + # 1. delegating to self.meta.render_field which + # 2. calls field.widget + # this bypasses that by making self.widget a method with the same interface as widget.__call__ + def widget(self, field, param_extensions=None, **kwargs): + + # error messages + error_message = None + if field.errors: + error_message = {"text": " ".join(field.errors).strip()} + + params = { + 'name': field.name, + 'errorMessage': error_message, + 'items': [ + { + "name": field.name, + "id": field.id, + "text": field.label.text, + "value": 'y', + "checked": field.data + } + ] + + } + + # extend default params with any sent in during instantiation + if self.param_extensions: + self.extend_params(params, self.param_extensions) + + # add any sent in though use in templates + if param_extensions: + self.extend_params(params, param_extensions) + + return Markup( + render_template('forms/fields/checkboxes/macro.njk', params=params)) + + # based on work done by @richardjpope: https://github.com/richardjpope/recourse/blob/master/recourse/forms.py#L6 class govukCheckboxesField(SelectMultipleField): @@ -539,9 +606,6 @@ class govukCheckboxesField(SelectMultipleField): def __init__(self, label='', validators=None, param_extensions=None, **kwargs): super(govukCheckboxesField, self).__init__(label, validators, **kwargs) - # default choices to a single True Boolean - if 'choices' not in kwargs: - self.choices = [('y', label)] self.param_extensions = param_extensions def get_item_from_option(self, option): @@ -573,26 +637,25 @@ class govukCheckboxesField(SelectMultipleField): params = { 'name': field.name, + "fieldset": { + "attributes": {"id": field.name}, + "legend": { + "text": field.label.text, + "classes": "govuk-fieldset__legend--s" + } + }, + "asList": self.render_as_list, 'errorMessage': error_message, 'items': items } - # add HTML to group items if more than one - if len(field.choices) > 1: - params.update({ - "fieldset": { - "attributes": {"id": field.name}, - "legend": { - "text": field.label.text, - "classes": "govuk-fieldset__legend--s" - } - }, - "asList": self.render_as_list - }) + # extend default params with any sent in during instantiation + if self.param_extensions: + self.extend_params(params, self.param_extensions) - # extend default params with any sent in - if self.param_extensions.keys(): - params.update(self.param_extensions) + # add any sent in though use in templates + if param_extensions: + self.extend_params(params, param_extensions) return Markup( render_template('forms/fields/checkboxes/macro.njk', params=params)) From a99ca640e3b8f2ecf14d1988d5963af2efa3e34a Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 7 May 2020 15:44:13 +0100 Subject: [PATCH 07/13] Split common checkbox methods off into mixin --- app/main/forms.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 6681f9b7e..cceb12b59 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -532,11 +532,7 @@ class RegisterUserFromOrgInviteForm(StripWhitespaceForm): auth_type = HiddenField('auth_type', validators=[DataRequired()]) -class govukCheckboxField(BooleanField): - - def __init__(self, label='', validators=None, param_extensions=None, **kwargs): - super(govukCheckboxField, self).__init__(label, validators, false_values=None, **kwargs) - self.param_extensions = param_extensions +class govukCheckboxesMixin: def extend_params(self, params, extensions): items = None @@ -561,6 +557,13 @@ class govukCheckboxField(BooleanField): else: params['items'][idx].update(items[idx]) + +class govukCheckboxField(govukCheckboxesMixin, BooleanField): + + def __init__(self, label='', validators=None, param_extensions=None, **kwargs): + super(govukCheckboxField, self).__init__(label, validators, false_values=None, **kwargs) + self.param_extensions = param_extensions + # self.__call__ renders the HTML for the field by: # 1. delegating to self.meta.render_field which # 2. calls field.widget @@ -600,7 +603,7 @@ class govukCheckboxField(BooleanField): # based on work done by @richardjpope: https://github.com/richardjpope/recourse/blob/master/recourse/forms.py#L6 -class govukCheckboxesField(SelectMultipleField): +class govukCheckboxesField(govukCheckboxesMixin, SelectMultipleField): render_as_list = False From e030d7d3a1115f7ec12872d1e660a5d6c8da57a0 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 2 Apr 2020 18:08:50 +0100 Subject: [PATCH 08/13] Update permissions page --- app/main/forms.py | 24 +++++++++++-------- app/main/views/manage_users.py | 4 ++-- .../views/manage-users/permissions.html | 14 ++--------- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index cceb12b59..9eab91803 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -713,7 +713,9 @@ class PermissionsForm(StripWhitespaceForm): (item['id'], item['name']) for item in ([{'name': 'Templates', 'id': None}] + all_template_folders) ] - folder_permissions = NestedCheckboxesField('Folders this team member can see') + folder_permissions = govukCollapsibleNestedCheckboxesField( + 'Folders this team member can see', + field_label='folder') login_authentication = RadioField( 'Sign in using', @@ -725,21 +727,23 @@ class PermissionsForm(StripWhitespaceForm): validators=[DataRequired()] ) - @property - def permissions(self): - return {role for role in roles.keys() if self[role].data is True} - - @property - def permissions_fields(self): - return (getattr(self, permission) for permission, _ in permissions) + permissions_field = govukCheckboxesField( + 'Permssions', + choices=[ + (value, label) for value, label in permissions + ], + param_extensions={ + "hint": {"text": "All team members can see sent messages."} + } + ) @classmethod def from_user(cls, user, service_id, **kwargs): return cls( **kwargs, **{ - role: user.has_permission_for_service(service_id, role) - for role in roles.keys() + "permissions_field": [ + role for role in roles.keys() if user.has_permission_for_service(service_id, role)] }, login_authentication=user.auth_type ) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 8102ba521..f404309fd 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -63,7 +63,7 @@ def invite_user(service_id): current_user.id, service_id, email_address, - form.permissions, + set(form.permissions_field.data), form.login_authentication.data, form.folder_permissions.data, ) @@ -102,7 +102,7 @@ def edit_user_permissions(service_id, user_id): if form.validate_on_submit(): user.set_permissions( service_id, - permissions=form.permissions, + permissions=set(form.permissions_field.data), folder_permissions=form.folder_permissions.data, ) if service_has_email_auth: diff --git a/app/templates/views/manage-users/permissions.html b/app/templates/views/manage-users/permissions.html index e6ee7637d..51169c37a 100644 --- a/app/templates/views/manage-users/permissions.html +++ b/app/templates/views/manage-users/permissions.html @@ -1,20 +1,10 @@ {% from "components/checkbox.html" import checkbox, checkboxes_nested %} {% from "components/radios.html" import radio, radios, conditional_radio_panel %} -
- - Permissions - - - All team members can see sent messages. - - {% for field in form.permissions_fields %} - {{ checkbox(field) }} - {% endfor %} -
+{{ form.permissions_field }} {% if form.folder_permissions.all_template_folders %} - {{ checkboxes_nested(form.folder_permissions, form.folder_permissions.children(), hide_legend=True, collapsible_opts={ 'field': 'folder' }) }} + {{ form.folder_permissions }} {% elif user and user.platform_admin %}

Platform admin users can access all template folders. From 0291fbc1cbf4a26cd6de8070756b06b432a6dda5 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 9 Apr 2020 20:56:17 +0100 Subject: [PATCH 09/13] Update manage folder page --- app/main/forms.py | 4 +++- app/templates/views/templates/manage-template-folder.html | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 9eab91803..e23b43cdd 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1806,7 +1806,9 @@ class TemplateFolderForm(StripWhitespaceForm): (item.id, item.name) for item in all_service_users ] - users_with_permission = MultiCheckboxField('Team members who can see this folder') + users_with_permission = govukCollapsibleCheckboxesField( + 'Team members who can see this folder', + field_label='folder') name = StringField('Folder name', validators=[DataRequired(message='Cannot be empty')]) diff --git a/app/templates/views/templates/manage-template-folder.html b/app/templates/views/templates/manage-template-folder.html index ddc20df80..6c817014b 100644 --- a/app/templates/views/templates/manage-template-folder.html +++ b/app/templates/views/templates/manage-template-folder.html @@ -26,7 +26,7 @@ {% call form_wrapper(action=url_for('main.manage_template_folder', service_id=current_service.id, template_folder_id=template_folder_id)) %} {{ textbox(form.name, width='1-1') }} {% if current_user.has_permissions("manage_service") and form.users_with_permission.all_service_users %} - {{ checkboxes(form.users_with_permission, collapsible_opts={ 'field': 'team member' }) }} + {{ form.users_with_permission }} {% endif %} {{ page_footer( From 33d8171675ea0fe321c863118710ba8ee97cbb33 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 9 Apr 2020 11:51:11 +0100 Subject: [PATCH 10/13] Update templates page Includes: - changes to the govukCheckboxesField class to allow params to be extended at render time - updates to templates and folders CSS --- .../stylesheets/components/message.scss | 34 +++++--- app/main/forms.py | 40 +++++++++- .../views/templates/_template_list.html | 79 ++++++++++++------- 3 files changed, 108 insertions(+), 45 deletions(-) diff --git a/app/assets/stylesheets/components/message.scss b/app/assets/stylesheets/components/message.scss index 3d63ff455..14e02ebf7 100644 --- a/app/assets/stylesheets/components/message.scss +++ b/app/assets/stylesheets/components/message.scss @@ -24,6 +24,9 @@ } } +$message-text-left-spacing: 22px; +$message-type-bottom-spacing: govuk-spacing(4); + .message { &-name { @@ -63,23 +66,14 @@ } &-type { - color: $secondary-text-colour; - margin: 0 0 govuk-spacing(4) 0; + color: $govuk-secondary-text-colour; + margin: 0 0 $message-type-bottom-spacing 0; + padding-left: 0; pointer-events: none; } } -#template-list { - - margin-top: govuk-spacing(6); - - &.top-gutter-5px { - margin-top: 5px; - } - -} - .template-list { &-item { @@ -123,6 +117,22 @@ } + &-hint, + &-label { + padding-left: $message-text-left-spacing; + } + + &-label { + padding-top: 0px; + padding-bottom: 0px; + } + + // Fix for GOVUK Frontend selector with high precendence + // https://github.com/alphagov/govuk-frontend/blob/v2.13.0/src/components/hint/_hint.scss + &-label:not(.govuk-label--m):not(.govuk-label--l):not(.govuk-label--xl)+.template-list-item-hint { + margin-bottom: $message-type-bottom-spacing; + } + } &-folder { diff --git a/app/main/forms.py b/app/main/forms.py index e23b43cdd..6837be289 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -623,11 +623,34 @@ class govukCheckboxesField(govukCheckboxesMixin, SelectMultipleField): def get_items_from_options(self, field): return [self.get_item_from_option(option) for option in field] + def extend_params(self, params, extensions): + items = None + param_items = len(params['items']) if 'items' in params else 0 + + # split items off from params to make it a pure dict + if 'items' in extensions: + items = extensions['items'] + del extensions['items'] + + # merge dicts + params.update(extensions) + + # merge items + if items: + if 'items' not in params: + params['items'] = items + else: + for idx, _item in enumerate(items): + if idx >= param_items: + params['items'].append(items[idx]) + else: + params['items'][idx].update(items[idx]) + # self.__call__ renders the HTML for the field by: # 1. delegating to self.meta.render_field which # 2. calls field.widget # this bypasses that by making self.widget a method with the same interface as widget.__call__ - def widget(self, field, **kwargs): + def widget(self, field, param_extensions=None, **kwargs): # error messages error_message = None @@ -1898,9 +1921,18 @@ class TemplateAndFoldersSelectionForm(Form): return self.move_to_new_folder_name.data return None - templates_and_folders = MultiCheckboxField('Choose templates or folders', validators=[ - required_for_ops('move-to-new-folder', 'move-to-existing-folder') - ]) + templates_and_folders = govukCheckboxesField( + 'Choose templates or folders', + validators=[required_for_ops('move-to-new-folder', 'move-to-existing-folder')], + choices=[], # added to keep order of arguments, added properly in __init__ + param_extensions={ + "fieldset": { + "legend": { + "classes": "govuk-visually-hidden" + } + } + } + ) # 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 diff --git a/app/templates/views/templates/_template_list.html b/app/templates/views/templates/_template_list.html index 5bcb03501..42483d33a 100644 --- a/app/templates/views/templates/_template_list.html +++ b/app/templates/views/templates/_template_list.html @@ -21,37 +21,58 @@ {% endif %}

{% else %} -
+ `; - formGroup = document.querySelector('.form-group'); + wrapper = document.querySelector('.selection-wrapper'); + formGroup = wrapper.querySelector('.govuk-form-group'); fieldset = formGroup.querySelector('fieldset'); - checkboxesContainer = fieldset.querySelector('.checkboxes-nested'); + checkboxesContainer = fieldset.querySelector('.govuk-checkboxes'); checkboxes = checkboxesContainer.querySelectorAll('input[type=checkbox]'); }); @@ -141,6 +144,12 @@ describe('Collapsible fieldset', () => { }); + test("removes the hint", () => { + + expect(document.querySelector('.govuk-hint')).toBeNull(); + + }); + }); test('has the right summary text when started with no checkboxes selected', () => { @@ -187,7 +196,7 @@ describe('Collapsible fieldset', () => { test("the summary doesn't have a folder icon if fields aren't called 'folder'", () => { - formGroup.dataset.fieldLabel = 'team member'; + wrapper.dataset.fieldLabel = 'team member'; // start module window.GOVUK.modules.start(); @@ -277,35 +286,74 @@ describe('Collapsible fieldset', () => { describe("the footer (that wraps the button)", () => { - beforeEach(() => { + describe("is inserted", () => { - // track calls to sticky JS - window.GOVUK.stickAtBottomWhenScrolling.recalculate = jest.fn(() => {}); + test("after the fieldset", () => { - // start module - window.GOVUK.modules.start(); + // start module + window.GOVUK.modules.start(); - // show the checkboxes - helpers.triggerEvent(formGroup.querySelector('.govuk-button'), 'click'); + // show the checkboxes + helpers.triggerEvent(formGroup.querySelector('.govuk-button'), 'click'); + + expect(formGroup.querySelector('.selection-footer').previousElementSibling.nodeName).toBe('FIELDSET'); + + }); + + test("after the root fieldset if the checkboxes are nested", () => { + + // add a nested list of checkboxes to the first checkbox item + const nestedCheckboxes = document.createElement('div'); + nestedCheckboxes.className = 'govuk-form-group govuk-form-group--nested'; + nestedCheckboxes.innerHTML = _checkboxes(11, 20); + checkboxesContainer.querySelector('.govuk-checkboxes__item').appendChild(nestedCheckboxes); + + // start module + window.GOVUK.modules.start(); + + // show the checkboxes + helpers.triggerEvent(formGroup.querySelector('.govuk-button'), 'click'); + + expect(formGroup.querySelector('.selection-footer').previousElementSibling.nodeName).toBe('FIELDSET'); + + }); }); - test("is made sticky when the fieldset is expanded", () => { + describe("its stickiness", () => { - expect(formGroup.querySelector('.selection-footer').classList.contains('js-stick-at-bottom-when-scrolling')).toBe(true); - expect(window.GOVUK.stickAtBottomWhenScrolling.recalculate.mock.calls.length).toBe(1); + beforeEach(() => { + + // track calls to sticky JS + window.GOVUK.stickAtBottomWhenScrolling.recalculate = jest.fn(() => {}); + + // start module + window.GOVUK.modules.start(); + + // show the checkboxes + helpers.triggerEvent(formGroup.querySelector('.govuk-button'), 'click'); + + }); + + test("is added when the fieldset is expanded", () => { + + expect(formGroup.querySelector('.selection-footer').classList.contains('js-stick-at-bottom-when-scrolling')).toBe(true); + expect(window.GOVUK.stickAtBottomWhenScrolling.recalculate.mock.calls.length).toBe(1); + + }); + + test("is removed when the fieldset is collapsed", () => { + + // click the button to collapse the fieldset + helpers.triggerEvent(formGroup.querySelector('.govuk-button'), 'click'); + + expect(formGroup.querySelector('.selection-footer').classList.contains('js-stick-at-bottom-when-scrolling')).toBe(false); + expect(window.GOVUK.stickAtBottomWhenScrolling.recalculate.mock.calls.length).toBe(2); + + }); }); - test("has its stickiness removed when the fieldset is collapsed", () => { - - // click the button to collapse the fieldset - helpers.triggerEvent(formGroup.querySelector('.govuk-button'), 'click'); - - expect(formGroup.querySelector('.selection-footer').classList.contains('js-stick-at-bottom-when-scrolling')).toBe(false); - expect(window.GOVUK.stickAtBottomWhenScrolling.recalculate.mock.calls.length).toBe(2); - - }); }); describe("when the selection changes", () => { @@ -339,7 +387,7 @@ describe('Collapsible fieldset', () => { test("if fields are called 'folders'", () => { - formGroup.dataset.fieldLabel = 'folder'; + wrapper.dataset.fieldLabel = 'folder'; checkFirstCheckbox(); @@ -359,7 +407,7 @@ describe('Collapsible fieldset', () => { test("if fields are called 'team member'", () => { - formGroup.dataset.fieldLabel = 'team member'; + wrapper.dataset.fieldLabel = 'team member'; checkFirstCheckbox(); @@ -379,7 +427,7 @@ describe('Collapsible fieldset', () => { test("if fields are called 'arbitrary thing'", () => { - formGroup.dataset.fieldLabel = 'arbitrary thing'; + wrapper.dataset.fieldLabel = 'arbitrary thing'; checkFirstCheckbox(); @@ -403,7 +451,7 @@ describe('Collapsible fieldset', () => { test("if fields are called 'folder'", () => { - formGroup.dataset.fieldLabel = 'folder'; + wrapper.dataset.fieldLabel = 'folder'; checkAllCheckboxes(); @@ -423,7 +471,7 @@ describe('Collapsible fieldset', () => { test("if fields are called 'team member'", () => { - formGroup.dataset.fieldLabel = 'team member'; + wrapper.dataset.fieldLabel = 'team member'; checkAllCheckboxes(); @@ -447,7 +495,7 @@ describe('Collapsible fieldset', () => { test("if fields are called 'folder'", () => { - formGroup.dataset.fieldLabel = 'folder'; + wrapper.dataset.fieldLabel = 'folder'; checkAllCheckboxesButTheLast(); @@ -466,7 +514,7 @@ describe('Collapsible fieldset', () => { test("if fields are called 'team member'", () => { - formGroup.dataset.fieldLabel = 'team member'; + wrapper.dataset.fieldLabel = 'team member'; checkAllCheckboxesButTheLast();