From 74c4722384325c59f794a128c5f2ea5c52033af9 Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 4 Apr 2024 20:42:25 +0100 Subject: [PATCH] Contrast and colour fixes (#115) * Fix tna-template--black-accent class * Better support for high contrast mode * Tidy up colour functions, remove old mixins * Fix form error colour on error summary component * Increase Storybook test timeout * Split up colour combination stories * Try multiple test runners * Build Storybook for tests only on CI * Build Storybook for tests only on CI * Use npm install rather than npm ci for some actions * Use SWC as a builder * Switch back to npm ci --- .github/actions/storybook-tests/action.yml | 2 +- .github/workflows/pull-request.yml | 2 +- .storybook/main.js | 2 +- .storybook/preview.js | 1 - CHANGELOG.md | 11 ++ package.json | 3 +- .../components/global-header/README.md | 2 +- .../global-header/global-header.scss | 32 ++--- .../components/header/header.scss | 45 ++----- .../components/hero/hero.scss | 3 +- .../components/search-field/search-field.scss | 31 +++-- .../colour-combinations.stories.js | 109 ++++++++++++----- .../colour-schemes/colour-themes.stories.js | 112 +++++++++++------- src/nationalarchives/tools/_a11y.scss | 4 - src/nationalarchives/tools/_colour.scss | 92 +++++--------- src/nationalarchives/utilities/_colour.scss | 39 +----- 16 files changed, 248 insertions(+), 242 deletions(-) diff --git a/.github/actions/storybook-tests/action.yml b/.github/actions/storybook-tests/action.yml index bd2afb69..10e45e5d 100644 --- a/.github/actions/storybook-tests/action.yml +++ b/.github/actions/storybook-tests/action.yml @@ -14,7 +14,7 @@ runs: run: npm ci shell: bash - name: Build Storybook - run: npm run build:storybook --test + run: npm run build:storybook:tests shell: bash - name: Start Storybook run: npm start & diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 2f355593..b09ee993 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -23,7 +23,7 @@ jobs: - name: Install dependencies run: npm ci - name: Build Storybook - run: npm run build:storybook + run: npm run build:storybook:tests - uses: chromaui/action@v1 with: projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }} diff --git a/.storybook/main.js b/.storybook/main.js index 657f3dea..34b69a30 100644 --- a/.storybook/main.js +++ b/.storybook/main.js @@ -14,7 +14,7 @@ module.exports = { ], framework: { name: "@storybook/html-webpack5", - options: {}, + options: { builder: { useSWC: true } }, }, staticDirs: ["../src/nationalarchives/assets"], webpackFinal: async (config, { configType }) => { diff --git a/.storybook/preview.js b/.storybook/preview.js index 12bbb954..0fc73356 100644 --- a/.storybook/preview.js +++ b/.storybook/preview.js @@ -6,7 +6,6 @@ import Cookies from "../src/nationalarchives/lib/cookies.mjs"; document.documentElement.classList.add( "tna-template", - "tna-template--light-theme", "tna-template--yellow-accent", ); if (window.self !== window.top) { diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ec9e388..6fea0bfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ### Changed + +- Better support for colour themes and high contrast options + ### Deprecated + +- Removed `tna-template--light-theme` option - `tna-template` is light by default +- Removed `accent-background` and `dark` mixins from colour tools + ### Removed ### Fixed + +- Fix incorrect `tna-template--black-accent` class +- Removed all redundant high contrast classes + ### Security ## [0.1.51](https://github.com/nationalarchives/tna-frontend/compare/v0.1.50...v0.1.51) - 2024-04-04 diff --git a/package.json b/package.json index 29011410..dd960b1a 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "scripts": { "start": "storybook dev -p 6006", "build:storybook": "storybook build -o storybook --webpack-stats-json", + "build:storybook:tests": "storybook build -o storybook --webpack-stats-json --test", "build:package": "./tasks/build-package.sh", "compile:sass": "sass --style=compressed --embed-sources src/nationalarchives:package/nationalarchives", "compile:scripts": "webpack", @@ -14,7 +15,7 @@ "test:html": "node tasks/generate-fixture-html.js && html-validate fixtures-html", "test:lint": "prettier --check '{src,.storybook,tasks,.}/**/*.{js,mjs,scss,json,html}' && stylelint 'src/**/*.scss' && eslint 'src/**/*.{js,mjs}'", "test:package": "node tasks/test-package.js", - "test:storybook": "test-storybook --maxWorkers=1", + "test:storybook": "test-storybook", "test:unit": "jest --verbose", "update:fixtures": "node tasks/update-fixtures.js" }, diff --git a/src/nationalarchives/components/global-header/README.md b/src/nationalarchives/components/global-header/README.md index 61b0cd5c..6aa61fc4 100644 --- a/src/nationalarchives/components/global-header/README.md +++ b/src/nationalarchives/components/global-header/README.md @@ -12,7 +12,7 @@ Use the cookie banner from TNA Frontend in your service. ### HTML -1. Add the classes `tna-template` and `tna-template--light-theme` to your `` element +1. Add the classes `tna-template` to your `` element 1. Add the class `tna-template__body` to your `` element 1. Add the HTML for the header and footer to the appropriate part of your page (HTML found below) 1. Change the `#main-content` in the `href` of the skip link to the ID of your `
` element diff --git a/src/nationalarchives/components/global-header/global-header.scss b/src/nationalarchives/components/global-header/global-header.scss index b62ceaaa..3ea8d1c8 100644 --- a/src/nationalarchives/components/global-header/global-header.scss +++ b/src/nationalarchives/components/global-header/global-header.scss @@ -9,27 +9,19 @@ padding-top: spacing.space(1); @include colour.contrast; - background: #010101; - - .tna-template--light-theme & { - background: linear-gradient( - 0deg, - rgb(34 34 34 / 100%) 0%, - rgb(0 0 0 / 100%) 100% - ); + background: linear-gradient( + 0deg, + rgb(34 34 34 / 100%) 0%, + rgb(0 0 0 / 100%) 100% + ); + + .tna-template--dark-theme & { + background: colour.brand-colour("black"); } .tna-template--system-theme & { - @media (prefers-color-scheme: light) { - background: linear-gradient( - 0deg, - rgb(34 34 34 / 100%) 0%, - rgb(0 0 0 / 100%) 100% - ); - } - @media (prefers-color-scheme: dark) { - background: #010101; + background: colour.brand-colour("black"); } } @@ -388,9 +380,11 @@ } } - @include colour.on-high-contrast { + @include colour.on-high-contrast-and-forced-colours { + @include colour.colour-border("keyline", 1px, solid, bottom); + &__top-navigation-link { - @include colour.colour-font("link"); + @include colour.colour-font("link", true); } } } diff --git a/src/nationalarchives/components/header/header.scss b/src/nationalarchives/components/header/header.scss index 7e6cf245..99993c6d 100644 --- a/src/nationalarchives/components/header/header.scss +++ b/src/nationalarchives/components/header/header.scss @@ -10,23 +10,19 @@ position: relative; - background-color: colour.brand-colour("black"); - - .tna-template--light-theme & { - background: linear-gradient( - 0deg, - rgb(34 34 34 / 100%) 0%, - rgb(0 0 0 / 100%) 100% - ); + background: linear-gradient( + 0deg, + rgb(34 34 34 / 100%) 0%, + rgb(0 0 0 / 100%) 100% + ); + + .tna-template--dark-theme & { + background: colour.brand-colour("black"); } .tna-template--system-theme & { - @media (prefers-color-scheme: light) { - background: linear-gradient( - 0deg, - rgb(34 34 34 / 100%) 0%, - rgb(0 0 0 / 100%) 100% - ); + @media (prefers-color-scheme: dark) { + background: colour.brand-colour("black"); } } @@ -481,22 +477,7 @@ } } - // @include colour.on-high-contrast { - // &::after { - // @include colour.colour-border("keyline-dark"); - // } - - // @include media.on-larger-than-mobile { - // &__navigation-item-link { - // &:hover, - // &--selected { - // &, - // &:link, - // &:visited { - // @include colour.colour-border("keyline-dark"); - // } - // } - // } - // } - // } + @include colour.on-high-contrast-and-forced-colours { + @include colour.colour-border("keyline", 1px, solid, bottom); + } } diff --git a/src/nationalarchives/components/hero/hero.scss b/src/nationalarchives/components/hero/hero.scss index dc8ce02a..1c8894b3 100644 --- a/src/nationalarchives/components/hero/hero.scss +++ b/src/nationalarchives/components/hero/hero.scss @@ -244,7 +244,8 @@ $shift-units: 5 !default; } &__content-inner { - padding: spacing.space(2) 0; + padding-right: 0; + padding-left: 0; } &--shifted &__content-inner { diff --git a/src/nationalarchives/components/search-field/search-field.scss b/src/nationalarchives/components/search-field/search-field.scss index a5d6ba11..f7004342 100644 --- a/src/nationalarchives/components/search-field/search-field.scss +++ b/src/nationalarchives/components/search-field/search-field.scss @@ -31,22 +31,39 @@ border-radius: 0.1px; .tna-template--dark-theme &, - .tna-background-contrast &/*, - .tna-background-accent &*/ { + .tna-background-contrast & { margin-right: forms.$form-field-border-width; border-width: 0; } - .tna-template--dark-theme .tna-background-accent &, - .tna-template--dark-theme .tna-background-accent-light &, - .tna-template--light-theme.tna-template--high-contrast-theme - .tna-background-contrast - & { + .tna-template--dark-theme .tna-background-accent-light & { margin-right: 0; border-width: forms.$form-field-border-width; } + + @media (prefers-color-scheme: dark) { + .tna-template--system-theme & { + margin-right: forms.$form-field-border-width; + + border-width: 0; + } + + .tna-background-accent-light & { + margin-right: 0; + + border-width: forms.$form-field-border-width; + } + } + + @include colour.on-high-contrast { + .tna-background-contrast & { + margin-right: 0; + + border-width: forms.$form-field-border-width; + } + } } &__button { diff --git a/src/nationalarchives/stories/utilities/colour-schemes/colour-combinations.stories.js b/src/nationalarchives/stories/utilities/colour-schemes/colour-combinations.stories.js index 7cc1dcbf..bbc3bf13 100644 --- a/src/nationalarchives/stories/utilities/colour-schemes/colour-combinations.stories.js +++ b/src/nationalarchives/stories/utilities/colour-schemes/colour-combinations.stories.js @@ -1,5 +1,6 @@ import Button from "../../../components/button/template.njk"; import Checkboxes from "../../../components/checkboxes/template.njk"; +import ErrorSummary from "../../../components/error-summary/template.njk"; import Radios from "../../../components/radios/template.njk"; import Select from "../../../components/select/template.njk"; import TextInput from "../../../components/text-input/template.njk"; @@ -11,13 +12,8 @@ export default { argTypes, }; -const Template = () => { - const themes = [ - "tna-template--light-theme", - "tna-template--light-theme tna-template--high-contrast-theme", - "tna-template--dark-theme", - "tna-template--dark-theme tna-template--high-contrast-theme", - ]; +const Template = ({ theme }) => { + const themeSlug = theme.replace(" ", "-").toLowerCase(); const accents = [ "", @@ -51,17 +47,12 @@ const Template = () => { )}
- ${themes.reduce( - (themeOutput, theme) => - `${themeOutput}${accents.reduce( - ( - accentOutput, - accent, - ) => `${accentOutput}
+ ${accents.reduce( + ( + accentOutput, + accent, + ) => `${accentOutput}
-

Theme: ${theme - .replace(/tna-template--/g, "") - .replace(/-theme/g, "")}

Accent: ${ accent.replace(/tna-accent-/g, "") || "[none]" }

@@ -71,7 +62,7 @@ const Template = () => { `${blockOutput}
-
Heading
+

Heading

Text / Dark / Light /

Link / Link (visited)

    @@ -85,18 +76,42 @@ const Template = () => { Chip
+ ${ErrorSummary({ + params: { + title: "Error", + headingLevel: 2, + items: [ + { + text: "Error", + href: `name-${themeSlug}-${block}-${accent}-2`, + }, + ], + disableAutoFocus: true, + }, + })} ${TextInput({ params: { label: "Input", - id: `name-${theme}-${block}-${accent}`, - name: `name-${theme}-${block}-${accent}`, + id: `name-${themeSlug}-${block}-${accent}`, + name: `name-${themeSlug}-${block}-${accent}`, value: `Lorem ipsum`, }, })} + ${TextInput({ + params: { + label: "Input", + id: `name-${themeSlug}-${block}-${accent}-2`, + name: `name-${themeSlug}-${block}-${accent}-2`, + value: `Lorem ipsum`, + error: { + text: "Error", + }, + }, + })} ${Checkboxes({ params: { - id: `categories-${theme}-${block}-${accent}`, - name: `categories-${theme}-${block}-${accent}`, + id: `categories-${themeSlug}-${block}-${accent}`, + name: `categories-${themeSlug}-${block}-${accent}`, items: [ { text: "Alpha", @@ -114,8 +129,8 @@ const Template = () => { })} ${Radios({ params: { - id: `type-${theme}-${block}-${accent}`, - name: `type-${theme}-${block}-${accent}`, + id: `type-${themeSlug}-${block}-${accent}`, + name: `type-${themeSlug}-${block}-${accent}`, items: [ { text: "Alpha", @@ -134,8 +149,8 @@ const Template = () => { ${Select({ params: { label: "Select", - id: `sort-${theme}-${block}-${accent}`, - name: `sort-${theme}-${block}-${accent}`, + id: `sort-${themeSlug}-${block}-${accent}`, + name: `sort-${themeSlug}-${block}-${accent}`, items: [ { text: "Relevance", @@ -180,18 +195,48 @@ const Template = () => { "", )}
`, - "", - )}`, "", )}
`; }; -export const Combinations = Template.bind({}); -Combinations.parameters = { +export const Light = Template.bind({}); +Light.parameters = { a11y: { - disable: true, + // disable: true, }, }; -Combinations.args = {}; +Light.args = { + theme: "", +}; + +export const Dark = Template.bind({}); +Dark.parameters = { + a11y: { + // disable: true, + }, +}; +Dark.args = { + theme: "tna-template--dark-theme", +}; + +export const HighContrast = Template.bind({}); +HighContrast.parameters = { + a11y: { + // disable: true, + }, +}; +HighContrast.args = { + theme: "tna-template--high-contrast-theme", +}; + +export const DarkHighContrast = Template.bind({}); +DarkHighContrast.parameters = { + a11y: { + // disable: true, + }, +}; +DarkHighContrast.args = { + theme: "tna-template--dark-theme tna-template--high-contrast-theme", +}; diff --git a/src/nationalarchives/stories/utilities/colour-schemes/colour-themes.stories.js b/src/nationalarchives/stories/utilities/colour-schemes/colour-themes.stories.js index 592a76a3..f5b4dda4 100644 --- a/src/nationalarchives/stories/utilities/colour-schemes/colour-themes.stories.js +++ b/src/nationalarchives/stories/utilities/colour-schemes/colour-themes.stories.js @@ -28,8 +28,8 @@ const argTypes = { "system", "light", "dark", - "light high-contrast", - "dark high-contrast", + // "light high-contrast", + // "dark high-contrast", ], }, accent: { @@ -46,7 +46,6 @@ export default { const Template = ({ theme, accent }) => { document.documentElement.classList.remove( "tna-template", - "tna-template--light-theme", "tna-template--yellow-accent", ); @@ -75,15 +74,13 @@ const Template = ({ theme, accent }) => { return `
{ classes: "tna-pagination--demo tna-!--margin-top-m", }, })} + ${SearchField({ + params: { + label: "Catalogue search results", + headingLevel: 3, + headingSize: "l", + id: "search1", + name: "q", + }, + })}
@@ -801,9 +808,19 @@ const Template = ({ theme, accent }) => { classes: "tna-pagination--demo tna-!--margin-top-m", }, })} + ${SearchField({ + params: { + label: "Catalogue search results", + headingLevel: 3, + headingSize: "l", + id: "search2", + name: "q", + }, + })}
@@ -885,9 +902,19 @@ const Template = ({ theme, accent }) => { classes: "tna-pagination--demo tna-!--margin-top-m", }, })} + ${SearchField({ + params: { + label: "Catalogue search results", + headingLevel: 3, + headingSize: "l", + id: "search3", + name: "q", + }, + })}
@@ -969,9 +996,19 @@ const Template = ({ theme, accent }) => { classes: "tna-pagination--demo tna-!--margin-top-m", }, })} + ${SearchField({ + params: { + label: "Catalogue search results", + headingLevel: 3, + headingSize: "l", + id: "search4", + name: "q", + }, + })}
@@ -1053,26 +1090,21 @@ const Template = ({ theme, accent }) => { classes: "tna-pagination--demo tna-!--margin-top-m", }, })} - -
- - -
-
-
${SearchField({ params: { label: "Catalogue search results", headingLevel: 3, headingSize: "l", - id: "search1", + id: "search5", name: "q", }, })} +
@@ -1374,20 +1406,20 @@ Dark.args = { accent: "pink", }; -export const LightHighContrast = Template.bind({}); -LightHighContrast.parameters = { - chromatic: { disableSnapshot: true }, -}; -LightHighContrast.args = { - theme: "light high-contrast", - accent: "pink", -}; +// export const LightHighContrast = Template.bind({}); +// LightHighContrast.parameters = { +// chromatic: { disableSnapshot: true }, +// }; +// LightHighContrast.args = { +// theme: "light high-contrast", +// accent: "pink", +// }; -export const DarkHighContrast = Template.bind({}); -DarkHighContrast.parameters = { - chromatic: { disableSnapshot: true }, -}; -DarkHighContrast.args = { - theme: "dark high-contrast", - accent: "pink", -}; +// export const DarkHighContrast = Template.bind({}); +// DarkHighContrast.parameters = { +// chromatic: { disableSnapshot: true }, +// }; +// DarkHighContrast.args = { +// theme: "dark high-contrast", +// accent: "pink", +// }; diff --git a/src/nationalarchives/tools/_a11y.scss b/src/nationalarchives/tools/_a11y.scss index 3cdc9b39..c40496a6 100644 --- a/src/nationalarchives/tools/_a11y.scss +++ b/src/nationalarchives/tools/_a11y.scss @@ -8,10 +8,6 @@ solid ); outline-offset: a11y.$focus-outline-offset; - - transition: - outline 100ms, - outline-offset 100ms; } @mixin active-outline { diff --git a/src/nationalarchives/tools/_colour.scss b/src/nationalarchives/tools/_colour.scss index 21c82d4b..8ca4ee22 100644 --- a/src/nationalarchives/tools/_colour.scss +++ b/src/nationalarchives/tools/_colour.scss @@ -154,65 +154,43 @@ solid; } +// Use light theme colours (except for "form-error") %light { @include colour-css-vars("form-error"); @media (prefers-contrast: more) { @include colour-css-vars-high-contrast("form-error"); } - - .tna-template--high-contrast-theme & { - @include colour-css-vars-high-contrast("form-error"); - } } @mixin light { @extend %light; } -%dark { - @include colour-css-vars-dark("form-error"); +// Remove accent and contrast values (except for "form-error") +%plain { + @include colour-css-vars("form-error"); @media (prefers-contrast: more) { - @include colour-css-vars-high-contrast-dark("form-error"); + @include colour-css-vars-high-contrast("form-error"); } - .tna-template--high-contrast-theme & { - @include colour-css-vars-high-contrast-dark("form-error"); - } -} - -@mixin dark { - @extend %dark; -} - -%plain { .tna-template--system-theme & { - @extend %light; - @media (prefers-color-scheme: dark) { - @include colour-css-vars-dark; + @include colour-css-vars-dark("form-error"); } @media (prefers-contrast: more) and (prefers-color-scheme: dark) { - @include colour-css-vars-high-contrast-dark; + @include colour-css-vars-high-contrast-dark("form-error"); } } - .tna-template--light-theme & { - @include colour-css-vars; - } - .tna-template--dark-theme & { - @include colour-css-vars-dark; - } - - .tna-template--high-contrast-theme & { - @include colour-css-vars-high-contrast; - } + @include colour-css-vars-dark("form-error"); - .tna-template--high-contrast-theme.tna-template--dark-theme & { - @include colour-css-vars-high-contrast-dark; + @media (prefers-contrast: more) { + @include colour-css-vars-high-contrast-dark("form-error"); + } } @include colour-background("background"); @@ -272,6 +250,18 @@ @extend %contrast-on-mobile; } +%tint { + --background: var(--background-tint); + + @include colour-background("background"); + + @include colour-font("font-base"); +} + +@mixin tint { + @extend %tint; +} + %accent { --background: var(--accent-background); --font-base: var(--accent-font-base); @@ -292,34 +282,12 @@ @extend %accent; } -%tint { - --background: var(--background-tint); - - @include colour-background("background"); - - @include colour-font("font-base"); -} - -@mixin tint { - @extend %tint; -} - -%accent-background { - @include colour-background("accent-background"); -} - -@mixin accent-background { - @extend %accent-background; -} - %accent-light { --background: var(--accent-background-light); --font-base: #{map.get(colour.$colour-palette-default, "font-base")}; --font-dark: #{map.get(colour.$colour-palette-default, "font-dark")}; --font-light: #{map.get(colour.$colour-palette-default, "font-light")}; --icon-light: #{map.get(colour.$colour-palette-default, "icon-light")}; - // --link: #{map.get(colour.$colour-palette-default, "link")}; - // --link-visited: #{map.get(colour.$colour-palette-default, "link-visited")}; --keyline: #{map.get(colour.$colour-palette-default, "keyline")}; --keyline-dark: #{map.get(colour.$colour-palette-default, "keyline-dark")}; --button-text: #{map.get(colour.$colour-palette-default, "button-text")}; @@ -336,9 +304,6 @@ "button-hover-background" )}; - @include colour-background("background"); - @include colour-font("font-base"); - .tna-template--system-theme & { @media (prefers-color-scheme: dark) { --link: #{map.get(colour.$colour-palette-default, "link")}; @@ -350,6 +315,9 @@ --link: #{map.get(colour.$colour-palette-default, "link")}; --link-visited: #{map.get(colour.$colour-palette-default, "link-visited")}; } + + @include colour-background("background"); + @include colour-font("font-base"); } @mixin accent-light { @@ -444,15 +412,9 @@ } @mixin on-high-contrast { - .tna-template--high-contrast-theme & { + @media (prefers-contrast: more) { @content; } - - .tna-template--system-theme & { - @media (prefers-contrast: more) { - @content; - } - } } @mixin on-forced-colours { diff --git a/src/nationalarchives/utilities/_colour.scss b/src/nationalarchives/utilities/_colour.scss index 9f1ca81c..1e9bd3ce 100644 --- a/src/nationalarchives/utilities/_colour.scss +++ b/src/nationalarchives/utilities/_colour.scss @@ -1,41 +1,20 @@ @use "../tools/colour"; .tna-template { - @include colour.colour-background("background"); - - &--system-theme, - &--light-theme { - @include colour.colour-css-vars; + @include colour.light; - @media (prefers-contrast: more) { - @include colour.colour-css-vars-high-contrast; - } - } + @include colour.colour-background("background"); &--system-theme { - // @include colour.colour-css-vars; - @media (prefers-color-scheme: dark) { @include colour.colour-css-vars-dark; } - // @media (prefers-contrast: more) { - // @include colour.colour-css-vars-high-contrast; - // } - @media (prefers-contrast: more) and (prefers-color-scheme: dark) { @include colour.colour-css-vars-high-contrast-dark; } } - // &--light-theme { - // @include colour.colour-css-vars; - - // @media (prefers-contrast: more) { - // @include colour.colour-css-vars-high-contrast; - // } - // } - &--dark-theme { @include colour.colour-css-vars-dark; @@ -44,19 +23,7 @@ } } - // &--high-contrast-theme { - // @include colour.colour-css-vars-high-contrast; - - // * { - // background-image: none !important; - // } - - // &.tna-template--dark-theme { - // @include colour.colour-css-vars-high-contrast-dark; - // } - // } - - &--black--accent { + &--black-accent { @include colour.black-accent; }