Skip to content

Commit

Permalink
FCT 1207-1224: unique id's for error and warning containers in all `-…
Browse files Browse the repository at this point in the history
…Field` components (#2950)

* feat(unique error ids): update FieldErrors and FieldWarnings to render the passed id on the wrapper container isntead of on the container for each message so that all errors/warnings can be referred to by aria-errormessage using a single id, update all -Field components to build the id's for their error and warning containers based on their unique id, update tests

* fix(async field tests): use async tests for async fields to get rid of act warning

* fix(changeset): version as patch instead of minor release
  • Loading branch information
ByronDWall authored Oct 10, 2024
1 parent 4c29416 commit c41a012
Show file tree
Hide file tree
Showing 39 changed files with 604 additions and 128 deletions.
23 changes: 23 additions & 0 deletions .changeset/big-mails-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
'@commercetools-uikit/localized-multiline-text-field': patch
'@commercetools-uikit/async-creatable-select-field': patch
'@commercetools-uikit/creatable-select-field': patch
'@commercetools-uikit/localized-text-field': patch
'@commercetools-uikit/multiline-text-field': patch
'@commercetools-uikit/search-select-field': patch
'@commercetools-uikit/async-select-field': patch
'@commercetools-uikit/date-range-field': patch
'@commercetools-uikit/date-time-field': patch
'@commercetools-uikit/password-field': patch
'@commercetools-uikit/number-field': patch
'@commercetools-uikit/select-field': patch
'@commercetools-uikit/money-field': patch
'@commercetools-uikit/radio-field': patch
'@commercetools-uikit/date-field': patch
'@commercetools-uikit/text-field': patch
'@commercetools-uikit/time-field': patch
'@commercetools-uikit/field-warnings': patch
'@commercetools-uikit/field-errors': patch
---

Ensure that `FieldWarnings` and `FieldErrors` display a single id by adding id to wrapper instead of to each error or warning, Ensure all `-Field` components assign unique id's to their error and warning message containers
12 changes: 12 additions & 0 deletions packages/components/field-errors/src/field-errors.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ describe('errorTypes', () => {
expect(screen.getByText(/field is required/i)).toBeInTheDocument();
});

it('should set the id as an attribute of the container', () => {
const { container } = render(
<FieldErrors
id="test-id"
errors={{ [FieldErrors.errorTypes.MISSING]: true }}
isVisible={true}
/>
);
expect(screen.getByRole('alert')).toBeInTheDocument();
expect(container.querySelector('#test-id')).toBeInTheDocument();
});

it('should render the "negative" error', () => {
render(
<FieldErrors
Expand Down
22 changes: 7 additions & 15 deletions packages/components/field-errors/src/field-errors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const FieldErrors = (props: TFieldErrorsProps) => {
if (!props.errors || !isObject(props.errors)) return null;

return (
<>
<div id={props.id} role="alert">
{Object.entries(props.errors)
// Only render errors which have truthy values, to avoid
// rendering an error for, e.g. { missing: false }
Expand All @@ -51,48 +51,40 @@ const FieldErrors = (props: TFieldErrorsProps) => {
// Render a custom error if one was provided.
// Custom errors take precedence over the default errors
if (errorElement)
return (
<ErrorMessage key={key} id={props.id}>
{errorElement}
</ErrorMessage>
);
return <ErrorMessage key={key}>{errorElement}</ErrorMessage>;

const defaultErrorElement = props.renderDefaultError
? props.renderDefaultError(key, error)
: null;
// Render a default error if one was provided.
// Default errors take precedence over the known errors
if (defaultErrorElement)
return (
<ErrorMessage key={key} id={props.id}>
{defaultErrorElement}
</ErrorMessage>
);
return <ErrorMessage key={key}>{defaultErrorElement}</ErrorMessage>;

// Try to see if we know this error and render that error instead then
if (key === FieldErrors.errorTypes.MISSING)
return (
<ErrorMessage key={key} id={props.id}>
<ErrorMessage key={key}>
<FormattedMessage {...messages.missingRequiredField} />
</ErrorMessage>
);
if (key === FieldErrors.errorTypes.NEGATIVE)
return (
<ErrorMessage key={key} id={props.id}>
<ErrorMessage key={key}>
<FormattedMessage {...messages.invalidNegativeNumber} />
</ErrorMessage>
);
if (key === FieldErrors.errorTypes.FRACTIONS)
return (
<ErrorMessage key={key} id={props.id}>
<ErrorMessage key={key}>
<FormattedMessage {...messages.invalidFractionalNumber} />
</ErrorMessage>
);
// Render nothing in case the error is not known and no custom error
// was returned
return null;
})}
</>
</div>
);
};

Expand Down
17 changes: 17 additions & 0 deletions packages/components/field-warnings/src/field-warnings.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,23 @@ describe('when not visible', () => {
});

describe('warning renderers', () => {
it('should set the id as an attribute of the container', () => {
const { container } = render(
<FieldWarnings
id="test-id"
warnings={{ customWarning: true }}
isVisible={true}
renderWarning={(key) =>
key === 'customWarning' ? 'RENDER_WARNING' : null
}
renderDefaultWarning={(key) =>
key === 'defaultWarning' ? 'RENDER_DEFAULT_WARNING' : null
}
/>
);
expect(screen.getByRole('status')).toBeInTheDocument();
expect(container.querySelector('#test-id')).toBeInTheDocument();
});
it('should give highest importance to renderWarning', () => {
render(
<FieldWarnings
Expand Down
14 changes: 4 additions & 10 deletions packages/components/field-warnings/src/field-warnings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const FieldWarnings = (props: TFieldWarningsProps) => {
if (!props.warnings || !isObject(props.warnings)) return null;

return (
<>
<div id={props.id} role="status">
{Object.entries(props.warnings)
// Only render warnings which have truthy values, to avoid
// rendering a warning that has falsy values.
Expand All @@ -49,11 +49,7 @@ const FieldWarnings = (props: TFieldWarningsProps) => {
// Render a custom warning if one was provided.
// Custom warnings take precedence over the default warnings
if (warningElement)
return (
<WarningMessage key={key} id={props.id}>
{warningElement}
</WarningMessage>
);
return <WarningMessage key={key}>{warningElement}</WarningMessage>;

const defaultWarningElement = props.renderDefaultWarning
? props.renderDefaultWarning(key, warning)
Expand All @@ -62,15 +58,13 @@ const FieldWarnings = (props: TFieldWarningsProps) => {
// Default warnings take precedence over the known warnings
if (defaultWarningElement)
return (
<WarningMessage key={key} id={props.id}>
{defaultWarningElement}
</WarningMessage>
<WarningMessage key={key}>{defaultWarningElement}</WarningMessage>
);
// Render nothing in case the warning is not known and no custom warning
// was returned
return null;
})}
</>
</div>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,25 @@ describe('when showing an info button', () => {
});

describe('when field is touched and has errors', () => {
it('should render an id for the error container that is based on the component id', async () => {
const { container, findByText } = renderAsyncCreatableSelectField({
touched: true,
errors: { missing: true },
});
await findByText(/field is required/i);
expect(
container.querySelector('#async-creatable-select-field-errors')
).toBeInTheDocument();
});
it('should set the aria-errormessage value to the id of the error container', async () => {
const { findByRole } = renderAsyncCreatableSelectField({
touched: true,
errors: { missing: true },
});
expect(await findByRole('combobox')).toHaveAccessibleErrorMessage(
/field is required/i
);
});
describe('when field empty', () => {
it('should render a default error', async () => {
const { findByText } = renderAsyncCreatableSelectField({
Expand All @@ -231,6 +250,17 @@ describe('when field is touched and has errors', () => {
});

describe('when field is touched and has warnings', () => {
it('should render an id for the warning container that is based on the component id', async () => {
const { container, findByText } = renderAsyncCreatableSelectField({
touched: true,
warnings: { customWarning: true },
renderWarning: () => 'Custom warning',
});
await findByText('Custom warning');
expect(
container.querySelector('#async-creatable-select-field-warnings')
).toBeInTheDocument();
});
describe('when there is a custom warning', () => {
it('should render the custom warning message', async () => {
const { findByText } = renderAsyncCreatableSelectField({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ type TCustomFormErrors<Values> = {
};

const sequentialId = createSequentialId('async-creatable-select-field-');
const sequentialErrorsId = createSequentialId(
'async-creatable-select-field-error-'
)();
const sequentialWarningsId = createSequentialId(
'async-creatable-select-field-warning-'
)();

const hasErrors = (errors?: TFieldErrors) =>
errors && Object.values(errors).some(Boolean);
Expand Down Expand Up @@ -463,6 +457,9 @@ export default class AsyncCreatableSelectField extends Component<
);
}

const errorsContainerId = `${this.state.id}-errors`;
const warningsContainerId = `${this.state.id}-warnings`;

return (
<Constraints.Horizontal max={this.props.horizontalConstraint}>
<Spacings.Stack scale="xs">
Expand All @@ -482,7 +479,7 @@ export default class AsyncCreatableSelectField extends Component<
aria-label={this.props['aria-label']}
aria-labelledby={this.props['aria-labelledby']}
aria-invalid={hasError}
aria-errormessage={sequentialErrorsId}
aria-errormessage={errorsContainerId}
isAutofocussed={this.props.isAutofocussed}
backspaceRemovesValue={this.props.backspaceRemovesValue}
components={this.props.components}
Expand Down Expand Up @@ -527,13 +524,13 @@ export default class AsyncCreatableSelectField extends Component<
{...filterDataAttributes(this.props)}
/>
<FieldErrors
id={sequentialErrorsId}
id={errorsContainerId}
errors={this.props.errors}
isVisible={hasError}
renderError={this.props.renderError}
/>
<FieldWarnings
id={sequentialWarningsId}
id={warningsContainerId}
warnings={this.props.warnings}
isVisible={hasWarning}
renderWarning={this.props.renderWarning}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,27 @@ describe('when showing an info button', () => {
});

describe('when field is touched and has errors', () => {
it('should render an id for the error container that is based on the component id', async () => {
const { container, findByText } = renderAsyncSelectField({
touched: true,
errors: { custom: true },
renderError: () => 'Custom error',
});
await findByText(/custom error/i);
expect(
container.querySelector('#async-select-field-errors')
).toBeInTheDocument();
});
it('should set the aria-errormessage value to the id of the error container', async () => {
const { findByRole } = renderAsyncSelectField({
touched: true,
errors: { custom: true },
renderError: () => 'Custom error',
});
expect(await findByRole('combobox')).toHaveAccessibleErrorMessage(
/Custom Error/i
);
});
describe('when field empty', () => {
it('should render a default error', async () => {
const { findByText } = renderAsyncSelectField({
Expand All @@ -213,6 +234,17 @@ describe('when field is touched and has errors', () => {
});

describe('when field is touched and has warnings', () => {
it('should render an id for the warning container that is based on the component id', async () => {
const { container, findByText } = renderAsyncSelectField({
touched: true,
warnings: { customWarning: true },
renderWarning: () => 'Custom warning',
});
await findByText('Custom warning');
expect(
container.querySelector('#async-select-field-warnings')
).toBeInTheDocument();
});
describe('when there is a custom warning', () => {
it('should render the custom warning message', async () => {
const { findByText } = renderAsyncSelectField({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ type TCustomEvent = {
};

const sequentialId = createSequentialId('async-select-field-');
const sequentialErrorsId = createSequentialId('async-select-field-error-')();
const sequentialWarningsId = createSequentialId(
'async-select-field-warning-'
)();

const hasErrors = (errors?: TFieldErrors) =>
errors && Object.values(errors).some(Boolean);
Expand Down Expand Up @@ -419,6 +415,9 @@ export default class AsyncSelectField extends Component<
);
}

const errorsContainerId = `${this.state.id}-errors`;
const warningsContainerId = `${this.state.id}-warnings`;

return (
<Constraints.Horizontal max={this.props.horizontalConstraint}>
<Spacings.Stack scale="xs">
Expand All @@ -438,7 +437,7 @@ export default class AsyncSelectField extends Component<
aria-label={this.props['aria-label']}
aria-labelledby={this.props['aria-labelledby']}
aria-invalid={hasError}
aria-errormessage={sequentialErrorsId}
aria-errormessage={errorsContainerId}
isAutofocussed={this.props.isAutofocussed}
backspaceRemovesValue={this.props.backspaceRemovesValue}
components={this.props.components}
Expand Down Expand Up @@ -478,13 +477,13 @@ export default class AsyncSelectField extends Component<
controlShouldRenderValue={this.props.controlShouldRenderValue}
/>
<FieldErrors
id={sequentialErrorsId}
id={errorsContainerId}
errors={this.props.errors}
isVisible={hasError}
renderError={this.props.renderError}
/>
<FieldWarnings
id={sequentialWarningsId}
id={warningsContainerId}
warnings={this.props.warnings}
isVisible={hasWarning}
renderWarning={this.props.renderWarning}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,24 @@ describe('when showing an info button', () => {
});

describe('when field is touched and has errors', () => {
it('should render an id for the error container that is based on the component id', () => {
const { container } = renderCreatableSelectField({
touched: true,
errors: { missing: true },
});
expect(
container.querySelector('#creatable-select-field-errors')
).toBeInTheDocument();
});
it('should set the aria-errormessage value to the id of the error container', () => {
const { getByRole } = renderCreatableSelectField({
touched: true,
errors: { missing: true },
});
expect(getByRole('combobox')).toHaveAccessibleErrorMessage(
/field is required/i
);
});
describe('when field empty', () => {
it('should render a default error', () => {
const { getByText } = renderCreatableSelectField({
Expand All @@ -200,6 +218,16 @@ describe('when field is touched and has errors', () => {
});

describe('when field is touched and has warnings', () => {
it('should render an id for the warning container that is based on the component id', () => {
const { container } = renderCreatableSelectField({
touched: true,
warnings: { customWarning: true },
renderWarning: () => 'Custom warning',
});
expect(
container.querySelector('#creatable-select-field-warnings')
).toBeInTheDocument();
});
describe('when there is a custom warning', () => {
it('should render the custom warning message', () => {
const { getByText } = renderCreatableSelectField({
Expand Down
Loading

0 comments on commit c41a012

Please sign in to comment.