diff --git a/design-tokens b/design-tokens index 326390434..760effa3f 160000 --- a/design-tokens +++ b/design-tokens @@ -1 +1 @@ -Subproject commit 326390434b5eb44bf82c3b1d2996b3a1012014eb +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", 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/components/FormStepSummary/index.js b/src/components/FormStepSummary/index.js index bb0021292..26f347b1b 100644 --- a/src/components/FormStepSummary/index.js +++ b/src/components/FormStepSummary/index.js @@ -1,25 +1,31 @@ +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} + -
-
+ + ); }; @@ -42,9 +48,9 @@ const FormStepSummary = ({editUrl, slug, name, data, editStepText = ''}) => { } return ( -
-
-

{name}

+
+
+ {name} {editStepText && ( @@ -54,7 +60,7 @@ const FormStepSummary = ({editUrl, slug, name, data, editStepText = ''}) => { )}
-
+ {data.map((item, index) => ( { component={item.component} /> ))} -
+
); }; 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); }); diff --git a/src/scss/components/_data-list.scss b/src/scss/components/_data-list.scss new file mode 100644 index 000000000..6ce8266b2 --- /dev/null +++ b/src/scss/components/_data-list.scss @@ -0,0 +1,75 @@ +@use 'microscope-sass/lib/bem'; + +@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. + * + * 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); + + // 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') { + 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: var(--of-utrecht-data-list-laptop-display, block); + } + + @include bem.element('item-key') { + font-weight: var(--of-utrecht-data-list-item-key-laptop-font-weight, 700); + } + } + } +} 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/scss/components/_summary.scss b/src/scss/components/_summary.scss index 04a767bef..bee6b3849 100644 --- a/src/scss/components/_summary.scss +++ b/src/scss/components/_summary.scss @@ -1,35 +1,51 @@ @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); + } } } 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/styles.scss b/src/styles.scss index d8dbcbf2b..dad4ec670 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'; @@ -54,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'; diff --git a/src/upgrade-notes.mdx b/src/upgrade-notes.mdx index abd11ec6c..d2dc49690 100644 --- a/src/upgrade-notes.mdx +++ b/src/upgrade-notes.mdx @@ -89,3 +89,64 @@ 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` + +**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; + --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; + --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`