diff --git a/CHANGELOG.md b/CHANGELOG.md index cd947f1c8e4..91f5d6dee85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,17 @@ - Converted `EuiFilterButton` to TypeScript ([#2761](https://github.com/elastic/eui/pull/2761)) - Converted `EuiFilterSelectItem` to TypeScript ([#2761](https://github.com/elastic/eui/pull/2761)) - Converted `EuiFieldSearch` to TypeScript ([#2775](https://github.com/elastic/eui/pull/2775)) +- Improved `EuiDescribedFormGroup` accessibility by avoiding duplicated output in screen readers ([#2783](https://github.com/elastic/eui/pull/2783)) - Added `data-test-subj` to the `EuiContextMenuItem` in `EuiTablePagination` ([#2778](https://github.com/elastic/eui/pull/2778)) **Bug fixes** - Increased column width on `EuiTableHeaderCellCheckbox` to prevent `EuiCheckbox`'s focus ring from getting clipped in `EuiBasicTable` ([#2770](https://github.com/elastic/eui/pull/2770)) +**Breaking changes** + +- Removed `idAria` prop from `EuiDescribedFormGroup` ([#2783](https://github.com/elastic/eui/pull/2783)) + **Deprecations** - `EuiIcon`'s `logoEnterpriseSearch` type deprecated in favor of `logoWorkplaceSearch` diff --git a/src-docs/src/views/form_layouts/described_form_group.js b/src-docs/src/views/form_layouts/described_form_group.js index 58c3abe07d1..aa77d9b9dee 100644 --- a/src-docs/src/views/form_layouts/described_form_group.js +++ b/src-docs/src/views/form_layouts/described_form_group.js @@ -10,6 +10,7 @@ import { EuiRange, EuiSelect, EuiSwitch, + EuiLink, } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -86,45 +87,34 @@ export default class extends Component { return ( Single text field} description={ - When using this with a single form row where this text serves as - the help text for the input, it is a good idea to pass{' '} - idAria="someID" to the form group and - pass - describedByIds={[someID]} to its form - row. + A single text field that can be used to display additional text. + It can have{' '} + + links + {' '} + or any other type of content. }> - - + + - No description}> - + No description}> + Multiple fields} - titleSize="m" + title={

Multiple fields

} description="Here are three form rows. The first form row does not have a title."> - We do not pass describedByIds when there are - multiple form rows. - - }> + helpText={This is a help text}> Use EuiDescribedFormGroup component to associate multiple EuiFormRows. It can also simply be used - with one EuiFormRow as a way to display help text - (or additional text) next to the field instead of below (on mobile, - will revert to being stacked). + with one EuiFormRow as a way to display additional + text next to the field (on mobile, it will revert to being stacked).

), props: { @@ -129,7 +128,6 @@ export const FormLayoutsExample = { }, demo: , snippet: `Set heading level based on context} description={ @@ -139,7 +137,6 @@ export const FormLayoutsExample = { > diff --git a/src/components/form/described_form_group/__snapshots__/described_form_group.test.js.snap b/src/components/form/described_form_group/__snapshots__/described_form_group.test.js.snap index 4e1616848fe..3e2ad107f75 100644 --- a/src/components/form/described_form_group/__snapshots__/described_form_group.test.js.snap +++ b/src/components/form/described_form_group/__snapshots__/described_form_group.test.js.snap @@ -1,239 +1,552 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`EuiDescribedFormGroup is rendered 1`] = ` -
+ Title + + } + titleSize="xs" > - - - -

- Title -

- - - Test description - - - +
- + + Title + + + - - - - -
+
+ +
+ + + + +
+ +
+ Test description +
+ +
+ +
+ + +
+ +
+
+ +
+
+ +
+ +
+ + + + `; exports[`EuiDescribedFormGroup props description is not rendered when it's not provided 1`] = ` -
+ Title + + } + titleSize="xs" > - - - -

- Title -

- - - +
- + + Title + + + - - - - -
+
+ +
+ + + +
+ + +
+ +
+
+ +
+
+ +
+ +
+ + + + `; exports[`EuiDescribedFormGroup props fullWidth is rendered 1`] = ` -
+ Title + + } + titleSize="xs" > - - - -

- Title -

- - - Test description - - - +
- + + Title + + + - - - - -
+
+ +
+ + + + +
+ +
+ Test description +
+ +
+ +
+ + +
+ +
+
+ +
+
+ +
+ +
+ + + + `; exports[`EuiDescribedFormGroup props gutterSize is rendered 1`] = ` -
+ Title + + } + titleSize="xs" > - - - -

- Title -

- - - Test description - - - +
- + + Title + + + - - - - -
+
+ +
+ + + + +
+ +
+ Test description +
+ +
+ +
+ + +
+ +
+
+ +
+
+ +
+ +
+ + + + `; exports[`EuiDescribedFormGroup props titleSize is rendered 1`] = ` -
+ Title + + } + titleSize="l" > - - - -

- Title -

- - - Test description - - - +
- + + Title + + + - - - - -
+
+ +
+ + + + +
+ +
+ Test description +
+ +
+ +
+ + +
+ +
+
+ +
+
+ +
+ +
+ + + + `; exports[`EuiDescribedFormGroup ties together parts for accessibility 1`] = ` @@ -244,7 +557,6 @@ exports[`EuiDescribedFormGroup ties together parts for accessibility 1`] = ` description="Test description" fullWidth={false} gutterSize="l" - idAria="test-id" title={

Title @@ -252,164 +564,168 @@ exports[`EuiDescribedFormGroup ties together parts for accessibility 1`] = ` } titleSize="xs" > -
- +
-
+ + Title + + + - -
- + +
-

- Title -

- - - - -
- - -
+ Test description +
+ +
+ +
+ + - -
- - - -
-
- - + Label + + +
+
-
+ - Error one -
- - -
+ Error one +
+ + - Error two -
- - -
+ Error two +
+ + - Help text -
- +
+ Help text +
+ +
- - - - - - - + + + + + + + `; diff --git a/src/components/form/described_form_group/described_form_group.js b/src/components/form/described_form_group/described_form_group.js index 6b130e57c35..3f9d2df5836 100644 --- a/src/components/form/described_form_group/described_form_group.js +++ b/src/components/form/described_form_group/described_form_group.js @@ -1,4 +1,4 @@ -import React, { Component } from 'react'; +import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; @@ -7,7 +7,8 @@ import { EuiText } from '../../text/text'; import { EuiFlexGroup, EuiFlexItem } from '../../flex'; import { GUTTER_SIZES } from '../../flex/flex_group'; -import makeId from '../form_row/make_id'; +import { EuiScreenReaderOnly } from '../../accessibility'; +import { EuiInnerText } from '../../inner_text'; const paddingSizeToClassNameMap = { xxxs: 'euiDescribedFormGroup__fieldPadding--xxxsmall', @@ -18,12 +19,7 @@ const paddingSizeToClassNameMap = { l: 'euiDescribedFormGroup__fieldPadding--large', }; -export class EuiDescribedFormGroup extends Component { - constructor(props) { - super(props); - this.ariaId = props.idAria || makeId(); - } - +export class EuiDescribedFormGroup extends PureComponent { render() { const { children, @@ -33,12 +29,9 @@ export class EuiDescribedFormGroup extends Component { titleSize, title, description, - idAria: userAriaId, ...rest } = this.props; - const ariaId = this.ariaId; - const classes = classNames( 'euiDescribedFormGroup', { @@ -52,45 +45,46 @@ export class EuiDescribedFormGroup extends Component { paddingSizeToClassNameMap[titleSize] ); - const ariaProps = { - 'aria-labelledby': `${ariaId}-title`, - }; - let renderedDescription; if (description) { renderedDescription = ( {description} ); - - // If user has defined an aria ID, assume they have passed the ID to - // the form row describedByIds and skip describedby here. - ariaProps['aria-describedby'] = userAriaId ? null : ariaId; } return ( -
- - - - {title} - - - {renderedDescription} - - - {children} - -
+ + {(ref, innerText) => ( +
+ + {innerText} + + + + + + + + + {renderedDescription} + + + {children} + +
+ )} + ); } } @@ -107,9 +101,11 @@ EuiDescribedFormGroup.propTypes = { gutterSize: PropTypes.oneOf(GUTTER_SIZES), fullWidth: PropTypes.bool, titleSize: PropTypes.oneOf(TITLE_SIZES), + /** + * For better accessibility, it's recommended the use of HTML headings + */ title: PropTypes.node.isRequired, description: PropTypes.node, - idAria: PropTypes.string, }; EuiDescribedFormGroup.defaultProps = { diff --git a/src/components/form/described_form_group/described_form_group.test.js b/src/components/form/described_form_group/described_form_group.test.js index 061392f055b..f0e838916b1 100644 --- a/src/components/form/described_form_group/described_form_group.test.js +++ b/src/components/form/described_form_group/described_form_group.test.js @@ -1,5 +1,5 @@ import React from 'react'; -import { shallow, mount } from 'enzyme'; +import { mount } from 'enzyme'; import { requiredProps } from '../../../test'; import { EuiFormRow } from '../form_row'; @@ -14,7 +14,7 @@ describe('EuiDescribedFormGroup', () => { }; test('is rendered', () => { - const component = shallow( + const component = mount( @@ -26,23 +26,15 @@ describe('EuiDescribedFormGroup', () => { }); test('ties together parts for accessibility', () => { - const describedFormGroupProps = { - idAria: 'test-id', - }; - const formRowProps = { label: 'Label', helpText: 'Help text', isInvalid: true, error: ['Error one', 'Error two'], - describedByIds: ['test-id'], }; const tree = mount( - + @@ -58,7 +50,7 @@ describe('EuiDescribedFormGroup', () => { fullWidth: true, }; - const component = shallow( + const component = mount( { gutterSize: 's', }; - const component = shallow( + const component = mount( { titleSize: 'l', }; - const component = shallow( + const component = mount( { }); test("description is not rendered when it's not provided", () => { - const component = shallow( + const component = mount( Title

}>