From 9085f08ce2b5245cf0d44992ee8050f599db06af Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 2 Jan 2024 15:48:58 +0100 Subject: [PATCH 1/7] :recycle: [#471] Refactor summary component markup The summary component now uses the utrecht-heading-2 component for its title element, with a couple of overrides so that we can keep applying the utrecht-heading-3 styles/appearance. The header itself remains a custom component, but we now expose a number of presentation parameters through design tokens. Padding-top for the first element is removed - a block/component should not affect its surroundings. This leads to small styling changes on the overview page, but the title spacing still provides sufficient styling. If more spacing is needed, we can wrap it in another component and apply flexbox/grid layout and the respective gaps to create a more comfy layout. Upgrade notes have been updated and all changes are backwards compatible, defaults are scheduled for removal in the next major version. --- src/components/FormStepSummary/index.js | 7 +-- src/scss/components/_summary.scss | 69 ++++++++++++++++-------- src/scss/components/_typography.scss | 3 -- src/scss/nl-design-system-community.scss | 3 ++ src/upgrade-notes.mdx | 27 ++++++++++ 5 files changed, 82 insertions(+), 27 deletions(-) diff --git a/src/components/FormStepSummary/index.js b/src/components/FormStepSummary/index.js index bb0021292..791f2aa14 100644 --- a/src/components/FormStepSummary/index.js +++ b/src/components/FormStepSummary/index.js @@ -1,3 +1,4 @@ +import {Heading2} from '@utrecht/component-library-react'; import PropTypes from 'prop-types'; import React from 'react'; @@ -42,9 +43,9 @@ const FormStepSummary = ({editUrl, slug, name, data, editStepText = ''}) => { } return ( -
-
-

{name}

+
+
+ {name} {editStepText && ( diff --git a/src/scss/components/_summary.scss b/src/scss/components/_summary.scss index 04a767bef..223dbeb96 100644 --- a/src/scss/components/_summary.scss +++ b/src/scss/components/_summary.scss @@ -1,35 +1,62 @@ @use 'microscope-sass/lib/bem'; -@import '~microscope-sass/lib/typography'; +@import '~microscope-sass/lib/responsive'; -@import '../mixins/prefix'; - -.#{prefix('summary')} { - &:first-child { - padding-top: $grid-margin-2; +/** + * Open Forms extension for summary page context. + */ +.utrecht-heading-2 { + @include bem.modifier('openforms-summary-step-name') { + // TODO: default values for `var(...)` are for backwards compatibility, remove in + // SDK 3.0. + font-size: var(--of-summary-step-name-font-size, 1.1875rem); + line-height: var(--of-summary-step-name-line-height, 1.17); + overflow-wrap: break-word; } +} - @include bem.element('step-header') { +/** + * Container for the header + datalist of the summary. + */ +.openforms-summary { + // TODO: default values for `var(...)` are for backwards compatibility, remove in + // SDK 3.0. + + @include bem.element('header') { display: flex; align-items: center; justify-content: space-between; - max-width: 100%; - flex-wrap: wrap; + column-gap: var(--of-summary-header-column-gap); - border-bottom-style: $typography-style-border; - border-bottom-width: $typography-size-border; - border-bottom-color: $color-border; - padding-bottom: $grid-margin-2; + padding-block-end: var(--of-summary-header-padding-block-end, 12px); - .fa-icon { - padding-right: 5px; - } + border-block-end-style: var(--of-summary-header-border-block-end-style, solid); + border-block-end-width: var(--of-summary-header-border-block-end-width, 1px); + border-block-end-color: var(--of-summary-header-border-block-end-color, var(--of-color-border)); } - @include bem.element('step-name') { - @include body; - @include h3; - max-width: 100%; - overflow-wrap: break-word; + // TODO: check if there's a modifier for utrecht-link with icon instead. + .utrecht-link.utrecht-link--openforms .fa-icon { + padding-inline-end: var(--of-summary-header-link-icon-padding-inline-end, 5px); } + + // Responsive styling - try to keep the "edit" link inline, if the form label is too + // long -> wrap onto the next line. + @include mobile-only { + @include bem.element('header') { + flex-wrap: wrap; + row-gap: var(--of-summary-step-name-header-mobile-row-gap); + } + } +} + +// TODO: move to design tokens repository +.openforms-theme { + --of-summary-step-name-font-size: var(--utrecht-heading-3-font-size); + --of-summary-step-name-line-height: var(--utrecht-heading-3-line-height, 1.17); + --of-summary-header-mobile-row-gap: 8px; + --of-summary-header-padding-block-end: 12px; + --of-summary-header-border-block-end-style: solid; + --of-summary-header-border-block-end-width: 1px; + --of-summary-header-border-block-end-color: var(--of-color-border); } diff --git a/src/scss/components/_typography.scss b/src/scss/components/_typography.scss index 5f55ecb38..f2cf878da 100644 --- a/src/scss/components/_typography.scss +++ b/src/scss/components/_typography.scss @@ -1,9 +1,6 @@ @use 'microscope-sass/lib/bem'; @use './anchor'; -@import '@utrecht/components/dist/heading-1/css/index'; -@import '@utrecht/components/dist/heading-3/css/index'; - @import '~microscope-sass/lib/grid'; @import '~microscope-sass/lib/typography'; diff --git a/src/scss/nl-design-system-community.scss b/src/scss/nl-design-system-community.scss index 9b0ecf7d9..1a5f87995 100644 --- a/src/scss/nl-design-system-community.scss +++ b/src/scss/nl-design-system-community.scss @@ -6,3 +6,6 @@ @import '@utrecht/components/dist/radio-button/css/index.css'; @import '@utrecht/components/dist/alert/css/index.css'; @import '@utrecht/components/dist/data-list/css/index.css'; +@import '@utrecht/components/dist/heading-1/css/index'; +@import '@utrecht/components/dist/heading-2/css/index'; +@import '@utrecht/components/dist/heading-3/css/index'; diff --git a/src/upgrade-notes.mdx b/src/upgrade-notes.mdx index abd11ec6c..273b99bb0 100644 --- a/src/upgrade-notes.mdx +++ b/src/upgrade-notes.mdx @@ -89,3 +89,30 @@ following design tokens: - `--of-language-selection-in-app-padding-block-start` - `--of-language-selection-in-app-mobile-padding-block-end` - `--of-language-selection-in-app-mobile-padding-block-start` + +### Form (step) summary + +We've refactored our custom components to make use of NL Design System components. + +**Step name/title** + +The `h2` with classname `.openforms-summary__step-name` is now the Heading 2 component ( with +classnames `.utrecht-heading-2.utrecht-heading-2-openforms-summary-step-name`). + +We have custom design tokens for the font-size and line-height that change the appearance to a +heading 3 element (for backwards compatibility). Relevant design tokens: + +- `--of-summary-step-name-font-size` +- `--of-summary-step-name-line-height` + +**Step header** + +The step header now provides design tokens for the bottom padding (space between text and border) +and the bottom border. The default values are specified to be backwards compatible. + +Relevant design tokens: + +- `--of-summary-header-padding-block-end` +- `--of-summary-header-border-block-end-style` +- `--of-summary-header-border-block-end-width` +- `--of-summary-header-border-block-end-color` From 0eb276dffe65c984ae8dab88bed2b50b64eac132 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 2 Jan 2024 16:41:54 +0100 Subject: [PATCH 2/7] :recycle: [#471] Refactor summary page to use data list component/markup Mobile and laptop+ styles should be equivalent. --- src/components/FormStepSummary/index.js | 27 ++++++----- src/scss/components/_data-list.scss | 64 +++++++++++++++++++++++++ src/styles.scss | 1 + 3 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 src/scss/components/_data-list.scss diff --git a/src/components/FormStepSummary/index.js b/src/components/FormStepSummary/index.js index 791f2aa14..26f347b1b 100644 --- a/src/components/FormStepSummary/index.js +++ b/src/components/FormStepSummary/index.js @@ -1,26 +1,31 @@ -import {Heading2} from '@utrecht/component-library-react'; +import { + DataList, + DataListItem, + DataListKey, + DataListValue, + Heading2, +} from '@utrecht/component-library-react'; import PropTypes from 'prop-types'; import React from 'react'; import FAIcon from 'components/FAIcon'; -import ComponentValueDisplay from 'components/FormStepSummary/ComponentValueDisplay'; import Link from 'components/Link'; import {DEBUG} from 'utils'; -import {getBEMClassName} from 'utils'; + +import ComponentValueDisplay from './ComponentValueDisplay'; const LabelValueRow = ({name, value, component}) => { if (!name) { return null; } - const className = getBEMClassName('summary-row', [component.type]); return ( -
-
{name}
-
+ + {name} + -
-
+ + ); }; @@ -55,7 +60,7 @@ const FormStepSummary = ({editUrl, slug, name, data, editStepText = ''}) => { )}
-
+ {data.map((item, index) => ( { component={item.component} /> ))} -
+
); }; diff --git a/src/scss/components/_data-list.scss b/src/scss/components/_data-list.scss new file mode 100644 index 000000000..5457dce26 --- /dev/null +++ b/src/scss/components/_data-list.scss @@ -0,0 +1,64 @@ +@use 'microscope-sass/lib/bem'; + +@import '~microscope-sass/lib/responsive'; + +/** + * Extensions on the utrecht-data-list component. + * + * Reference: https://nl-design-system.github.io/utrecht/storybook/?path=/docs/css_css-data-list--docs + */ +.utrecht-data-list { + @include bem.modifier('openforms') { + display: flex; + flex-direction: column; + gap: var(--of-utrecht-data-list-gap, var(--of-summary-row-spacing)); + padding-block-start: var( + --of-utrecht-data-list-padding-block-start, + var(--of-summary-row-spacing) + ); + padding-block-end: var( + --of-utrecht-data-list-padding-block-end, + calc(2 * var(--of-summary-row-spacing)) + ); + + @include bem.element('item') { + gap: var(--of-utrecht-data-list-item-gap, 8px); + } + + @include bem.element('item-key') { + overflow-wrap: break-word; + flex-basis: 50%; + } + + @include bem.element('item-value') { + overflow-wrap: break-word; + flex-basis: 50%; + } + + @include laptop { + @include bem.element('item') { + display: flex; + } + + @include bem.element('item-key') { + font-weight: var(--of-utrecht-data-list-item-key-laptop-font-weight, 700); + } + } + } +} + +// TODO: move to design tokens repository +.openforms-theme { + --of-utrecht-data-list-padding-block-end: 20px; + + // apply body text styling + --utrecht-data-list-item-key-color: var(--of-color-fg); + --utrecht-data-list-item-key-font-size: 1rem; + --utrecht-data-list-item-key-font-weight: 700; + --utrecht-data-list-item-key-line-height: 1.5; + --of-utrecht-data-list-item-key-laptop-font-weight: 400; + + --utrecht-data-list-item-value-color: var(--of-color-fg); + --utrecht-data-list-item-value-font-size: 1rem; + --utrecht-data-list-item-value-line-height: 1.5; +} diff --git a/src/styles.scss b/src/styles.scss index d8dbcbf2b..8cd84bc3f 100644 --- a/src/styles.scss +++ b/src/styles.scss @@ -30,6 +30,7 @@ @import './scss/components/card'; @import './scss/components/checkbox'; @import './scss/components/content'; +@import './scss/components/data-list'; @import './scss/components/errors'; @import './scss/components/formio-component'; @import './scss/components/help-text'; From e66755e91f7add5781f40cda5931972078f52995 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 2 Jan 2024 17:20:43 +0100 Subject: [PATCH 3/7] :recycle: [#471] Port special-case styling for fieldset/editgrid labels --- .../FormStepSummary.stories.js | 163 ++++++++++++++++++ src/scss/components/_data-list.scss | 27 +++ src/upgrade-notes.mdx | 33 ++++ 3 files changed, 223 insertions(+) diff --git a/src/components/FormStepSummary/FormStepSummary.stories.js b/src/components/FormStepSummary/FormStepSummary.stories.js index 2c51e9df8..9dc5583db 100644 --- a/src/components/FormStepSummary/FormStepSummary.stories.js +++ b/src/components/FormStepSummary/FormStepSummary.stories.js @@ -29,6 +29,11 @@ export default { ], editStepText: 'Edit', }, + argTypes: { + name: {type: 'string'}, + editStepText: {type: 'string'}, + slug: {table: {disable: true}}, + }, }; export const Default = { @@ -64,3 +69,161 @@ export const NotEditable = { }); }, }; + +export const WithFieldsetAsFirstElement = { + name: 'Layout components: fieldset, first child', + args: { + data: [ + { + name: 'A fieldset', + value: '', + component: { + type: 'fieldset', + key: 'fieldset', + label: 'A fieldset', + }, + }, + { + name: 'A textfield', + value: 'A value', + component: { + type: 'textfield', + key: 'textfield1', + label: 'A textfield', + }, + }, + { + name: 'Another textfield', + value: 'Another value', + component: { + type: 'textfield', + key: 'textfield2', + label: 'Another textfield', + }, + }, + ], + }, +}; + +export const WithFieldsetNotAsFirstElement = { + name: 'Layout components: fieldset, not first child', + args: { + data: [ + { + name: 'A number', + value: 3.14, + component: { + type: 'number', + key: 'number', + label: 'A number', + }, + }, + { + name: 'A fieldset', + value: '', + component: { + type: 'fieldset', + key: 'fieldset', + label: 'A fieldset', + }, + }, + { + name: 'A textfield', + value: 'A value', + component: { + type: 'textfield', + key: 'textfield1', + label: 'A textfield', + }, + }, + { + name: 'Another textfield', + value: 'Another value', + component: { + type: 'textfield', + key: 'textfield2', + label: 'Another textfield', + }, + }, + ], + }, +}; + +export const WithEditGridAsFirstElement = { + name: 'Layout components: editgrid, first child', + args: { + data: [ + { + name: 'A repeating group', + value: '', + component: { + type: 'editgrid', + key: 'editgrid', + label: 'A repeating group', + }, + }, + { + name: 'A textfield', + value: 'A value', + component: { + type: 'textfield', + key: 'textfield1', + label: 'A textfield', + }, + }, + { + name: 'Another textfield', + value: 'Another value', + component: { + type: 'textfield', + key: 'textfield2', + label: 'Another textfield', + }, + }, + ], + }, +}; + +export const WithEditGridNotAsFirstElement = { + name: 'Layout components: editgrid, not first child', + args: { + data: [ + { + name: 'A number', + value: 3.14, + component: { + type: 'number', + key: 'number', + label: 'A number', + }, + }, + { + name: 'A repeating group', + value: '', + component: { + type: 'editgrid', + key: 'editgrid', + label: 'A repeating group', + }, + }, + { + name: 'A textfield', + value: 'A value', + component: { + type: 'textfield', + key: 'textfield1', + label: 'A textfield', + }, + }, + { + name: 'Another textfield', + value: 'Another value', + component: { + type: 'textfield', + key: 'textfield2', + label: 'Another textfield', + }, + }, + ], + }, +}; diff --git a/src/scss/components/_data-list.scss b/src/scss/components/_data-list.scss index 5457dce26..8e074a941 100644 --- a/src/scss/components/_data-list.scss +++ b/src/scss/components/_data-list.scss @@ -2,6 +2,24 @@ @import '~microscope-sass/lib/responsive'; +@mixin component-type-overrides($component-type) { + .utrecht-data-list__item-key { + font-size: var(--of-utrecht-data-list-item-key-#{$component-type}-font-size, 0.875rem); + font-weight: var(--of-utrecht-data-list-item-key-#{$component-type}-font-weight, 700); + line-height: var(--of-utrecht-data-list-item-key-#{$component-type}-line-height, 1.2); + margin-block-end: var( + --of-utrecht-data-list-item-key-#{$component-type}-margin-block-end, + 0.5rem + ); + } + &:not(:first-child) { + margin-block-start: var( + --of-utrecht-data-list-item-#{$component-type}-margin-block-start, + 20px + ); + } +} + /** * Extensions on the utrecht-data-list component. * @@ -23,6 +41,15 @@ @include bem.element('item') { gap: var(--of-utrecht-data-list-item-gap, 8px); + + // Styling modifications for specific component types + @include bem.modifier('openforms-fieldset') { + @include component-type-overrides('fieldset'); + } + + @include bem.modifier('openforms-editgrid') { + @include component-type-overrides('editgrid'); + } } @include bem.element('item-key') { diff --git a/src/upgrade-notes.mdx b/src/upgrade-notes.mdx index 273b99bb0..a267cc60f 100644 --- a/src/upgrade-notes.mdx +++ b/src/upgrade-notes.mdx @@ -116,3 +116,36 @@ Relevant design tokens: - `--of-summary-header-border-block-end-style` - `--of-summary-header-border-block-end-width` - `--of-summary-header-border-block-end-color` + +**Summary row** + +The key/value pairs in the summary row have been migrated to use the NLDS Data List component, using +the `.utrecht-data-list*` classes and design tokens. + +In the default Open Forms theme, this is backwards compatible, however if you're using your own +theme you now need to define some design tokens to keep the existing styles ( if those aren't +defined yet): + +```css +.your-theme { + --of-utrecht-data-list-padding-block-end: 20px; + + --utrecht-data-list-item-key-color: var(--of-color-fg); /* or an actual color value */ + --utrecht-data-list-item-key-font-size: 1rem; + --utrecht-data-list-item-key-font-weight: 700; + --utrecht-data-list-item-key-line-height: 1.5; + --of-utrecht-data-list-item-key-laptop-font-weight: 400; + + --utrecht-data-list-item-value-color: var(--of-color-fg); /* or an actual color value */ + --utrecht-data-list-item-value-font-size: 1rem; + --utrecht-data-list-item-value-line-height: 1.5; +} +``` + +Spacing tokens fall back to `--of-summary-row-spacing` or values derived from this token. We +recommend explicitly defining the design token values: + +- `--of-utrecht-data-list-gap` +- `--of-utrecht-data-list-padding-block-start` +- `--of-utrecht-data-list-padding-block-end` +- `--of-utrecht-data-list-item-gap` From 43bb7c642772dc38dea0b3262e7e8295be1c781a Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 2 Jan 2024 17:26:31 +0100 Subject: [PATCH 4/7] :fire: [#471] Delete obsolete custom CSS --- src/scss/components/_summary-row.scss | 70 --------------------------- src/styles.scss | 1 - 2 files changed, 71 deletions(-) delete mode 100644 src/scss/components/_summary-row.scss diff --git a/src/scss/components/_summary-row.scss b/src/scss/components/_summary-row.scss deleted file mode 100644 index 9404d534f..000000000 --- a/src/scss/components/_summary-row.scss +++ /dev/null @@ -1,70 +0,0 @@ -@use 'sass:math'; -@use 'microscope-sass/lib/bem'; - -@import '~microscope-sass/lib/typography'; - -@import '../mixins/prefix'; - -$block: '.#{prefix(summary-row)}'; - -#{$block} { - $summary-row-spacing: var(--of-summary-row-spacing); - - @include body; - @include margin(true, $properties: padding-top, $value-mobile: $summary-row-spacing); - @include laptop { - display: flex; - } - - @include bem.element('label') { - overflow-wrap: break-word; - - @include mobile-only { - font-weight: bold; - } - - @include laptop { - flex-grow: 1; - max-width: 50%; - box-sizing: border-box; - padding-right: $grid-margin-1; - } - } - - @include bem.element('value') { - overflow-wrap: break-word; - - @include laptop { - flex-grow: 1; - max-width: 50%; - } - } - - @include bem.modifier('fieldset') { - &:not(:first-child) { - @include margin($properties: padding-top); - } - - @include bem.element('label') { - @include h4(true, $margin-properties: margin-bottom); - font-weight: bold; - } - } - - // TODO Fix so that the label of the repetitions don't have the - // same style as the parent repeating group - @include bem.modifier('editgrid') { - &:not(:first-child) { - @include margin($properties: padding-top); - } - - @include bem.element('label') { - @include h4(true); - font-weight: bold; - } - } - - &:last-child { - @include margin(true, $properties: padding-bottom); - } -} diff --git a/src/styles.scss b/src/styles.scss index 8cd84bc3f..dad4ec670 100644 --- a/src/styles.scss +++ b/src/styles.scss @@ -55,7 +55,6 @@ @import './scss/components/multi-value-row'; @import './scss/components/multi-value-table'; @import './scss/components/react-modal'; -@import './scss/components/summary-row'; @import './scss/components/co-sign'; @import './scss/components/leaflet-map'; @import './scss/components/signature'; From 6a4ab63c8b1349fd9ca62c6b232120b7d8f6ac98 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Tue, 2 Jan 2024 17:40:26 +0100 Subject: [PATCH 5/7] :recycle: [#471] Move design token values to their own repository --- design-tokens | 2 +- src/scss/components/_data-list.scss | 18 +----------------- src/scss/components/_summary.scss | 11 ----------- src/upgrade-notes.mdx | 1 + 4 files changed, 3 insertions(+), 29 deletions(-) diff --git a/design-tokens b/design-tokens index 326390434..ce61470d6 160000 --- a/design-tokens +++ b/design-tokens @@ -1 +1 @@ -Subproject commit 326390434b5eb44bf82c3b1d2996b3a1012014eb +Subproject commit ce61470d68ef1a6fc365a4b0bc245382b83be4d5 diff --git a/src/scss/components/_data-list.scss b/src/scss/components/_data-list.scss index 8e074a941..6ce8266b2 100644 --- a/src/scss/components/_data-list.scss +++ b/src/scss/components/_data-list.scss @@ -64,7 +64,7 @@ @include laptop { @include bem.element('item') { - display: flex; + display: var(--of-utrecht-data-list-laptop-display, block); } @include bem.element('item-key') { @@ -73,19 +73,3 @@ } } } - -// TODO: move to design tokens repository -.openforms-theme { - --of-utrecht-data-list-padding-block-end: 20px; - - // apply body text styling - --utrecht-data-list-item-key-color: var(--of-color-fg); - --utrecht-data-list-item-key-font-size: 1rem; - --utrecht-data-list-item-key-font-weight: 700; - --utrecht-data-list-item-key-line-height: 1.5; - --of-utrecht-data-list-item-key-laptop-font-weight: 400; - - --utrecht-data-list-item-value-color: var(--of-color-fg); - --utrecht-data-list-item-value-font-size: 1rem; - --utrecht-data-list-item-value-line-height: 1.5; -} diff --git a/src/scss/components/_summary.scss b/src/scss/components/_summary.scss index 223dbeb96..bee6b3849 100644 --- a/src/scss/components/_summary.scss +++ b/src/scss/components/_summary.scss @@ -49,14 +49,3 @@ } } } - -// TODO: move to design tokens repository -.openforms-theme { - --of-summary-step-name-font-size: var(--utrecht-heading-3-font-size); - --of-summary-step-name-line-height: var(--utrecht-heading-3-line-height, 1.17); - --of-summary-header-mobile-row-gap: 8px; - --of-summary-header-padding-block-end: 12px; - --of-summary-header-border-block-end-style: solid; - --of-summary-header-border-block-end-width: 1px; - --of-summary-header-border-block-end-color: var(--of-color-border); -} diff --git a/src/upgrade-notes.mdx b/src/upgrade-notes.mdx index a267cc60f..d2dc49690 100644 --- a/src/upgrade-notes.mdx +++ b/src/upgrade-notes.mdx @@ -129,6 +129,7 @@ defined yet): ```css .your-theme { --of-utrecht-data-list-padding-block-end: 20px; + --of-utrecht-data-list-laptop-display: flex; --utrecht-data-list-item-key-color: var(--of-color-fg); /* or an actual color value */ --utrecht-data-list-item-key-font-size: 1rem; From 5680b0aa5beca98b60076ea7c8a1315cb33bd1cb Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 3 Jan 2024 12:15:04 +0100 Subject: [PATCH 6/7] :green_heart: [#471] Update tests to testing-library * Simplified the tests by using testing-library render * Simplified tests further by parametrizing them * Updated assertion queries to use accessible role queries rather than relying on specific class names --- src/components/FormStepSummary/test.spec.js | 159 +++++++------------- 1 file changed, 55 insertions(+), 104 deletions(-) diff --git a/src/components/FormStepSummary/test.spec.js b/src/components/FormStepSummary/test.spec.js index 1aad69282..1490e9dc7 100644 --- a/src/components/FormStepSummary/test.spec.js +++ b/src/components/FormStepSummary/test.spec.js @@ -1,38 +1,27 @@ +import {render, screen} from '@testing-library/react'; import messagesNL from 'i18n/compiled/nl.json'; import React from 'react'; -import {createRoot} from 'react-dom/client'; -import {act} from 'react-dom/test-utils'; import {IntlProvider} from 'react-intl'; import {MemoryRouter} from 'react-router-dom'; import {testEmptyFields} from './fixtures'; import FormStepSummary, {LabelValueRow} from './index'; -let container = null; -let root = null; -beforeEach(() => { - // setup a DOM element as a render target - container = document.createElement('div'); - document.body.appendChild(container); - root = createRoot(container); -}); - -afterEach(() => { - // cleanup on exiting - act(() => { - root.unmount(); - container.remove(); - root = null; - container = null; - }); -}); - const Wrap = ({children}) => ( {children} ); +// NOTE - ideally, in these tests we would query by the roles that speak more to the +// imagination than 'term' and 'definition', however these are still "too new" and not +// supported yet by testing-library (if they will ever be). +// +// In practice, you should read `*byRole('definition')` as `*byRole('associationlistitemvalue')`, +// and a similar mapping applies between 'term' and 'associationlistitemkey'. +// +// MDN: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/structural_roles#structural_roles_with_html_equivalents + it('Unfilled dates displayed properly', () => { const dateComponent = { key: 'dateOfBirth', @@ -40,12 +29,9 @@ it('Unfilled dates displayed properly', () => { format: 'dd-MM-yyyy', }; - act(() => { - root.render(); - }); + render(); - const value = container.getElementsByClassName('openforms-summary-row__value')[0].textContent; - expect(value).toEqual(''); + expect(screen.getByRole('definition')).toHaveTextContent(''); }); it('Multi-value select field displayed properly', () => { @@ -71,41 +57,23 @@ it('Multi-value select field displayed properly', () => { }, }; - act(() => { - root.render( - - - - ); - }); - - const value = container.getElementsByClassName('openforms-summary-row__value')[0].textContent; - expect(value).toEqual('Dog; Fish'); + render( + + + + ); + + expect(screen.getByRole('definition')).toHaveTextContent('Dog; Fish'); }); -it('Empty fields', () => { - act(() => { - root.render( - - - - ); - }); - - const emptyValues = container.getElementsByClassName('openforms-summary-row__value'); - - for (const emptyValue of emptyValues) { - expect(emptyValue.textContent).toEqual(''); - } +it.each(testEmptyFields)('Empty fields (%s)', dataEntry => { + render( + + + + ); + + expect(screen.getByRole('definition')).toHaveTextContent(''); }); it('Columns without labels are not rendered', () => { @@ -115,13 +83,10 @@ it('Columns without labels are not rendered', () => { columns: [], }; - act(() => { - root.render(); - }); + render(); - const tableRows = container.getElementsByClassName('openforms-summary-row'); - - expect(tableRows.length).toEqual(0); + expect(screen.queryByRole('definition')).not.toBeInTheDocument(); + expect(screen.queryByRole('term')).not.toBeInTheDocument(); }); it('Number fields with zero values are displayed', () => { @@ -130,57 +95,43 @@ it('Number fields with zero values are displayed', () => { type: 'number', multiple: false, }; - - act(() => { - root.render( - - - - ); - }); - - const value = container.getElementsByClassName('openforms-summary-row__value')[0].textContent; - expect(value).toEqual('0'); + render( + + + + ); + expect(screen.getByRole('definition')).toHaveTextContent('0'); }); -it('Number fields with zero values are displayed', () => { +it('Currency fields with zero values are displayed', () => { const numberComponent = { key: 'currencyComponent', type: 'currency', multiple: false, }; + render( + + + + ); - act(() => { - root.render( - - - - ); - }); - - const value = container.getElementsByClassName('openforms-summary-row__value')[0].textContent; - expect(value).toEqual('€ 0,00'); + expect(screen.getByRole('definition')).toHaveTextContent('€ 0,00'); }); -it('Checkboxes are capitalised', () => { - const dateComponent = { +it.each([ + [true, 'Ja'], + [false, 'Nee'], +])("Checkboxes are capitalised (value '%s' -> %s)", (value, text) => { + const checkbox = { key: 'checkbox', type: 'checkbox', }; - act(() => { - root.render( - - - - - ); - }); - - const valueTrue = container.getElementsByClassName('openforms-summary-row__value')[0].textContent; - expect(valueTrue).toEqual('Ja'); - - const valueFalse = container.getElementsByClassName('openforms-summary-row__value')[1] - .textContent; - expect(valueFalse).toEqual('Nee'); + render( + + + + ); + + expect(screen.getByRole('definition')).toHaveTextContent(text); }); From f7576cd002a993e4b6193c46ad3b1a816029cf38 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 4 Jan 2024 15:44:26 +0100 Subject: [PATCH 7/7] :arrow_up: [#471] Bump to latest version of design tokens package --- design-tokens | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/design-tokens b/design-tokens index ce61470d6..760effa3f 160000 --- a/design-tokens +++ b/design-tokens @@ -1 +1 @@ -Subproject commit ce61470d68ef1a6fc365a4b0bc245382b83be4d5 +Subproject commit 760effa3f918f0174eb52c57f8d0b94f7d1ce20a diff --git a/package.json b/package.json index 3109c6841..b99cae5ad 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "@floating-ui/react": "^0.24.2", "@formio/protected-eval": "^1.2.1", "@fortawesome/fontawesome-free": "^6.1.1", - "@open-formulieren/design-tokens": "^0.51.0", + "@open-formulieren/design-tokens": "^0.52.1", "@sentry/react": "^6.13.2", "@sentry/tracing": "^6.13.2", "@trivago/prettier-plugin-sort-imports": "^4.0.0",