From 1a0b69be58c3e22c23d219ec62b5c3a58cca797f Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 20 Oct 2023 16:48:49 +0200 Subject: [PATCH] :recycle: [#571] Refactor away openforms-form-control This wrapper was applied to the formio component node rendered by the vanilla formio renderer, and copied over (for vertical spacing reasons) to the React implementation of form field components. The UtrechtFormField and UtrechtFormFieldset components already act like a wrapper/container, and we've introduced a separate class name to manage the vertical spacing of components. For interop with formio, we do some CSS/SASS extension gymnastics, which we'll be able to remove once we have our own React-based formio renderer. --- .../appointments/fields/DateSelect.js | 14 +- src/components/forms/Checkbox/Checkbox.js | 46 +++--- src/components/forms/DateField/DateField.js | 36 +++-- .../forms/InputGroup/InputGroup.mdx | 4 +- .../forms/InputGroup/InputGroup.stories.js | 40 +++--- .../forms/NumberField/NumberField.js | 52 ++++--- src/components/forms/RadioField/RadioField.js | 70 +++++----- .../forms/SelectField/SelectField.js | 132 +++++++++--------- src/components/forms/TextField/TextField.js | 40 +++--- src/components/forms/Wrapper.js | 24 ---- src/components/forms/index.js | 1 - src/formio/components/Component.js | 13 ++ src/formio/components/composite.stories.js | 18 +++ src/formio/templates/component.ejs | 7 +- src/formio/templates/field.ejs | 47 +++++-- src/formio/templates/fieldset.ejs | 2 +- src/scss/components/_deprecated-select.scss | 1 + src/scss/components/_file-upload.scss | 1 + src/scss/components/_form-control.scss | 94 ------------- .../components/_form-field-container.scss | 23 +++ src/scss/components/_form-field.scss | 2 - src/scss/components/_formio-component.scss | 55 ++++++++ src/scss/nl-design-system-community.scss | 1 + src/styles.scss | 3 +- 24 files changed, 358 insertions(+), 368 deletions(-) delete mode 100644 src/components/forms/Wrapper.js delete mode 100644 src/scss/components/_form-control.scss create mode 100644 src/scss/components/_form-field-container.scss create mode 100644 src/scss/components/_formio-component.scss diff --git a/src/components/appointments/fields/DateSelect.js b/src/components/appointments/fields/DateSelect.js index 8d9049f7e..e535830a0 100644 --- a/src/components/appointments/fields/DateSelect.js +++ b/src/components/appointments/fields/DateSelect.js @@ -58,14 +58,12 @@ const DateSelect = ({products, onChange}) => { if (!loading && location && availableDates && !availableDates.length) { // TODO: add label? make this a polite warning/error/alert? return ( -
- - - -
+ + + ); } diff --git a/src/components/forms/Checkbox/Checkbox.js b/src/components/forms/Checkbox/Checkbox.js index b9a85f051..48d16ac3c 100644 --- a/src/components/forms/Checkbox/Checkbox.js +++ b/src/components/forms/Checkbox/Checkbox.js @@ -3,7 +3,7 @@ import {Field, useFormikContext} from 'formik'; import PropTypes from 'prop-types'; import React, {useId} from 'react'; -import {HelpText, ValidationErrors, Wrapper} from 'components/forms'; +import {HelpText, ValidationErrors} from 'components/forms'; import {LabelContent} from '../Label'; @@ -24,29 +24,27 @@ const Checkbox = ({ const invalid = touched && !!error; return ( - - - -
- - {label} - -
- {description} - {touched && } - - + + +
+ + {label} + +
+ {description} + {touched && } + ); }; diff --git a/src/components/forms/DateField/DateField.js b/src/components/forms/DateField/DateField.js index bae1f2e5f..47342d467 100644 --- a/src/components/forms/DateField/DateField.js +++ b/src/components/forms/DateField/DateField.js @@ -3,7 +3,7 @@ import {Field, useFormikContext} from 'formik'; import PropTypes from 'prop-types'; import React from 'react'; -import {HelpText, ValidationErrors, Wrapper} from 'components/forms'; +import {HelpText, ValidationErrors} from 'components/forms'; import DateInputGroup from './DateInputGroup'; import DatePicker from './DatePicker'; @@ -66,24 +66,22 @@ const DateField = ({ } return ( - - - - {description} - {touched && } - - + + + {description} + {touched && } + ); }; diff --git a/src/components/forms/InputGroup/InputGroup.mdx b/src/components/forms/InputGroup/InputGroup.mdx index a8a86e865..9a318f8c8 100644 --- a/src/components/forms/InputGroup/InputGroup.mdx +++ b/src/components/forms/InputGroup/InputGroup.mdx @@ -23,8 +23,8 @@ example: The input group is a low level component, intended to replace a regular `Textbox` or `Select` component. -It should be composed with the `Wrapper`, `FormField`, `FormLabel`, `HelpText` and -`ValidationErrors` components. +It should be composed with the `FormField`, `FormLabel`, `HelpText` and `ValidationErrors` +components. ## Props diff --git a/src/components/forms/InputGroup/InputGroup.stories.js b/src/components/forms/InputGroup/InputGroup.stories.js index b26980558..e39b8dc19 100644 --- a/src/components/forms/InputGroup/InputGroup.stories.js +++ b/src/components/forms/InputGroup/InputGroup.stories.js @@ -1,7 +1,7 @@ import {FormField, FormLabel, Textbox} from '@utrecht/component-library-react'; import {Field} from 'formik'; -import {SelectField, TextField, Wrapper} from 'components/forms'; +import {SelectField, TextField} from 'components/forms'; import {FormikDecorator} from 'story-utils/decorators'; import {InputGroup, InputGroupItem} from './InputGroup'; @@ -42,27 +42,25 @@ export const Basic = { export const InLargerForm = { render: args => ( - <> +
- - - - - - Sub 1 - - - - - - Sub 2 - - - - - - + + + + + Sub 1 + + + + + + Sub 2 + + + + + - +
), args: { label: 'Group related inputs', diff --git a/src/components/forms/NumberField/NumberField.js b/src/components/forms/NumberField/NumberField.js index 351ed9c80..879c1a29b 100644 --- a/src/components/forms/NumberField/NumberField.js +++ b/src/components/forms/NumberField/NumberField.js @@ -5,7 +5,7 @@ import React from 'react'; import {useIntl} from 'react-intl'; import {NumericFormat} from 'react-number-format'; -import {HelpText, Label, ValidationErrors, Wrapper} from 'components/forms'; +import {HelpText, Label, ValidationErrors} from 'components/forms'; const getSeparators = locale => { const numberFormat = new Intl.NumberFormat(locale); @@ -48,32 +48,30 @@ const NumberField = ({ }; return ( - - - - - - - {description} - {touched && } - - + + + + + + {description} + {touched && } + ); }; diff --git a/src/components/forms/RadioField/RadioField.js b/src/components/forms/RadioField/RadioField.js index d6ea73a40..8b464b372 100644 --- a/src/components/forms/RadioField/RadioField.js +++ b/src/components/forms/RadioField/RadioField.js @@ -10,7 +10,7 @@ import {Field, useFormikContext} from 'formik'; import PropTypes from 'prop-types'; import React from 'react'; -import {HelpText, ValidationErrors, Wrapper} from 'components/forms'; +import {HelpText, ValidationErrors} from 'components/forms'; import {LabelContent} from 'components/forms/Label'; /** @@ -36,43 +36,41 @@ export const RadioField = ({ const invalid = touched && !!error; const descriptionid = `${id}-description`; return ( - -
- - - {label} - - +
+ + + {label} + + - {options.map(({value: optionValue, label: optionLabel}, index) => ( - - - - - {optionLabel} - - - - ))} + {options.map(({value: optionValue, label: optionLabel}, index) => ( + + + + + {optionLabel} + + + + ))} - {description} - {touched && } -
- + {description} + {touched && } +
); }; diff --git a/src/components/forms/SelectField/SelectField.js b/src/components/forms/SelectField/SelectField.js index 164f05e29..8f6fbb63f 100644 --- a/src/components/forms/SelectField/SelectField.js +++ b/src/components/forms/SelectField/SelectField.js @@ -7,7 +7,7 @@ import React, {useEffect} from 'react'; import {FormattedMessage} from 'react-intl'; import Select from 'react-select'; -import {HelpText, Label, ValidationErrors, Wrapper} from 'components/forms'; +import {HelpText, Label, ValidationErrors} from 'components/forms'; const SelectField = ({ name, @@ -75,72 +75,70 @@ const SelectField = ({ }; return ( - - - - - classNames('utrecht-select', 'utrecht-select--openforms', { - 'utrecht-select--focus': state.isFocused, - 'utrecht-select--focus-visible': state.isFocused, - 'utrecht-select--disabled': disabled, - 'utrecht-select--invalid': invalid, - 'utrecht-select--required': isRequired, - }), - menu: () => 'rs-menu', - option: state => - classNames('rs-menu__option', { - 'rs-menu__option--focus': state.isFocused, - 'rs-menu__option--visible-focus': state.isFocused, - }), - singleValue: () => 'rs-value rs-value--single', - multiValue: () => 'rs-value rs-value--multi', - noOptionsMessage: () => 'rs-no-options', - }} - styles={{ - control: (baseStyles, state) => { - return omit(baseStyles, 'outline'); - }, - }} - unstyled - options={options} - getOptionValue={opt => opt[valueProperty]} - isDisabled={disabled} - loadingMessage={() => ( - - )} - noOptionsMessage={() => ( - - )} - onKeyDown={handleKeyDown} - {...props} - onChange={newValue => { - const isSingle = !Array.isArray(newValue); - const normalized = isSingle ? [newValue] : newValue; - const rawValues = normalized.map(val => val?.[valueProperty] ?? null); - const rawValue = isSingle ? rawValues[0] : rawValues; - setValue(rawValue, validateOnChange); - onChange?.({target: {name, value: rawValue}}); - }} - value={value} - onBlur={() => setTouched(true)} - /> - {description} - {touched && } - - + + + + classNames('utrecht-select', 'utrecht-select--openforms', { + 'utrecht-select--focus': state.isFocused, + 'utrecht-select--focus-visible': state.isFocused, + 'utrecht-select--disabled': disabled, + 'utrecht-select--invalid': invalid, + 'utrecht-select--required': isRequired, + }), + menu: () => 'rs-menu', + option: state => + classNames('rs-menu__option', { + 'rs-menu__option--focus': state.isFocused, + 'rs-menu__option--visible-focus': state.isFocused, + }), + singleValue: () => 'rs-value rs-value--single', + multiValue: () => 'rs-value rs-value--multi', + noOptionsMessage: () => 'rs-no-options', + }} + styles={{ + control: (baseStyles, state) => { + return omit(baseStyles, 'outline'); + }, + }} + unstyled + options={options} + getOptionValue={opt => opt[valueProperty]} + isDisabled={disabled} + loadingMessage={() => ( + + )} + noOptionsMessage={() => ( + + )} + onKeyDown={handleKeyDown} + {...props} + onChange={newValue => { + const isSingle = !Array.isArray(newValue); + const normalized = isSingle ? [newValue] : newValue; + const rawValues = normalized.map(val => val?.[valueProperty] ?? null); + const rawValue = isSingle ? rawValues[0] : rawValues; + setValue(rawValue, validateOnChange); + onChange?.({target: {name, value: rawValue}}); + }} + value={value} + onBlur={() => setTouched(true)} + /> + {description} + {touched && } + ); }; diff --git a/src/components/forms/TextField/TextField.js b/src/components/forms/TextField/TextField.js index 21ffa9a17..6a0aaf248 100644 --- a/src/components/forms/TextField/TextField.js +++ b/src/components/forms/TextField/TextField.js @@ -3,7 +3,7 @@ import {Field, useFormikContext} from 'formik'; import PropTypes from 'prop-types'; import React from 'react'; -import {HelpText, Label, ValidationErrors, Wrapper} from 'components/forms'; +import {HelpText, Label, ValidationErrors} from 'components/forms'; export const TextField = ({ name, @@ -21,26 +21,24 @@ export const TextField = ({ const {error, touched} = getFieldMeta(name); const invalid = touched && !!error; return ( - - - - - - - {description} - {touched && } - - + + + + + + {description} + {touched && } + ); }; diff --git a/src/components/forms/Wrapper.js b/src/components/forms/Wrapper.js deleted file mode 100644 index 3221018d3..000000000 --- a/src/components/forms/Wrapper.js +++ /dev/null @@ -1,24 +0,0 @@ -import PropTypes from 'prop-types'; -import React, {useContext} from 'react'; - -import {ConfigContext} from 'Context'; -import {getBEMClassName} from 'utils'; - -const Wrapper = ({children}) => { - const {requiredFieldsWithAsterisk} = useContext(ConfigContext); - return ( -
- {children} -
- ); -}; - -Wrapper.propTypes = { - children: PropTypes.node, -}; - -export default Wrapper; diff --git a/src/components/forms/index.js b/src/components/forms/index.js index 06e772329..d92e08a8a 100644 --- a/src/components/forms/index.js +++ b/src/components/forms/index.js @@ -13,4 +13,3 @@ export {default as Label} from './Label'; export {default as HelpText} from './HelpText'; export {InputGroup, InputGroupItem} from './InputGroup'; export {default as ValidationErrors} from './ValidationErrors'; -export {default as Wrapper} from './Wrapper'; diff --git a/src/formio/components/Component.js b/src/formio/components/Component.js index d29083fbd..11d2fecd8 100644 --- a/src/formio/components/Component.js +++ b/src/formio/components/Component.js @@ -5,6 +5,19 @@ import {applyPrefix} from '../utils'; const FormioComponent = Formio.Components.components.component; +/** + * Build class names for 'component' component. + * + * @deprecated: these class names are no longer rendered in the templates and the CSS + * overrides are removed. + * + * @todo: check impact of --$type variants in styles + * @todo: check impact of --multiple in styles + * @todo: check if any other components than content use customClass + * @todo: check how the --required modifier is/was used (asterisk?) & take into account + * the string 'true' values! + * @todo: check impact of --hidden modifier + */ function getClassName() { return classNames( applyPrefix('form-control'), diff --git a/src/formio/components/composite.stories.js b/src/formio/components/composite.stories.js index fb58b6a84..1342dd3b2 100644 --- a/src/formio/components/composite.stories.js +++ b/src/formio/components/composite.stories.js @@ -40,6 +40,24 @@ export default { html: '

Some WYSIWYG content

', customClass: 'info', }, + { + type: 'fieldset', + key: 'fieldset', + label: 'Fieldset label', + hideHeader: true, + components: [ + { + type: 'checkbox', + key: 'checkbox', + label: 'Checkbox', + }, + { + type: 'textfield', + key: 'nestedTextfield', + label: 'Nested text field', + }, + ], + }, { type: 'textfield', key: 'hiddenTextfield', diff --git a/src/formio/templates/component.ejs b/src/formio/templates/component.ejs index 10234e08b..2824f223b 100644 --- a/src/formio/templates/component.ejs +++ b/src/formio/templates/component.ejs @@ -1,10 +1,5 @@ -
+
{% if (ctx.visible) { %} {{ctx.children}} {% } %} -
diff --git a/src/formio/templates/field.ejs b/src/formio/templates/field.ejs index 09132b1cf..e2d21aa4c 100644 --- a/src/formio/templates/field.ejs +++ b/src/formio/templates/field.ejs @@ -1,5 +1,6 @@ + {% if (ctx.component.type === 'radio' || ctx.component.type === 'selectboxes') { %} -
+
{% } %} + +
+ +{% } else if (ctx.component.type === 'form') { %} + + {{ctx.element}} + + {% } else { %}
- {% if (!ctx.label.hidden && ctx.label.labelPosition !== 'bottom') { %} - {{ ctx.labelMarkup }} - {% } %} + {% if (!ctx.label.hidden && ctx.label.labelPosition !== 'bottom') { %} + {{ ctx.labelMarkup }} + {% } %} - {% if (ctx.label.hidden && ctx.label.className && ctx.component.validate.required) { %} - - {% } %} + {% if (ctx.label.hidden && ctx.label.className && ctx.component.validate.required) { %} + + {% } %} - {{ctx.element}} + {{ctx.element}} + + {% if (!ctx.label.hidden && ctx.label.labelPosition === 'bottom') { %} + {{ ctx.labelMarkup }} + {% } %} - {% if (!ctx.label.hidden && ctx.label.labelPosition === 'bottom') { %} - {{ ctx.labelMarkup }} - {% } %} + {% if (ctx.component.description) { %} +
{{ctx.component.description}}
+ {% } %} - {% if (ctx.component.description) { %} -
{{ctx.component.description}}
- {% } %} +
{% } %} diff --git a/src/formio/templates/fieldset.ejs b/src/formio/templates/fieldset.ejs index eb7902216..30dcd32e5 100644 --- a/src/formio/templates/fieldset.ejs +++ b/src/formio/templates/fieldset.ejs @@ -8,7 +8,7 @@ {% } %} {% if (!ctx.collapsed) { %} -
+
{{ctx.children}}
{% } %} diff --git a/src/scss/components/_deprecated-select.scss b/src/scss/components/_deprecated-select.scss index 84e3d97be..bbd6efaf2 100644 --- a/src/scss/components/_deprecated-select.scss +++ b/src/scss/components/_deprecated-select.scss @@ -11,6 +11,7 @@ $select-background-color: var(--of-select-background-color, $color-white); // Styling for the choicejs widget for the select component // Overwriting styles found here: https://github.com/Choices-js/Choices/blob/master/src/styles/choices.scss +// FIXME: check impact of form-control removal .#{prefix(form-control--select)} { $control-height: $grid-row-height; $list-hpadding: $grid-margin-2; diff --git a/src/scss/components/_file-upload.scss b/src/scss/components/_file-upload.scss index 5ea2aa93c..3644bd5ed 100644 --- a/src/scss/components/_file-upload.scss +++ b/src/scss/components/_file-upload.scss @@ -11,6 +11,7 @@ we don't have strict BEM naming here. @import '../mixins/prefix'; @import '../mixins/bootstrap'; +// FIXME: these class names are gone -> find different selector .#{prefix(form-control--file)} { @include body; @include anchor.extend-utrecht-link; diff --git a/src/scss/components/_form-control.scss b/src/scss/components/_form-control.scss deleted file mode 100644 index c53d5ed76..000000000 --- a/src/scss/components/_form-control.scss +++ /dev/null @@ -1,94 +0,0 @@ -@use 'microscope-sass/lib/bem'; - -@import '~microscope-sass/lib/typography'; - -@import '../mixins/prefix'; - -.#{prefix(form-control)} { - position: relative; - - // add vertical space/padding on top for elements except the first child in a container - // This is sort of the inverse of @include margin(auto) which operates on the last-child. - &:not(:first-child) { - @include responsive( - $properties: padding-top, - $value-mobile: $grid-margin-3, - $value-tablet: $grid-margin-4 - ); - } - - @include bem.modifier('hidden') { - display: none; - } - - &.error { - @include border(left, var(--of-color-danger), 4px); - padding-left: $grid-margin-3; - } - - &.warning { - @include border(left, var(--of-color-warning), 4px); - padding-left: $grid-margin-3; - } - - &.success { - @include border(left, var(--of-color-success), 4px); - padding-left: $grid-margin-3; - } - - &.info { - @include border(left, var(--of-color-info), 4px); - padding-left: $grid-margin-3; - } - - &.formio-error-wrapper, - &.has-error { - @include border(left, var(--of-color-danger), 4px); - padding: $grid-margin-3; - background-color: var(--of-alert-error-bg); - } - - .#{prefix(prefix)} { - @include margin($value-mobile: $typography-margin-list); - } - - .#{prefix(body)}, - .#{prefix(suffix)} { - @include margin(true, margin-top, $value-mobile: $typography-margin-list); - } - - .#{prefix(body)} { - display: block; - - &--inline { - display: inline-block; - margin-block-start: 0; - } - } - - &--no-asterisks { - .required-field:after, - .utrecht-form-label.utrecht-form-label--openforms-required:after { - // Override the asterisk - content: '' !important; - } - } - - .group { - display: flex; - align-items: center; - } - - // Overwriting Formio style - .control-label--hidden { - position: static !important; - } - - &--editgrid { - & > label { - // Only the label of the whole repeating group has h3 style. Not the label of each field - // within a repeat of the group. - @include h3; - } - } -} diff --git a/src/scss/components/_form-field-container.scss b/src/scss/components/_form-field-container.scss new file mode 100644 index 000000000..662d26b48 --- /dev/null +++ b/src/scss/components/_form-field-container.scss @@ -0,0 +1,23 @@ +@use 'microscope-sass/lib/bem'; + +/** + * NL DS is discussing spacing between form fields, for the time being we need to + * solve this ourselves. + * + * The canonical component for this is "openforms-form-field-container", but note that + * we also target some other classes to share these styles because of formio reasons. + * + * This is the path of least resistance to achieve our goals and rip out form-control + * components. + */ +.openforms-form-field-container { + display: flex; + flex-direction: column; + gap: var(--of-form-field-container-gap, 24px); // 24px fallback for backwards compatibilty +} + +// class set on the top-level formio form which renders its 'components' ( +// ``configuration.components`` in JSON) +.formio-form { + @extend .openforms-form-field-container; +} diff --git a/src/scss/components/_form-field.scss b/src/scss/components/_form-field.scss index 8178481ca..ee030dc04 100644 --- a/src/scss/components/_form-field.scss +++ b/src/scss/components/_form-field.scss @@ -1,7 +1,5 @@ @use 'microscope-sass/lib/bem'; -@import '@utrecht/components/dist/form-field/css/index.css'; - // Overrides of the utrecht form field styles for our own theme .openforms-theme { .utrecht-form-field { diff --git a/src/scss/components/_formio-component.scss b/src/scss/components/_formio-component.scss new file mode 100644 index 000000000..0eb7c16ff --- /dev/null +++ b/src/scss/components/_formio-component.scss @@ -0,0 +1,55 @@ +@use 'microscope-sass/lib/bem'; +@use '@utrecht/components/form-field/css/mixin' as form-field; +@use '@utrecht/components/form-fieldset/css/mixin' as form-fieldset; + +@import '~microscope-sass/lib/typography'; + +@import '../mixins/prefix'; + +// These styles are required because form.io doesn't re-render with new classnames, +// instead it looks up the component ref and then adds the validation error classnames. +// The (direct) child inside this node are the utrecht components that need to gain +// the validation-error styles, so we use sass extend and include to achieve that. +[ref='component'] { + &.has-error, + &.formio-error-wrapper { + > .utrecht-form-field { + @extend .utrecht-form-field--invalid; + @include form-field.utrecht-form-field--invalid; + } + + > .utrecht-form-fieldset { + @extend .utrecht-form-fieldset--invalid; + @include form-fieldset.utrecht-form-fieldset--invalid; + } + } +} + +.#{prefix(form-control)} { + .#{prefix(body)} { + display: block; + + &--inline { + display: inline-block; + margin-block-start: 0; + } + } + + &--no-asterisks { + .required-field:after, + .utrecht-form-label.utrecht-form-label--openforms-required:after { + // Override the asterisk + content: '' !important; + } + } + + .group { + display: flex; + align-items: center; + } + + // Overwriting Formio style + .control-label--hidden { + position: static !important; + } +} diff --git a/src/scss/nl-design-system-community.scss b/src/scss/nl-design-system-community.scss index f85c20328..086e764ae 100644 --- a/src/scss/nl-design-system-community.scss +++ b/src/scss/nl-design-system-community.scss @@ -1,5 +1,6 @@ @import '@utrecht/components/dist/document/css/index.css'; @import '@utrecht/components/dist/img/css/index.css'; @import '@utrecht/components/dist/paragraph/css/index.css'; +@import '@utrecht/components/dist/form-field/css/index.css'; @import '@utrecht/components/dist/form-fieldset/css/index.css'; @import '@utrecht/components/dist/radio-button/css/index.css'; diff --git a/src/styles.scss b/src/styles.scss index 12d60dbf3..1bc9e1e28 100644 --- a/src/styles.scss +++ b/src/styles.scss @@ -27,10 +27,11 @@ @import './scss/components/card'; @import './scss/components/content'; @import './scss/components/errors'; -@import './scss/components/form-control'; +@import './scss/components/formio-component'; @import './scss/components/help-text'; @import './scss/components/image'; @import './scss/components/form-field'; +@import './scss/components/form-field-container'; @import './scss/components/radio-field'; // must come after form-field due to some OF-specific overrides @import './scss/components/input'; @import './scss/components/label';