From 39ac6e4582a65bae6789c649ce25ca04d605024f Mon Sep 17 00:00:00 2001 From: Brett Kyle Date: Wed, 7 Jun 2023 16:31:57 +0100 Subject: [PATCH 1/7] Remove JavaScript from Details component --- packages/govuk-frontend/src/govuk/all.mjs | 7 - .../src/govuk/components/details/details.mjs | 162 ------------------ .../govuk/components/details/details.test.js | 162 ------------------ .../src/govuk/components/globals.test.mjs | 1 - 4 files changed, 332 deletions(-) delete mode 100644 packages/govuk-frontend/src/govuk/components/details/details.mjs delete mode 100644 packages/govuk-frontend/src/govuk/components/details/details.test.js diff --git a/packages/govuk-frontend/src/govuk/all.mjs b/packages/govuk-frontend/src/govuk/all.mjs index 42d15e33a5..16d2517599 100644 --- a/packages/govuk-frontend/src/govuk/all.mjs +++ b/packages/govuk-frontend/src/govuk/all.mjs @@ -3,7 +3,6 @@ import { Accordion } from './components/accordion/accordion.mjs' import { Button } from './components/button/button.mjs' import { CharacterCount } from './components/character-count/character-count.mjs' import { Checkboxes } from './components/checkboxes/checkboxes.mjs' -import { Details } from './components/details/details.mjs' import { ErrorSummary } from './components/error-summary/error-summary.mjs' import { Header } from './components/header/header.mjs' import { NotificationBanner } from './components/notification-banner/notification-banner.mjs' @@ -51,11 +50,6 @@ function initAll (config) { new Checkboxes($checkbox).init() }) - const $details = $scope.querySelectorAll('[data-module="govuk-details"]') - $details.forEach(($detail) => { - new Details($detail).init() - }) - // Find first error summary module to enhance. const $errorSummary = $scope.querySelector('[data-module="govuk-error-summary"]') if ($errorSummary) { @@ -97,7 +91,6 @@ export { // Components Accordion, Button, - Details, CharacterCount, Checkboxes, ErrorSummary, diff --git a/packages/govuk-frontend/src/govuk/components/details/details.mjs b/packages/govuk-frontend/src/govuk/components/details/details.mjs deleted file mode 100644 index c65ca32844..0000000000 --- a/packages/govuk-frontend/src/govuk/components/details/details.mjs +++ /dev/null @@ -1,162 +0,0 @@ -/** - * JavaScript 'polyfill' for HTML5's
and elements - * and 'shim' to add accessiblity enhancements for all browsers - * - * http://caniuse.com/#feat=details - */ -import { generateUniqueID } from '../../common/index.mjs' - -const KEY_ENTER = 13 -const KEY_SPACE = 32 - -/** - * Details component - */ -export class Details { - /** - * - * @param {Element} $module - HTML element to use for details - */ - constructor ($module) { - if (!($module instanceof HTMLElement) || !document.body.classList.contains('govuk-frontend-supported')) { - return this - } - - /** @private */ - this.$module = $module - - /** @private */ - this.$summary = null - - /** @private */ - this.$content = null - } - - /** - * Initialise component - */ - init () { - // Check that required elements are present - if (!this.$module) { - return - } - - // If there is native details support, we want to avoid running code to polyfill native behaviour. - const hasNativeDetails = 'HTMLDetailsElement' in window && - this.$module instanceof HTMLDetailsElement - - if (!hasNativeDetails) { - this.polyfillDetails() - } - } - - /** - * Polyfill component in older browsers - * - * @private - */ - polyfillDetails () { - const $module = this.$module - - // Save shortcuts to the inner summary and content elements - const $summary = this.$summary = $module.getElementsByTagName('summary').item(0) - const $content = this.$content = $module.getElementsByTagName('div').item(0) - - // If
doesn't have a and a
representing the content - // it means the required HTML structure is not met so the script will stop - if (!$summary || !$content) { - return - } - - // If the content doesn't have an ID, assign it one now - // which we'll need for the summary's aria-controls assignment - if (!$content.id) { - $content.id = `details-content-${generateUniqueID()}` - } - - // Add ARIA role="group" to details - $module.setAttribute('role', 'group') - - // Add role=button to summary - $summary.setAttribute('role', 'button') - - // Add aria-controls - $summary.setAttribute('aria-controls', $content.id) - - // Set tabIndex so the summary is keyboard accessible for non-native elements - // - // We have to use the camelcase `tabIndex` property as there is a bug in IE6/IE7 when we set the correct attribute lowercase: - // See http://web.archive.org/web/20170120194036/http://www.saliences.com/browserBugs/tabIndex.html for more information. - $summary.tabIndex = 0 - - // Detect initial open state - if (this.$module.hasAttribute('open')) { - $summary.setAttribute('aria-expanded', 'true') - } else { - $summary.setAttribute('aria-expanded', 'false') - $content.style.display = 'none' - } - - // Bind an event to handle summary elements - this.polyfillHandleInputs(() => this.polyfillSetAttributes()) - } - - /** - * Define a statechange function that updates aria-expanded and style.display - * - * @private - * @returns {boolean} Returns true - */ - polyfillSetAttributes () { - if (this.$module.hasAttribute('open')) { - this.$module.removeAttribute('open') - this.$summary.setAttribute('aria-expanded', 'false') - this.$content.style.display = 'none' - } else { - this.$module.setAttribute('open', 'open') - this.$summary.setAttribute('aria-expanded', 'true') - this.$content.style.display = '' - } - - return true - } - - /** - * Handle cross-modal click events - * - * @private - * @param {(event: UIEvent) => void} callback - function - */ - polyfillHandleInputs (callback) { - this.$summary.addEventListener('keypress', (event) => { - const $target = event.target - // When the key gets pressed - check if it is enter or space - if (event.keyCode === KEY_ENTER || event.keyCode === KEY_SPACE) { - if ($target instanceof HTMLElement && $target.nodeName.toLowerCase() === 'summary') { - // Prevent space from scrolling the page - // and enter from submitting a form - event.preventDefault() - // Click to let the click event do all the necessary action - if ($target.click) { - $target.click() - } else { - // except Safari 5.1 and under don't support .click() here - callback(event) - } - } - } - }) - - // Prevent keyup to prevent clicking twice in Firefox when using space key - this.$summary.addEventListener('keyup', (event) => { - const $target = event.target - if (event.keyCode === KEY_SPACE) { - if ($target instanceof HTMLElement && $target.nodeName.toLowerCase() === 'summary') { - event.preventDefault() - } - } - }) - - this.$summary.addEventListener('click', callback) - } -} diff --git a/packages/govuk-frontend/src/govuk/components/details/details.test.js b/packages/govuk-frontend/src/govuk/components/details/details.test.js deleted file mode 100644 index 2087f19840..0000000000 --- a/packages/govuk-frontend/src/govuk/components/details/details.test.js +++ /dev/null @@ -1,162 +0,0 @@ -const { goToComponent, goToExample } = require('govuk-frontend-helpers/puppeteer') - -describe('details', () => { - it('should not polyfill when details element is available', async () => { - await goToComponent(page, 'details') - - const summaryAriaExpanded = await page.evaluate(() => { - return document.querySelector('summary').getAttribute('aria-expanded') - }) - expect(summaryAriaExpanded).toBe(null) - }) - - describe('/examples/details-polyfill', () => { - beforeAll(async () => { - await goToExample(page, 'details-polyfill') - }) - - it('should add to summary the button role', async () => { - const summaryRole = await page.evaluate(() => { - return document.getElementById('default').querySelector('summary').getAttribute('role') - }) - expect(summaryRole).toBe('button') - }) - - it('should set the element controlled by the summary using aria-controls', async () => { - const summaryAriaControls = await page.evaluate(() => { - return document.getElementById('default').querySelector('summary').getAttribute('aria-controls') - }) - const controlledContainerId = await page.evaluate(() => { - return document.getElementById('default').querySelector('.govuk-details__text').getAttribute('id') - }) - expect(summaryAriaControls).toBe(controlledContainerId) - }) - - it('should set the expanded state of the summary to false using aria-expanded', async () => { - const summaryAriaExpanded = await page.evaluate(() => { - return document.getElementById('default').querySelector('summary').getAttribute('aria-expanded') - }) - expect(summaryAriaExpanded).toBe('false') - }) - - it('should hide the content using display: none', async () => { - const containerDisplay = await page.evaluate(() => { - const container = document.getElementById('default').querySelector('.govuk-details__text') - return window.getComputedStyle(container).getPropertyValue('display') - }) - - expect(containerDisplay).toBe('none') - }) - - it('should indicate the open state of the content', async () => { - const detailsOpen = await page.evaluate(() => { - return document.getElementById('default').getAttribute('open') - }) - expect(detailsOpen).toBeNull() - }) - - describe('when details is triggered', () => { - beforeEach(async () => { - await goToExample(page, 'details-polyfill') - }) - - it('should indicate the expanded state of the summary using aria-expanded', async () => { - await page.click('#default summary') - - const summaryAriaExpanded = await page.evaluate(() => { - return document.getElementById('default').querySelector('summary').getAttribute('aria-expanded') - }) - expect(summaryAriaExpanded).toBe('true') - }) - - it('should make the content visible', async () => { - await page.click('#default summary') - - const containerDisplay = await page.evaluate(() => { - const container = document.getElementById('default').querySelector('.govuk-details__text') - return window.getComputedStyle(container).getPropertyValue('display') - }) - - expect(containerDisplay).toBe('block') - }) - - it('should indicate the open state of the content', async () => { - await page.click('#default summary') - - const detailsOpen = await page.evaluate(() => { - return document.getElementById('default').getAttribute('open') - }) - expect(detailsOpen).not.toBeNull() - }) - }) - }) - - describe('expanded', () => { - beforeEach(async () => { - await goToExample(page, 'details-polyfill') - }) - - it('should indicate the expanded state of the summary using aria-expanded', async () => { - const summaryAriaExpanded = await page.evaluate(() => { - return document.getElementById('expanded').querySelector('summary').getAttribute('aria-expanded') - }) - expect(summaryAriaExpanded).toBe('true') - }) - - it('should keep the content visible', async () => { - const containerDisplay = await page.evaluate(() => { - const container = document.getElementById('expanded').querySelector('.govuk-details__text') - return window.getComputedStyle(container).getPropertyValue('display') - }) - - expect(containerDisplay).toBe('block') - }) - - it('should indicate the open state of the content', async () => { - const detailsOpen = await page.evaluate(() => { - return document.getElementById('expanded').getAttribute('open') - }) - expect(detailsOpen).not.toBeNull() - }) - - it('should not be affected when clicking the revealed content', async () => { - await page.click('#expanded .govuk-details__text') - - const summaryAriaExpanded = await page.evaluate(() => { - return document.getElementById('expanded').querySelector('summary').getAttribute('aria-expanded') - }) - expect(summaryAriaExpanded).toBe('true') - }) - - describe('when details is triggered', () => { - it('should indicate the expanded state of the summary using aria-expanded', async () => { - await page.click('#expanded summary') - - const summaryAriaExpanded = await page.evaluate(() => { - return document.getElementById('expanded').querySelector('summary').getAttribute('aria-expanded') - }) - expect(summaryAriaExpanded).toBe('false') - }) - - it('should hide the content using display: none', async () => { - await page.click('#expanded summary') - - const containerDisplay = await page.evaluate(() => { - const container = document.getElementById('expanded').querySelector('.govuk-details__text') - return window.getComputedStyle(container).getPropertyValue('display') - }) - - expect(containerDisplay).toBe('none') - }) - - it('should indicate the open state of the content', async () => { - await page.click('#expanded summary') - - const detailsOpen = await page.evaluate(() => { - return document.getElementById('expanded').getAttribute('open') - }) - expect(detailsOpen).toBeNull() - }) - }) - }) -}) diff --git a/packages/govuk-frontend/src/govuk/components/globals.test.mjs b/packages/govuk-frontend/src/govuk/components/globals.test.mjs index 81c36a860b..cc5b328865 100644 --- a/packages/govuk-frontend/src/govuk/components/globals.test.mjs +++ b/packages/govuk-frontend/src/govuk/components/globals.test.mjs @@ -39,7 +39,6 @@ describe('GOV.UK Frontend', () => { 'Button', 'CharacterCount', 'Checkboxes', - 'Details', 'ErrorSummary', 'Header', 'NotificationBanner', From 93f7c8a984ff7a8ec6c596a33d2536393b6641d9 Mon Sep 17 00:00:00 2001 From: Brett Kyle Date: Wed, 7 Jun 2023 16:33:49 +0100 Subject: [PATCH 2/7] Remove data-module from Details template --- .../govuk-frontend/src/govuk/components/details/template.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/govuk-frontend/src/govuk/components/details/template.njk b/packages/govuk-frontend/src/govuk/components/details/template.njk index 809e49d153..15a53107ba 100644 --- a/packages/govuk-frontend/src/govuk/components/details/template.njk +++ b/packages/govuk-frontend/src/govuk/components/details/template.njk @@ -1,4 +1,4 @@ -
+
{{ params.summaryHtml | safe if params.summaryHtml else params.summaryText }} From bc276496f36d2fec27ed08f1e51d13712f77198a Mon Sep 17 00:00:00 2001 From: Brett Kyle Date: Wed, 7 Jun 2023 16:37:50 +0100 Subject: [PATCH 3/7] Remove generateUniqueID helper --- .../govuk-frontend/src/govuk/common/index.mjs | 20 ------------------- .../src/govuk/common/index.unit.test.mjs | 2 -- 2 files changed, 22 deletions(-) diff --git a/packages/govuk-frontend/src/govuk/common/index.mjs b/packages/govuk-frontend/src/govuk/common/index.mjs index 22b175cc9a..67c39bdeb2 100644 --- a/packages/govuk-frontend/src/govuk/common/index.mjs +++ b/packages/govuk-frontend/src/govuk/common/index.mjs @@ -8,26 +8,6 @@ * @module common/index */ -/** - * Used to generate a unique string, allows multiple instances of the component - * without them conflicting with each other. - * https://stackoverflow.com/a/8809472 - * - * @private - * @returns {string} Unique ID - */ -export function generateUniqueID () { - let d = new Date().getTime() - if (typeof window.performance !== 'undefined' && typeof window.performance.now === 'function') { - d += window.performance.now() // use high-precision timer if available - } - return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) { - const r = (d + Math.random() * 16) % 16 | 0 - d = Math.floor(d / 16) - return (c === 'x' ? r : (r & 0x3 | 0x8)).toString(16) - }) -} - /** * Config flattening function * diff --git a/packages/govuk-frontend/src/govuk/common/index.unit.test.mjs b/packages/govuk-frontend/src/govuk/common/index.unit.test.mjs index 2395929db5..70c9c4f838 100644 --- a/packages/govuk-frontend/src/govuk/common/index.unit.test.mjs +++ b/packages/govuk-frontend/src/govuk/common/index.unit.test.mjs @@ -1,7 +1,5 @@ import { mergeConfigs, extractConfigByNamespace } from './index.mjs' -// TODO: Write unit tests for `generateUniqueID` - describe('Common JS utilities', () => { describe('mergeConfigs', () => { const config1 = { From 7413d930ec89b9a52412a5f965666a0add412940 Mon Sep 17 00:00:00 2001 From: Brett Kyle Date: Tue, 6 Jun 2023 10:58:47 +0100 Subject: [PATCH 4/7] Wrap everything in support query EdgeHTML is the only browser engine that supports `-ms-ime-align`, so we can check Edge 12 - 18 by applying this property. https://browserstrangeness.github.io/css_hacks.html We can target Edge 16 - 18 more specifically, but keeping it simple for now. This approach just wraps almost everything in a check to see the browser DOESN'T support `-ms-ime-align`, but separates some spacing and border styles to make things look tolerable. --- .../src/govuk/components/details/_index.scss | 119 ++++++++++-------- 1 file changed, 64 insertions(+), 55 deletions(-) diff --git a/packages/govuk-frontend/src/govuk/components/details/_index.scss b/packages/govuk-frontend/src/govuk/components/details/_index.scss index 739c856dcb..9d2efcfd46 100644 --- a/packages/govuk-frontend/src/govuk/components/details/_index.scss +++ b/packages/govuk-frontend/src/govuk/components/details/_index.scss @@ -11,78 +11,87 @@ // Make the focus outline shrink-wrap the text content of the summary display: inline-block; - // Absolutely position the marker against this element - position: relative; - margin-bottom: govuk-spacing(1); - - // Allow for absolutely positioned marker and align with disclosed text - padding-left: govuk-spacing(4) + $govuk-border-width; - - // Style the summary to look like a link... - color: $govuk-link-colour; - cursor: pointer; - - &:hover { - color: $govuk-link-hover-colour; - } - - &:focus { - @include govuk-focused-text; - } } - // ...but only underline the text, not the arrow - .govuk-details__summary-text { - @include govuk-link-decoration; + .govuk-details__text { + padding-top: govuk-spacing(3); + padding-bottom: govuk-spacing(3); + padding-left: govuk-spacing(4); + border-left: $govuk-border-width solid $govuk-border-colour; } - .govuk-details__summary:hover .govuk-details__summary-text { - @include govuk-link-hover-decoration; + .govuk-details__text p { + margin-top: 0; + margin-bottom: govuk-spacing(4); } - // Remove the underline when focussed to avoid duplicate borders - .govuk-details__summary:focus .govuk-details__summary-text { - text-decoration: none; + .govuk-details__text > :last-child { + margin-bottom: 0; } - // Remove the default details marker so we can style our own consistently and - // ensure it displays in Firefox (see implementation.md for details) - .govuk-details__summary::-webkit-details-marker { - display: none; - } + // Fix for older browsers which: + // - support ES6 modules but not the
element (Edge 16 - 18) + // - do not support ES6 modules or the
element (eg, IE8+) + // -ms-ime-align is only supported by Edge 12 - 18 + // Older browsers ignore the entire feature query and use the styles above + @supports not (-ms-ime-align: auto) { + .govuk-details__summary { - // Append our own open / closed marker using a pseudo-element - .govuk-details__summary::before { - content: ""; - position: absolute; + // Absolutely position the marker against this element + position: relative; - top: -1px; - bottom: 0; - left: 0; + // Allow for absolutely positioned marker and align with disclosed text + padding-left: govuk-spacing(4) + $govuk-border-width; - margin: auto; + // Style the summary to look like a link... + color: $govuk-link-colour; + cursor: pointer; - @include govuk-shape-arrow($direction: right, $base: 14px); + &:hover { + color: $govuk-link-hover-colour; + } - .govuk-details[open] > & { - @include govuk-shape-arrow($direction: down, $base: 14px); + &:focus { + @include govuk-focused-text; + } + } + // ...but only underline the text, not the arrow + .govuk-details__summary-text { + @include govuk-link-decoration; } - } - .govuk-details__text { - padding-top: govuk-spacing(3); - padding-bottom: govuk-spacing(3); - padding-left: govuk-spacing(4); - border-left: $govuk-border-width solid $govuk-border-colour; - } + .govuk-details__summary:hover .govuk-details__summary-text { + @include govuk-link-hover-decoration; + } - .govuk-details__text p { - margin-top: 0; - margin-bottom: govuk-spacing(4); - } + // Remove the underline when focussed to avoid duplicate borders + .govuk-details__summary:focus .govuk-details__summary-text { + text-decoration: none; + } - .govuk-details__text > :last-child { - margin-bottom: 0; + // Remove the default details marker so we can style our own consistently and + // ensure it displays in Firefox (see implementation.md for details) + .govuk-details__summary::-webkit-details-marker { + display: none; + } + + // Append our own open / closed marker using a pseudo-element + .govuk-details__summary::before { + content: ""; + position: absolute; + + top: -1px; + bottom: 0; + left: 0; + + margin: auto; + + @include govuk-shape-arrow($direction: right, $base: 14px); + + .govuk-details[open] > & { + @include govuk-shape-arrow($direction: down, $base: 14px); + } + } } } From 4d6a8b05c9ab6a4a04731adca15847397fb116c4 Mon Sep 17 00:00:00 2001 From: Brett Kyle Date: Mon, 12 Jun 2023 09:20:27 +0100 Subject: [PATCH 5/7] Style to look like inset text on older browsers --- .../src/govuk/components/details/_index.scss | 47 +++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/govuk-frontend/src/govuk/components/details/_index.scss b/packages/govuk-frontend/src/govuk/components/details/_index.scss index 9d2efcfd46..9d4ddcc416 100644 --- a/packages/govuk-frontend/src/govuk/components/details/_index.scss +++ b/packages/govuk-frontend/src/govuk/components/details/_index.scss @@ -5,20 +5,38 @@ @include govuk-responsive-margin(6, "bottom"); display: block; + + border-left: $govuk-border-width-wide solid $govuk-border-colour; } .govuk-details__summary { // Make the focus outline shrink-wrap the text content of the summary display: inline-block; + margin-top: govuk-spacing(3); + margin-bottom: govuk-spacing(1); } + .govuk-details__summary-text { + @include govuk-typography-weight-bold; + @include govuk-responsive-margin(4, "bottom"); + padding-left: govuk-spacing(4); + + > :first-child { + margin-top: 0; + } + + > :only-child, + > :last-child { + margin-bottom: 0; + } + } + .govuk-details__text { padding-top: govuk-spacing(3); padding-bottom: govuk-spacing(3); padding-left: govuk-spacing(4); - border-left: $govuk-border-width solid $govuk-border-colour; } .govuk-details__text p { @@ -30,17 +48,28 @@ margin-bottom: 0; } - // Fix for older browsers which: + // We wrap styles for newer browsers in a feature query, which is ignored by + // older browsers, which always expand the details element. + // + // Additionally, -ms-ime-align is only supported by Edge 12 - 18 + // + // This ensures we don't use these styles in browsers which: // - support ES6 modules but not the
element (Edge 16 - 18) // - do not support ES6 modules or the
element (eg, IE8+) - // -ms-ime-align is only supported by Edge 12 - 18 - // Older browsers ignore the entire feature query and use the styles above @supports not (-ms-ime-align: auto) { + // Override default border + .govuk-details { + border-left: none; + } + .govuk-details__summary { // Absolutely position the marker against this element position: relative; + // Override default top margin + margin-top: 0; + // Allow for absolutely positioned marker and align with disclosed text padding-left: govuk-spacing(4) + $govuk-border-width; @@ -59,6 +88,12 @@ // ...but only underline the text, not the arrow .govuk-details__summary-text { @include govuk-link-decoration; + // Override default font weight + @include govuk-typography-weight-regular; + // Override default margin + margin-bottom: 0; + // Override default padding + padding-left: 0; } .govuk-details__summary:hover .govuk-details__summary-text { @@ -93,5 +128,9 @@ @include govuk-shape-arrow($direction: down, $base: 14px); } } + + .govuk-details__text { + border-left: $govuk-border-width solid $govuk-border-colour; + } } } From 0d35415fee89c97a17ab31acafaae93ddc76a3b5 Mon Sep 17 00:00:00 2001 From: Brett Kyle Date: Tue, 20 Jun 2023 11:10:05 +0100 Subject: [PATCH 6/7] Default to native styling --- .../src/govuk/components/details/_index.scss | 41 ++++++++--------- .../components/details/implementation.md | 44 +++++++++++++++++++ 2 files changed, 63 insertions(+), 22 deletions(-) diff --git a/packages/govuk-frontend/src/govuk/components/details/_index.scss b/packages/govuk-frontend/src/govuk/components/details/_index.scss index 9d4ddcc416..8f7e43eec4 100644 --- a/packages/govuk-frontend/src/govuk/components/details/_index.scss +++ b/packages/govuk-frontend/src/govuk/components/details/_index.scss @@ -5,24 +5,16 @@ @include govuk-responsive-margin(6, "bottom"); display: block; - - border-left: $govuk-border-width-wide solid $govuk-border-colour; } .govuk-details__summary { // Make the focus outline shrink-wrap the text content of the summary display: inline-block; - margin-top: govuk-spacing(3); - margin-bottom: govuk-spacing(1); } .govuk-details__summary-text { - @include govuk-typography-weight-bold; - @include govuk-responsive-margin(4, "bottom"); - padding-left: govuk-spacing(4); - > :first-child { margin-top: 0; } @@ -48,6 +40,25 @@ margin-bottom: 0; } + // Hack to target IE8 - IE11 (and REALLY old Firefox) + // These browsers don't support the details element, so fall back to looking + // like inset text + @media screen\0 { + .govuk-details { + border-left: $govuk-border-width-wide solid $govuk-border-colour; + } + + .govuk-details__summary { + margin-top: govuk-spacing(3); + } + + .govuk-details__summary-text { + @include govuk-typography-weight-bold; + @include govuk-responsive-margin(4, "bottom"); + padding-left: govuk-spacing(4); + } + } + // We wrap styles for newer browsers in a feature query, which is ignored by // older browsers, which always expand the details element. // @@ -57,19 +68,11 @@ // - support ES6 modules but not the
element (Edge 16 - 18) // - do not support ES6 modules or the
element (eg, IE8+) @supports not (-ms-ime-align: auto) { - // Override default border - .govuk-details { - border-left: none; - } - .govuk-details__summary { // Absolutely position the marker against this element position: relative; - // Override default top margin - margin-top: 0; - // Allow for absolutely positioned marker and align with disclosed text padding-left: govuk-spacing(4) + $govuk-border-width; @@ -88,12 +91,6 @@ // ...but only underline the text, not the arrow .govuk-details__summary-text { @include govuk-link-decoration; - // Override default font weight - @include govuk-typography-weight-regular; - // Override default margin - margin-bottom: 0; - // Override default padding - padding-left: 0; } .govuk-details__summary:hover .govuk-details__summary-text { diff --git a/packages/govuk-frontend/src/govuk/components/details/implementation.md b/packages/govuk-frontend/src/govuk/components/details/implementation.md index ed982510ff..0d3f6464d9 100644 --- a/packages/govuk-frontend/src/govuk/components/details/implementation.md +++ b/packages/govuk-frontend/src/govuk/components/details/implementation.md @@ -1,5 +1,49 @@ ## Implementation notes +### Styling in older browsers + +#### Browsers with support for `type=module`, `
` and feature queries + +https://browsersl.ist/#q=supports+details+and+supports+css-featurequeries+and+supports+es6-module®ion=GB + +Previously, we provided a polyfill for older browsers which do not support the +`
` component. Since most browsers now DO support `
`, we've +removed that polyfill, and we need to make sure browsers which don't support +`
` don't use any styles that make them look interactable. + +By wrapping these styles in a feature query, we can capture the vast majority of +browsers which have full support. + +#### Browsers that support `type=module` and feature queries but not `
` + +https://browsersl.ist/#q=supports+es6-module+and+not+supports+details+and+supports+css-featurequeries®ion=GB + +This only affects Edge 16 - 18. We filter these out of the support query by +checking for `-ms-ime-align: auto`, which isn't supported by any other browsers. + +#### Browsers that support `
` but not `type=module` or feature queries + +https://browsersl.ist/#q=supports+details+and+not+supports+css-featurequeries+and+not+supports+es6-module®ion=GB + +These will default to their native styling of the `
` element, with a +few of our spacing and font styles. + +#### Browsers which don't support `
`, `type=module` or feature queries + +https://browsersl.ist/#q=%3E0.0000001%25+and+not+supports+details+and+not+supports+es6-module+and+not+supports+css-featurequeries®ion=GB + +This is largely IE 8 - 11. + +We can account for IE by styling them like inset text, via a `@media screen\0` +hack that no other browser supports. + +#### Browsers that support feature queries, but not `
` or `type=module` + +https://browsersl.ist/#q=supports+css-featurequeries+and+not+supports+details+and+not+supports+es6-module®ion=GB + +This is the only gap, as these browsers will get styled to look interactable, +even though they aren't. This is largely Opera Mini. + ### Marker styling Firefox uses display: list-item to show the arrow marker for the summary From c8f57d06780a32254156179cc0da31f65f287223 Mon Sep 17 00:00:00 2001 From: Brett Kyle Date: Tue, 20 Jun 2023 15:34:06 +0100 Subject: [PATCH 7/7] Add CHANGELOG entry --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f119b9047..b08436cefd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,16 @@ This change was made in [pull request #3599: Enable new link styles by default]( You must make the following changes when you migrate to this release, or your service might break. +#### Check that details components work as expected + +The Details component no longer uses JavaScript, and is no longer polyfilled in older browsers. + +If you aren’t using our Nunjucks macros, ensure you remove the `data-module="govuk-details"` attribute from all `
` elements. + +If you have extended browser support requirements, check that the Details component works as expected in older browsers. + +This change was introduced in [pull request #3766: Remove JavaScript from Details component](https://github.com/alphagov/govuk-frontend/pull/3766). + #### Update package file paths In preparation for additional build targets, we've moved our package files into a directory called `dist`.