fix: Error UI improvements for nested lists/objects (#3726)
This commit is contained in:
@ -349,7 +349,7 @@ export function changeDraftField(
|
||||
|
||||
export function changeDraftFieldValidation(
|
||||
uniquefieldId: string,
|
||||
errors: { type: string; message: string }[],
|
||||
errors: { type: string; parentIds: string[]; message: string }[],
|
||||
) {
|
||||
return {
|
||||
type: DRAFT_VALIDATION_ERRORS,
|
||||
|
@ -115,6 +115,11 @@ class EditorControl extends React.Component {
|
||||
t: PropTypes.func.isRequired,
|
||||
isEditorComponent: PropTypes.bool,
|
||||
isNewEditorComponent: PropTypes.bool,
|
||||
parentIds: PropTypes.arrayOf(PropTypes.string),
|
||||
};
|
||||
|
||||
static defaultProps = {
|
||||
parentIds: [],
|
||||
};
|
||||
|
||||
state = {
|
||||
@ -123,6 +128,17 @@ class EditorControl extends React.Component {
|
||||
|
||||
uniqueFieldId = uniqueId(`${this.props.field.get('name')}-field-`);
|
||||
|
||||
isAncestorOfFieldError = () => {
|
||||
const { fieldsErrors } = this.props;
|
||||
|
||||
if (fieldsErrors && fieldsErrors.size > 0) {
|
||||
return Object.values(fieldsErrors.toJS()).some(arr =>
|
||||
arr.some(err => err.parentIds && err.parentIds.includes(this.uniqueFieldId)),
|
||||
);
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
render() {
|
||||
const {
|
||||
value,
|
||||
@ -153,6 +169,7 @@ class EditorControl extends React.Component {
|
||||
isSelected,
|
||||
isEditorComponent,
|
||||
isNewEditorComponent,
|
||||
parentIds,
|
||||
t,
|
||||
} = this.props;
|
||||
|
||||
@ -164,6 +181,9 @@ class EditorControl extends React.Component {
|
||||
const onValidateObject = onValidate;
|
||||
const metadata = fieldsMetaData && fieldsMetaData.get(fieldName);
|
||||
const errors = fieldsErrors && fieldsErrors.get(this.uniqueFieldId);
|
||||
const childErrors = this.isAncestorOfFieldError();
|
||||
const hasErrors = !!errors || childErrors;
|
||||
|
||||
return (
|
||||
<ClassNames>
|
||||
{({ css, cx }) => (
|
||||
@ -184,7 +204,7 @@ class EditorControl extends React.Component {
|
||||
)}
|
||||
<FieldLabel
|
||||
isActive={isSelected || this.state.styleActive}
|
||||
hasErrors={!!errors}
|
||||
hasErrors={hasErrors}
|
||||
htmlFor={this.uniqueFieldId}
|
||||
>
|
||||
{`${field.get('label', field.get('name'))}${
|
||||
@ -204,7 +224,7 @@ class EditorControl extends React.Component {
|
||||
{
|
||||
[css`
|
||||
${styleStrings.widgetError};
|
||||
`]: !!errors,
|
||||
`]: hasErrors,
|
||||
},
|
||||
)}
|
||||
classNameWidget={css`
|
||||
@ -255,10 +275,11 @@ class EditorControl extends React.Component {
|
||||
onValidateObject={onValidateObject}
|
||||
isEditorComponent={isEditorComponent}
|
||||
isNewEditorComponent={isNewEditorComponent}
|
||||
parentIds={parentIds}
|
||||
t={t}
|
||||
/>
|
||||
{fieldHint && (
|
||||
<ControlHint active={isSelected || this.state.styleActive} error={!!errors}>
|
||||
<ControlHint active={isSelected || this.state.styleActive} error={hasErrors}>
|
||||
{fieldHint}
|
||||
</ControlHint>
|
||||
)}
|
||||
|
@ -118,11 +118,12 @@ export default class Widget extends Component {
|
||||
};
|
||||
|
||||
validatePresence = (field, value) => {
|
||||
const t = this.props.t;
|
||||
const { t, parentIds } = this.props;
|
||||
const isRequired = field.get('required', true);
|
||||
if (isRequired && isEmpty(value)) {
|
||||
const error = {
|
||||
type: ValidationErrorTypes.PRESENCE,
|
||||
parentIds,
|
||||
message: t('editor.editorControlPane.widget.required', {
|
||||
fieldLabel: field.get('label', field.get('name')),
|
||||
}),
|
||||
@ -134,7 +135,7 @@ export default class Widget extends Component {
|
||||
};
|
||||
|
||||
validatePattern = (field, value) => {
|
||||
const t = this.props.t;
|
||||
const { t, parentIds } = this.props;
|
||||
const pattern = field.get('pattern', false);
|
||||
|
||||
if (isEmpty(value)) {
|
||||
@ -144,6 +145,7 @@ export default class Widget extends Component {
|
||||
if (pattern && !RegExp(pattern.first()).test(value)) {
|
||||
const error = {
|
||||
type: ValidationErrorTypes.PATTERN,
|
||||
parentIds,
|
||||
message: t('editor.editorControlPane.widget.regexPattern', {
|
||||
fieldLabel: field.get('label', field.get('name')),
|
||||
pattern: pattern.last(),
|
||||
@ -157,7 +159,7 @@ export default class Widget extends Component {
|
||||
};
|
||||
|
||||
validateWrappedControl = field => {
|
||||
const t = this.props.t;
|
||||
const { t, parentIds } = this.props;
|
||||
if (typeof this.wrappedControlValid !== 'function') {
|
||||
throw new Error(oneLine`
|
||||
this.wrappedControlValid is not a function. Are you sure widget
|
||||
@ -188,6 +190,7 @@ export default class Widget extends Component {
|
||||
|
||||
const error = {
|
||||
type: ValidationErrorTypes.CUSTOM,
|
||||
parentIds,
|
||||
message: t('editor.editorControlPane.widget.processing', {
|
||||
fieldLabel: field.get('label', field.get('name')),
|
||||
}),
|
||||
@ -257,6 +260,7 @@ export default class Widget extends Component {
|
||||
controlRef,
|
||||
isEditorComponent,
|
||||
isNewEditorComponent,
|
||||
parentIds,
|
||||
t,
|
||||
} = this.props;
|
||||
return React.createElement(controlComponent, {
|
||||
@ -301,6 +305,7 @@ export default class Widget extends Component {
|
||||
isNewEditorComponent,
|
||||
fieldsErrors,
|
||||
controlRef,
|
||||
parentIds,
|
||||
t,
|
||||
});
|
||||
}
|
||||
|
@ -110,6 +110,7 @@ export default class ListControl extends React.Component {
|
||||
|
||||
static defaultProps = {
|
||||
value: List(),
|
||||
parentIds: [],
|
||||
};
|
||||
|
||||
constructor(props) {
|
||||
@ -410,6 +411,15 @@ export default class ListControl extends React.Component {
|
||||
this.validations = this.validations.filter(item => updatedKeys.includes(item.key));
|
||||
};
|
||||
|
||||
hasError = index => {
|
||||
const { fieldsErrors } = this.props;
|
||||
if (fieldsErrors && fieldsErrors.size > 0) {
|
||||
return Object.values(fieldsErrors.toJS()).some(arr =>
|
||||
arr.some(err => err.parentIds && err.parentIds.includes(this.state.keys[index])),
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
// eslint-disable-next-line react/display-name
|
||||
renderItem = (item, index) => {
|
||||
const {
|
||||
@ -421,12 +431,15 @@ export default class ListControl extends React.Component {
|
||||
fieldsErrors,
|
||||
controlRef,
|
||||
resolveWidget,
|
||||
parentIds,
|
||||
forID,
|
||||
} = this.props;
|
||||
|
||||
const { itemsCollapsed, keys } = this.state;
|
||||
const collapsed = itemsCollapsed[index];
|
||||
const key = keys[index];
|
||||
let field = this.props.field;
|
||||
const hasError = this.hasError(index);
|
||||
|
||||
if (this.getValueType() === valueTypes.MIXED) {
|
||||
field = getTypedFieldForValue(field, item);
|
||||
@ -448,7 +461,9 @@ export default class ListControl extends React.Component {
|
||||
dragHandleHOC={SortableHandle}
|
||||
data-testid={`styled-list-item-top-bar-${key}`}
|
||||
/>
|
||||
<NestedObjectLabel collapsed={collapsed}>{this.objectLabel(item)}</NestedObjectLabel>
|
||||
<NestedObjectLabel collapsed={collapsed} error={hasError}>
|
||||
{this.objectLabel(item)}
|
||||
</NestedObjectLabel>
|
||||
<ClassNames>
|
||||
{({ css, cx }) => (
|
||||
<ObjectControl
|
||||
@ -472,6 +487,8 @@ export default class ListControl extends React.Component {
|
||||
validationKey={key}
|
||||
collapsed={collapsed}
|
||||
data-testid={`object-control-${key}`}
|
||||
hasError={hasError}
|
||||
parentIds={[...parentIds, forID, key]}
|
||||
/>
|
||||
)}
|
||||
</ClassNames>
|
||||
|
@ -52,6 +52,7 @@ describe('ListControl', () => {
|
||||
entry: fromJS({
|
||||
path: 'posts/index.md',
|
||||
}),
|
||||
forID: 'forID',
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
|
@ -148,6 +148,7 @@ exports[`ListControl should add to list when add button is clicked 1`] = `
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-18"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -218,6 +219,7 @@ exports[`ListControl should add to list when add button is clicked 1`] = `
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,0"
|
||||
validationkey="0"
|
||||
value="Map {}"
|
||||
/>
|
||||
@ -375,6 +377,7 @@ exports[`ListControl should remove from list when remove button is clicked 1`] =
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-22"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -445,6 +448,7 @@ exports[`ListControl should remove from list when remove button is clicked 1`] =
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"minimize_collapsed\\": true, \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,0"
|
||||
validationkey="0"
|
||||
value="Map { \\"string\\": \\"item 1\\" }"
|
||||
/>
|
||||
@ -473,6 +477,7 @@ exports[`ListControl should remove from list when remove button is clicked 1`] =
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"minimize_collapsed\\": true, \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,1"
|
||||
validationkey="1"
|
||||
value="Map { \\"string\\": \\"item 2\\" }"
|
||||
/>
|
||||
@ -631,6 +636,7 @@ exports[`ListControl should remove from list when remove button is clicked 2`] =
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-18"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -701,6 +707,7 @@ exports[`ListControl should remove from list when remove button is clicked 2`] =
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"minimize_collapsed\\": true, \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,1"
|
||||
validationkey="1"
|
||||
value="Map { \\"string\\": \\"item 2\\" }"
|
||||
/>
|
||||
@ -714,6 +721,7 @@ exports[`ListControl should render empty list 1`] = `
|
||||
<DocumentFragment>
|
||||
<input
|
||||
class="classNameWrapper"
|
||||
id="forID"
|
||||
type="text"
|
||||
value=""
|
||||
/>
|
||||
@ -868,6 +876,7 @@ exports[`ListControl should render list with fields with collapse = "false" and
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-22"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -938,6 +947,7 @@ exports[`ListControl should render list with fields with collapse = "false" and
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,0"
|
||||
validationkey="0"
|
||||
value="Map { \\"string\\": \\"item 1\\" }"
|
||||
/>
|
||||
@ -966,6 +976,7 @@ exports[`ListControl should render list with fields with collapse = "false" and
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,1"
|
||||
validationkey="1"
|
||||
value="Map { \\"string\\": \\"item 2\\" }"
|
||||
/>
|
||||
@ -1123,6 +1134,7 @@ exports[`ListControl should render list with fields with collapse = "false" and
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-22"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -1193,6 +1205,7 @@ exports[`ListControl should render list with fields with collapse = "false" and
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"minimize_collapsed\\": true, \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,0"
|
||||
validationkey="0"
|
||||
value="Map { \\"string\\": \\"item 1\\" }"
|
||||
/>
|
||||
@ -1221,6 +1234,7 @@ exports[`ListControl should render list with fields with collapse = "false" and
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"minimize_collapsed\\": true, \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,1"
|
||||
validationkey="1"
|
||||
value="Map { \\"string\\": \\"item 2\\" }"
|
||||
/>
|
||||
@ -1379,6 +1393,7 @@ exports[`ListControl should render list with fields with default collapse ("true
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-22"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -1449,6 +1464,7 @@ exports[`ListControl should render list with fields with default collapse ("true
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,0"
|
||||
validationkey="0"
|
||||
value="Map { \\"string\\": \\"item 1\\" }"
|
||||
/>
|
||||
@ -1477,6 +1493,7 @@ exports[`ListControl should render list with fields with default collapse ("true
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"fields\\": List [ Map { \\"label\\": \\"String\\", \\"name\\": \\"string\\", \\"widget\\": \\"string\\" } ] }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,1"
|
||||
validationkey="1"
|
||||
value="Map { \\"string\\": \\"item 2\\" }"
|
||||
/>
|
||||
@ -1617,6 +1634,7 @@ exports[`ListControl should render list with fields with default collapse ("true
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-14"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -1815,6 +1833,7 @@ exports[`ListControl should render list with nested object 1`] = `
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-22"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -1885,6 +1904,7 @@ exports[`ListControl should render list with nested object 1`] = `
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"field\\": Map { \\"name\\": \\"object\\", \\"widget\\": \\"object\\", \\"label\\": \\"Object\\", \\"fields\\": List [ Map { \\"name\\": \\"title\\", \\"widget\\": \\"string\\", \\"label\\": \\"Title\\" } ] } }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,0"
|
||||
validationkey="0"
|
||||
value="Map { \\"object\\": Map { \\"title\\": \\"item 1\\" } }"
|
||||
/>
|
||||
@ -1913,6 +1933,7 @@ exports[`ListControl should render list with nested object 1`] = `
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"field\\": Map { \\"name\\": \\"object\\", \\"widget\\": \\"object\\", \\"label\\": \\"Object\\", \\"fields\\": List [ Map { \\"name\\": \\"title\\", \\"widget\\": \\"string\\", \\"label\\": \\"Title\\" } ] } }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,1"
|
||||
validationkey="1"
|
||||
value="Map { \\"object\\": Map { \\"title\\": \\"item 2\\" } }"
|
||||
/>
|
||||
@ -2070,6 +2091,7 @@ exports[`ListControl should render list with nested object with collapse = false
|
||||
|
||||
<div
|
||||
class="classNameWrapper emotion-22"
|
||||
id="forID"
|
||||
>
|
||||
<div
|
||||
class="emotion-12 emotion-13"
|
||||
@ -2140,6 +2162,7 @@ exports[`ListControl should render list with nested object with collapse = false
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"field\\": Map { \\"name\\": \\"object\\", \\"widget\\": \\"object\\", \\"label\\": \\"Object\\", \\"fields\\": List [ Map { \\"name\\": \\"title\\", \\"widget\\": \\"string\\", \\"label\\": \\"Title\\" } ] } }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,0"
|
||||
validationkey="0"
|
||||
value="Map { \\"object\\": Map { \\"title\\": \\"item 1\\" } }"
|
||||
/>
|
||||
@ -2168,6 +2191,7 @@ exports[`ListControl should render list with nested object with collapse = false
|
||||
field="Map { \\"name\\": \\"list\\", \\"label\\": \\"List\\", \\"collapsed\\": false, \\"field\\": Map { \\"name\\": \\"object\\", \\"widget\\": \\"object\\", \\"label\\": \\"Object\\", \\"fields\\": List [ Map { \\"name\\": \\"title\\", \\"widget\\": \\"string\\", \\"label\\": \\"Title\\" } ] } }"
|
||||
fieldserrors="Map {}"
|
||||
forlist="true"
|
||||
parentids="forID,1"
|
||||
validationkey="1"
|
||||
value="Map { \\"object\\": Map { \\"title\\": \\"item 2\\" } }"
|
||||
/>
|
||||
@ -2181,6 +2205,7 @@ exports[`ListControl should render list with string array 1`] = `
|
||||
<DocumentFragment>
|
||||
<input
|
||||
class="classNameWrapper"
|
||||
id="forID"
|
||||
type="text"
|
||||
value="item 1, item 2"
|
||||
/>
|
||||
|
@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
|
||||
import ImmutablePropTypes from 'react-immutable-proptypes';
|
||||
import { ClassNames } from '@emotion/core';
|
||||
import { Map, List } from 'immutable';
|
||||
import { ObjectWidgetTopBar, lengths } from 'netlify-cms-ui-default';
|
||||
import { ObjectWidgetTopBar, lengths, colors } from 'netlify-cms-ui-default';
|
||||
|
||||
const styleStrings = {
|
||||
nestedObjectControl: `
|
||||
@ -36,6 +36,7 @@ export default class ObjectControl extends React.Component {
|
||||
resolveWidget: PropTypes.func.isRequired,
|
||||
clearFieldErrors: PropTypes.func.isRequired,
|
||||
fieldsErrors: ImmutablePropTypes.map.isRequired,
|
||||
hasError: PropTypes.bool,
|
||||
};
|
||||
|
||||
static defaultProps = {
|
||||
@ -79,6 +80,7 @@ export default class ObjectControl extends React.Component {
|
||||
fieldsErrors,
|
||||
editorControl: EditorControl,
|
||||
controlRef,
|
||||
parentIds,
|
||||
} = this.props;
|
||||
|
||||
if (field.get('widget') === 'hidden') {
|
||||
@ -99,6 +101,7 @@ export default class ObjectControl extends React.Component {
|
||||
onValidate={onValidateObject}
|
||||
processControlRef={controlRef && controlRef.bind(this)}
|
||||
controlRef={controlRef}
|
||||
parentIds={parentIds}
|
||||
/>
|
||||
);
|
||||
}
|
||||
@ -115,7 +118,7 @@ export default class ObjectControl extends React.Component {
|
||||
};
|
||||
|
||||
render() {
|
||||
const { field, forID, classNameWrapper, forList } = this.props;
|
||||
const { field, forID, classNameWrapper, forList, hasError } = this.props;
|
||||
const collapsed = forList ? this.props.collapsed : this.state.collapsed;
|
||||
const multiFields = field.get('fields');
|
||||
const singleField = field.get('field');
|
||||
@ -136,6 +139,11 @@ export default class ObjectControl extends React.Component {
|
||||
${styleStrings.nestedObjectControl}
|
||||
`]: forList,
|
||||
},
|
||||
{
|
||||
[css`
|
||||
border-color: ${colors.textFieldBorder};
|
||||
`]: forList ? !hasError : false,
|
||||
},
|
||||
)}
|
||||
>
|
||||
{forList ? null : (
|
||||
|
Reference in New Issue
Block a user