From 810e880bb5135012351e83b06ccc040451f922ca Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 14 Nov 2019 16:22:06 +0000 Subject: [PATCH 1/7] Update all, but one, `
` to component Includes: - in gulpfile.js: - add details macro to list of those copied from GOVUK Frontend - remove existing details polyfill - convert all, but one,
tags to use GOVUK Frontend details component - add jinja boolean filter to help setting 'open' attribute of
tags Notes on the `
` not included in this: The `
` used for notifications items on the API integration page is not possible to reproduce with the GOV.UK Frontend macro so I'm splitting the resulting work out into it's own commit. --- app/__init__.py | 1 + .../stylesheets/govuk-frontend/_all.scss | 1 + app/templates/views/get-started.html | 32 ++++++++++--------- app/templates/views/platform-admin/index.html | 13 ++++++-- .../views/platform-admin/services.html | 26 +++++++++------ gulpfile.js | 4 +-- 6 files changed, 47 insertions(+), 30 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 4c24d5b28..59b918f81 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -743,6 +743,7 @@ def add_template_filters(application): format_phone_number_human_readable, format_thousands, id_safe, + convert_to_boolean, ]: application.add_template_filter(fn) diff --git a/app/assets/stylesheets/govuk-frontend/_all.scss b/app/assets/stylesheets/govuk-frontend/_all.scss index 760d837cc..9ecefd894 100644 --- a/app/assets/stylesheets/govuk-frontend/_all.scss +++ b/app/assets/stylesheets/govuk-frontend/_all.scss @@ -17,6 +17,7 @@ $govuk-assets-path: "/static/"; @import 'components/header/_header'; @import 'components/footer/_footer'; @import 'components/back-link/_back-link'; +@import 'components/details/_details'; @import "utilities/all"; @import "overrides/all"; diff --git a/app/templates/views/get-started.html b/app/templates/views/get-started.html index 09e165a8d..0d20b17b9 100644 --- a/app/templates/views/get-started.html +++ b/app/templates/views/get-started.html @@ -1,6 +1,7 @@ {% extends "content_template.html" %} {% from "components/table.html" import mapping_table, row, text_field, edit_field, field with context %} {% from "components/sub-navigation.html" import sub_navigation %} +{% from "components/details/macro.njk" import govukDetails %} {% block per_page_title %} Get started @@ -14,21 +15,22 @@

  • Check if GOV.UK Notify is right for you

    Read about our features, pricing and roadmap.

    -
    - Organisations that can use Notify -
    -

    Notify is available to:

    -
      -
    • central government departments
      • -
    • local authorities
      • -
    • state-funded schools
      • -
    • housing associations
      • -
    • the NHS
      • -
    • companies owned by local or central government that deliver services on their behalf
      • -
    -

    Notify is not currently available to charities.

    -
    -
    + {{ govukDetails({ + "summaryText": "Organisations that can use Notify", + "html": ''' +
    +

    Notify is available to:

    +
      +
    • central government departments
      • +
    • local authorities
      • +
    • state-funded schools
      • +
    • housing associations
      • +
    • the NHS
      • +
    • companies owned by local or central government that deliver services on their behalf
      • +
    +

    Notify is not currently available to charities.

    +
    ''' + }) }}
  • diff --git a/app/templates/views/platform-admin/index.html b/app/templates/views/platform-admin/index.html index 6da4e87d2..03fb8a021 100644 --- a/app/templates/views/platform-admin/index.html +++ b/app/templates/views/platform-admin/index.html @@ -4,6 +4,7 @@ {% from "components/message-count-label.html" import message_count_label %} {% from "components/status-box.html" import status_box %} {% from "components/form.html" import form_wrapper %} +{% from "components/details/macro.njk" import govukDetails %} {% block per_page_title %} Platform admin @@ -14,15 +15,21 @@

    Summary

    -
    - Apply filters + + {% set details_content %} {% call form_wrapper(method="get") %} {{ textbox(form.start_date, hint="Enter start date in format YYYY-MM-DD") }} {{ textbox(form.end_date, hint="Enter end date in format YYYY-MM-DD") }}
    {% endcall %} -
    + {% endset %} + + {{ govukDetails({ + "summaryText": "Help with nationality", + "html": details_content, + "open": form.errors | convert_to_boolean + }) }}
    {% for noti_type in global_stats %} diff --git a/app/templates/views/platform-admin/services.html b/app/templates/views/platform-admin/services.html index c10abb095..c42a76af7 100644 --- a/app/templates/views/platform-admin/services.html +++ b/app/templates/views/platform-admin/services.html @@ -6,6 +6,7 @@ {% from "components/message-count-label.html" import message_count_label %} {% from "components/table.html" import mapping_table, field, stats_fields, row_group, row, right_aligned_field_heading, hidden_field_heading, text_field %} {% from "components/form.html" import form_wrapper %} +{% from "components/details/macro.njk" import govukDetails %} {% macro stats_fields(channel, data) -%} @@ -101,16 +102,21 @@ {{ page_title|capitalize }} -
    - Apply filters - {% call form_wrapper(method="get") %} - {{ textbox(form.start_date, hint="Enter start date in format YYYY-MM-DD") }} - {{ textbox(form.end_date, hint="Enter end date in format YYYY-MM-DD") }} - {{ checkbox(form.include_from_test_key) }} -
    - - {% endcall %} -
    + + {% set details_content %} + {% call form_wrapper(method="get") %} + {{ textbox(form.start_date, hint="Enter start date in format YYYY-MM-DD") }} + {{ textbox(form.end_date, hint="Enter end date in format YYYY-MM-DD") }} + {{ checkbox(form.include_from_test_key) }} +
    + + {% endcall %} + {% endset %} + + {{ govukDetails({ + "summaryText": "Apply filters", + "html": details_content + }) }} {% include "views/platform-admin/_global_stats.html" %} diff --git a/gulpfile.js b/gulpfile.js index ff2903295..257aef277 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -63,7 +63,8 @@ const copy = { 'skip-link', 'header', 'footer', - 'back-link' + 'back-link', + 'details' ]; let done = 0; @@ -143,7 +144,6 @@ const javascripts = () => { paths.src + 'javascripts/govuk/cookie-functions.js', paths.src + 'javascripts/cookieMessage.js', paths.src + 'javascripts/stick-to-window-when-scrolling.js', - paths.src + 'javascripts/detailsPolyfill.js', paths.src + 'javascripts/apiKey.js', paths.src + 'javascripts/autofocus.js', paths.src + 'javascripts/enhancedTextbox.js', From d13db305c145e13df24407d51fecd3b9bfeaeec4 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Tue, 3 Dec 2019 09:47:48 +0000 Subject: [PATCH 2/7] Convert `
    ` on API page to macro HTML The GOV.UK Frontend details component macro wraps its `` text in a ``. We put a `

    ` in the `` (actually valid use, based on the spec) so this breaks when the `` wraps it. This converts the existing `
    ` tag to use all the class names the macro creates, but with all the `` contents in the `

    `. Also adds font-smoothing to the messages on the API page. This was previously set globally for all fonts in the GOV.UK Template CSS but is now just set for the New Transport 'nta' font. Included because the messages use the monospace font so don't have it by default. --- app/assets/stylesheets/views/api.scss | 73 ++++++++++++++++++--------- app/templates/views/api/index.html | 46 +++++++++-------- 2 files changed, 72 insertions(+), 47 deletions(-) diff --git a/app/assets/stylesheets/views/api.scss b/app/assets/stylesheets/views/api.scss index 98e21685e..4049c43c5 100644 --- a/app/assets/stylesheets/views/api.scss +++ b/app/assets/stylesheets/views/api.scss @@ -1,6 +1,8 @@ .api-notifications { font-family: monospace; + -webkit-font-smoothing: antialiased; + -moz-osx-font-smoothing: grayscale; border-bottom: 1px solid $border-colour; &-item { @@ -8,38 +10,59 @@ border-top: 1px solid $border-colour; padding: 10px 0 0 0; - &-title { - color: $link-colour; + &__heading, + &__data, + &__view { + font-family: monospace; + } + + &__heading { + display: block; + margin-bottom: $gutter-half; + + &::before { + top: -1.3em; + } + } + + &__meta { + + display: block; + color: $secondary-text-colour; text-decoration: none; - display: block; + + &-key, + &-time { + color: $secondary-text-colour; + display: inline-block; + width: auto; + } + + @include govuk-media-query($from: tablet) { + &-key, + &-time { + width: 50%; + } + + &-time { + text-align: right; + } + } + } - &-recipient { - display: inline; - } + &__data { - &-meta { - display: block; - color: $secondary-text-colour; - } + border-left: none; + padding-left: 25px; - &-time { - text-align: right; - } + &-name { + color: $secondary-text-colour; + } - &-key { - display: inline-block; - padding-left: 46px; - } - - &-data { - - padding-left: 31px; - color: $secondary-text-colour; - - &-item { - padding-bottom: 15px; + &-value { color: $text-colour; + padding-bottom: 15px; } } diff --git a/app/templates/views/api/index.html b/app/templates/views/api/index.html index 9397a97b5..cebb5442d 100644 --- a/app/templates/views/api/index.html +++ b/app/templates/views/api/index.html @@ -39,43 +39,45 @@
    {% if not api_notifications.notifications %}
    -

    +

    When you send messages via the API they’ll appear here.

    -

    +

    Notify deletes messages after 7 days.

    {% endif %} {% for notification in api_notifications.notifications %} -
    - -

    +
    + +

    + {{ notification.to }} + + + + {{notification.key_name}} + + + + +

    - - - {{notification.key_name}} - - - - -     -
    -
    +
    +
    {% for key in [ 'id', 'client_reference', 'notification_type', 'created_at', 'updated_at', 'sent_at', 'status' ] %} {% if notification[key] %} -
    {{ key }}:
    -
    {{ notification[key] }}
    +
    {{ key }}:
    +
    {{ notification[key] }}
    {% endif %} {% endfor %} {% if notification.status not in ('pending-virus-check', 'virus-scan-failed') %} - View {{ message_count_label(1, notification.template.template_type, suffix='') }} + View {{ message_count_label(1, notification.template.template_type, suffix='') }} {% endif %}
    @@ -84,11 +86,11 @@ {% if api_notifications.notifications %}
    {% if api_notifications.notifications|length == 50 %} -

    +

    Only showing the first 50 messages.

    {% endif %} -

    +

    Notify deletes messages after 7 days.

    From 146d5cc07a1116a5915efee62c32d2c396aa951e Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 29 Nov 2019 17:40:45 +0000 Subject: [PATCH 3/7] Fix test broken by HTML changes on API page I moved the meta for each message into the `

    ` in each `` to make the heading unique. This broke the test. --- tests/app/main/views/test_api_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 326f68d1e..5ba9a68c1 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -37,7 +37,7 @@ def test_should_show_api_page( rows = page.find_all('details') assert len(rows) == 5 for row in rows: - assert row.find('h3').string.strip() == '07123456789' + assert row.select('h3 .govuk-details__summary-text')[0].string.strip() == '07123456789' def test_should_show_api_page_with_lots_of_notifications( From 5dce31fc5500af4109cc559b34a1b29f8065d368 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 13 Dec 2019 09:41:34 +0000 Subject: [PATCH 4/7] Correct platform admin details summary text --- app/templates/views/platform-admin/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/platform-admin/index.html b/app/templates/views/platform-admin/index.html index 03fb8a021..c404f78c3 100644 --- a/app/templates/views/platform-admin/index.html +++ b/app/templates/views/platform-admin/index.html @@ -26,7 +26,7 @@ {% endset %} {{ govukDetails({ - "summaryText": "Help with nationality", + "summaryText": "Apply filters", "html": details_content, "open": form.errors | convert_to_boolean }) }} From 84f90a7e1c4e9823ac4655dcd230cec63e74cf03 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 13 Dec 2019 09:50:04 +0000 Subject: [PATCH 5/7] Remove redundant details polyfill We're now using the GOV.UK Frontend one. --- app/assets/javascripts/detailsPolyfill.js | 196 ---------------------- 1 file changed, 196 deletions(-) delete mode 100644 app/assets/javascripts/detailsPolyfill.js diff --git a/app/assets/javascripts/detailsPolyfill.js b/app/assets/javascripts/detailsPolyfill.js deleted file mode 100644 index ce0692195..000000000 --- a/app/assets/javascripts/detailsPolyfill.js +++ /dev/null @@ -1,196 +0,0 @@ -// From -// https://github.com/alphagov/govuk_elements/blob/4926897dc7734db2fc5e5ebb6acdc97f86e22e50/public/javascripts/vendor/details.polyfill.js -// -// --- -// -//
    polyfill -// http://caniuse.com/#feat=details - -// FF Support for HTML5's
    and -// https://bugzilla.mozilla.org/show_bug.cgi?id=591737 - -// http://www.sitepoint.com/fixing-the-details-element/ - -(function () { - 'use strict'; - - var NATIVE_DETAILS = typeof document.createElement('details').open === 'boolean'; - - // Add event construct for modern browsers or IE - // which fires the callback with a pre-converted target reference - function addEvent(node, type, callback) { - if (node.addEventListener) { - node.addEventListener(type, function (e) { - callback(e, e.target); - }, false); - } else if (node.attachEvent) { - node.attachEvent('on' + type, function (e) { - callback(e, e.srcElement); - }); - } - } - - // Handle cross-modal click events - function addClickEvent(node, callback) { - // Prevent space(32) from scrolling the page - addEvent(node, 'keypress', function (e, target) { - if (target.nodeName === 'SUMMARY') { - if (e.keyCode === 32) { - if (e.preventDefault) { - e.preventDefault(); - } else { - e.returnValue = false; - } - } - } - }); - // When the key comes up - check if it is enter(13) or space(32) - addEvent(node, 'keyup', function (e, target) { - if (e.keyCode === 13 || e.keyCode === 32) { callback(e, target); } - }); - addEvent(node, 'mouseup', function (e, target) { - callback(e, target); - }); - } - - // Get the nearest ancestor element of a node that matches a given tag name - function getAncestor(node, match) { - do { - if (!node || node.nodeName.toLowerCase() === match) { - break; - } - } while ((node = node.parentNode)); - - return node; - } - - // Create a started flag so we can prevent the initialisation - // function firing from both DOMContentLoaded and window.onload - var started = false; - - // Initialisation function - function addDetailsPolyfill(list) { - - // If this has already happened, just return - // else set the flag so it doesn't happen again - if (started) { - return; - } - started = true; - - // Get the collection of details elements, but if that's empty - // then we don't need to bother with the rest of the scripting - if ((list = document.getElementsByTagName('details')).length === 0) { - return; - } - - // else iterate through them to apply their initial state - var n = list.length, i = 0; - for (i; i < n; i++) { - var details = list[i]; - - // Save shortcuts to the inner summary and content elements - details.__summary = details.getElementsByTagName('summary').item(0); - details.__content = details.getElementsByTagName('div').item(0); - - // If the content doesn't have an ID, assign it one now - // which we'll need for the summary's aria-controls assignment - if (!details.__content.id) { - details.__content.id = 'details-content-' + i; - } - - // Add ARIA role="group" to details - details.setAttribute('role', 'group'); - - // Add role=button to summary - details.__summary.setAttribute('role', 'button'); - - // Add aria-controls - details.__summary.setAttribute('aria-controls', details.__content.id); - - // Set tabIndex so the summary is keyboard accessible for non-native elements - // http://www.saliences.com/browserBugs/tabIndex.html - if (!NATIVE_DETAILS) { - details.__summary.tabIndex = 0; - } - - // Detect initial open state - var openAttr = details.getAttribute('open') !== null; - if (openAttr === true) { - details.__summary.setAttribute('aria-expanded', 'true'); - details.__content.setAttribute('aria-hidden', 'false'); - } else { - details.__summary.setAttribute('aria-expanded', 'false'); - details.__content.setAttribute('aria-hidden', 'true'); - if (!NATIVE_DETAILS) { - details.__content.style.display = 'none'; - } - } - - // Create a circular reference from the summary back to its - // parent details element, for convenience in the click handler - details.__summary.__details = details; - - // If this is not a native implementation, create an arrow - // inside the summary - - var twisty = document.createElement('i'); - - if (openAttr === true) { - twisty.className = 'arrow arrow-open'; - twisty.appendChild(document.createTextNode('\u25bc')); - } else { - twisty.className = 'arrow arrow-closed'; - twisty.appendChild(document.createTextNode('\u25ba')); - } - - details.__summary.__twisty = details.__summary.insertBefore(twisty, details.__summary.firstChild); - details.__summary.__twisty.setAttribute('aria-hidden', 'true'); - - } - - // Define a statechange function that updates aria-expanded and style.display - // Also update the arrow position - function statechange(summary) { - - var expanded = summary.__details.__summary.getAttribute('aria-expanded') === 'true'; - var hidden = summary.__details.__content.getAttribute('aria-hidden') === 'true'; - - summary.__details.__summary.setAttribute('aria-expanded', (expanded ? 'false' : 'true')); - summary.__details.__content.setAttribute('aria-hidden', (hidden ? 'false' : 'true')); - - if (!NATIVE_DETAILS) { - summary.__details.__content.style.display = (expanded ? 'none' : ''); - - var hasOpenAttr = summary.__details.getAttribute('open') !== null; - if (!hasOpenAttr) { - summary.__details.setAttribute('open', 'open'); - } else { - summary.__details.removeAttribute('open'); - } - } - - if (summary.__twisty) { - summary.__twisty.firstChild.nodeValue = (expanded ? '\u25ba' : '\u25bc'); - summary.__twisty.setAttribute('class', (expanded ? 'arrow arrow-closed' : 'arrow arrow-open')); - } - - return true; - } - - // Bind a click event to handle summary elements - addClickEvent(document, function(e, summary) { - if (!(summary = getAncestor(summary, 'summary'))) { - return true; - } - return statechange(summary); - }); - } - - // Bind two load events for modern and older browsers - // If the first one fires it will set a flag to block the second one - // but if it's not supported then the second one will fire - addEvent(document, 'DOMContentLoaded', addDetailsPolyfill); - addEvent(window, 'load', addDetailsPolyfill); - -})(); From afd3fe57813eb91d06cb86e7df2f7f207b0895de Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Mon, 16 Dec 2019 17:03:42 +0000 Subject: [PATCH 6/7] Include details JS --- app/assets/javascripts/modules/all.mjs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/app/assets/javascripts/modules/all.mjs b/app/assets/javascripts/modules/all.mjs index 7940c17df..7041044d3 100644 --- a/app/assets/javascripts/modules/all.mjs +++ b/app/assets/javascripts/modules/all.mjs @@ -7,6 +7,21 @@ // Exported items will be added to the window.GOVUK namespace. // For example, `export { Frontend }` will assign `Frontend` to `window.Frontend` import Header from 'govuk-frontend/components/header/header'; +import Details from 'govuk-frontend/components/details/details'; + +/** + * TODO: Ideally this would be a NodeList.prototype.forEach polyfill + * This seems to fail in IE8, requires more investigation. + * See: https://github.com/imagitama/nodelist-foreach-polyfill + */ +function nodeListForEach (nodes, callback) { + if (window.NodeList.prototype.forEach) { + return nodes.forEach(callback) + } + for (var i = 0; i < nodes.length; i++) { + callback.call(window, nodes[i], i, nodes); + } +} // Copy of the initAll function from https://github.com/alphagov/govuk-frontend/blob/v2.13.0/src/all.js // except it only includes, and initialises, the components used by this application. @@ -18,6 +33,12 @@ function initAll (options) { // Defaults to the entire document if nothing is set. var scope = typeof options.scope !== 'undefined' ? options.scope : document + // Find all global details elements to enhance. + var $details = scope.querySelectorAll('details') + nodeListForEach($details, function ($detail) { + new Details($detail).init() + }) + // Find first header module to enhance. var $toggleButton = scope.querySelector('[data-module="header"]') new Header($toggleButton).init() @@ -26,6 +47,7 @@ function initAll (options) { // Create separate namespace for GOVUK Frontend. var Frontend = { "Header": Header, + "Details": Details, "initAll": initAll } From d55b3caa463b05ed401243ff8984d087709c1dbd Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Tue, 17 Dec 2019 11:24:57 +0000 Subject: [PATCH 7/7] Disable tel number detection on IOS Any string pattern identifiable as a telephone number is turned into a link on IOS devices by default. We use telephone numbers in several ways, in particular to link to sms notifications, so need this behaviour turned off unless specifically required. --- app/templates/admin_template.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 64d78f397..1469ebd7c 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -18,6 +18,9 @@ .govuk-header__container { border-color: {{header_colour}} } + {% block meta_format_detection %} + + {% endblock %} {% block meta %} {% endblock %} {% endblock %}