From d7d277a65128d3f6212008ab475f92d37c478e17 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Fri, 14 Feb 2020 13:53:35 -0500 Subject: [PATCH 1/5] improving a11y of BottomBar and ControlBar --- src/components/bottom_bar/bottom_bar.tsx | 61 +++++++++----- src/components/control_bar/control_bar.tsx | 94 +++++++++++++++------- 2 files changed, 109 insertions(+), 46 deletions(-) diff --git a/src/components/bottom_bar/bottom_bar.tsx b/src/components/bottom_bar/bottom_bar.tsx index 67d633ac67b..5ef9b148079 100644 --- a/src/components/bottom_bar/bottom_bar.tsx +++ b/src/components/bottom_bar/bottom_bar.tsx @@ -1,10 +1,9 @@ -import React, { Component } from 'react'; import classNames from 'classnames'; - -import { CommonProps } from '../common'; -import { EuiPortal } from '../portal'; +import React, { Component } from 'react'; import { EuiScreenReaderOnly } from '../accessibility'; +import { CommonProps } from '../common'; import { EuiI18n } from '../i18n'; +import { EuiPortal } from '../portal'; type BottomBarPaddingSize = 'none' | 's' | 'm' | 'l'; @@ -28,10 +27,15 @@ interface Props extends CommonProps { * Padding applied to the bar */ paddingSize?: BottomBarPaddingSize; + + /** + * Customize the screen reader heading that helps users find this control. Default is "Page level controls". + */ + title?: string; } export class EuiBottomBar extends Component { - private bar: HTMLDivElement | null = null; + private bar: HTMLElement | null = null; componentDidMount() { const height = this.bar ? this.bar.clientHeight : -1; @@ -42,7 +46,7 @@ export class EuiBottomBar extends Component { } componentWillUnmount() { - document.body.style.paddingBottom = null; + document.body.style.paddingBottom = ''; if (this.props.bodyClassName) { document.body.classList.remove(this.props.bodyClassName); } @@ -54,6 +58,7 @@ export class EuiBottomBar extends Component { className, paddingSize = 'm', bodyClassName, + title, ...rest } = this.props; @@ -65,22 +70,42 @@ export class EuiBottomBar extends Component { return ( + + {(screenReaderHeading: string) => ( + // Though it would be better to use aria-labelledby than aria-label and not repeat the same string twice + // A bug in voiceover won't list some landmarks in the rotor without an aria-label +
{ + this.bar = node; + }} + {...rest}> + +

{title ? title : screenReaderHeading}

+ + {children} +
+ )} +

- + {title ? ( + + ) : ( + + )}

-
{ - this.bar = node; - }} - {...rest}> - {children} -
); } diff --git a/src/components/control_bar/control_bar.tsx b/src/components/control_bar/control_bar.tsx index 61c245c8805..1320951fe88 100644 --- a/src/components/control_bar/control_bar.tsx +++ b/src/components/control_bar/control_bar.tsx @@ -1,27 +1,28 @@ +import classNames from 'classnames'; import React, { + ButtonHTMLAttributes, Component, HTMLAttributes, - ButtonHTMLAttributes, Ref, } from 'react'; -import classNames from 'classnames'; -import { - CommonProps, - ExclusiveUnion, - PropsForAnchor, - PropsForButton, -} from '../common'; -// @ts-ignore-next-line +import { EuiScreenReaderOnly } from '../accessibility'; import { EuiBreadcrumbs, EuiBreadcrumbsProps } from '../breadcrumbs'; import { EuiButton, EuiButtonIcon, - EuiButtonProps, EuiButtonIconProps, + EuiButtonProps, } from '../button'; -import { EuiPortal } from '../portal'; +import { + CommonProps, + ExclusiveUnion, + PropsForAnchor, + PropsForButton, +} from '../common'; +import { EuiI18n } from '../i18n'; import { EuiIcon } from '../icon'; import { EuiIconProps } from '../icon/icon'; +import { EuiPortal } from '../portal'; /** * Extends EuiButton excluding `size`. Requires `label` as the `children`. @@ -190,6 +191,11 @@ export type EuiControlBarProps = HTMLAttributes & * Optional class applied to the body used when `position = fixed` */ bodyClassName?: string; + + /** + * Customize the screen reader heading that helps users find this control. Default is "Page level controls". + */ + title?: string; }; interface EuiControlBarState { @@ -208,7 +214,7 @@ export class EuiControlBar extends Component< showContent: false, showOnMobile: false, }; - private bar: HTMLDivElement | null = null; + private bar: HTMLElement | null = null; componentDidMount() { if (this.props.position === 'fixed') { @@ -221,7 +227,7 @@ export class EuiControlBar extends Component< } componentWillUnmount() { - document.body.style.paddingBottom = null; + document.body.style.paddingBottom = ''; if (this.props.bodyClassName) { document.body.classList.remove(this.props.bodyClassName); } @@ -245,6 +251,7 @@ export class EuiControlBar extends Component< style, position, bodyClassName, + title, ...rest } = this.props; @@ -396,24 +403,55 @@ export class EuiControlBar extends Component< }; const controlBar = ( -
-
{ - this.bar = node; - }}> - {controls.map((control, index) => { - return controlItem(control, index); - })} -
- {this.props.showContent ? ( -
{children}
- ) : null} -
+ + {(screenReaderHeading: string) => ( +
+ +

{title ? title : screenReaderHeading}

+ +
{ + this.bar = node; + }}> + {controls.map((control, index) => { + return controlItem(control, index); + })} +
+ {this.props.showContent ? ( +
{children}
+ ) : null} +
+ )} + ); return position === 'fixed' ? ( - {controlBar} + + {controlBar} + +

+ {title ? ( + + ) : ( + + )} +

+ + ) : ( controlBar ); From fe410a6a850e97e6060bd118cf58df17901234d4 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Fri, 14 Feb 2020 14:01:44 -0500 Subject: [PATCH 2/5] adding changelog line and a comment --- CHANGELOG.md | 1 + src/components/control_bar/control_bar.tsx | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd58c908825..4b4c6a79ae4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ **Bug fixes** - Fixed building dev & docs on Windows ([#2847](https://github.com/elastic/eui/pull/2847)) +- Fixed screen reader discovery issues with `EuiBottomBar` and `EuiControlBar` ([#2861](https://github.com/elastic/eui/pull/2861)) ## [`19.0.0`](https://github.com/elastic/eui/tree/v19.0.0) diff --git a/src/components/control_bar/control_bar.tsx b/src/components/control_bar/control_bar.tsx index 1320951fe88..e21a4d204c4 100644 --- a/src/components/control_bar/control_bar.tsx +++ b/src/components/control_bar/control_bar.tsx @@ -407,6 +407,8 @@ export class EuiControlBar extends Component< token="euiControlBar.screenReaderHeading" default="Page level controls"> {(screenReaderHeading: string) => ( + // Though it would be better to use aria-labelledby than aria-label and not repeat the same string twice + // A bug in voiceover won't list some landmarks in the rotor without an aria-label
Date: Fri, 14 Feb 2020 17:26:52 -0500 Subject: [PATCH 3/5] updating jest snapshots --- .../__snapshots__/bottom_bar.test.tsx.snap | 81 +- .../__snapshots__/control_bar.test.tsx.snap | 1922 ++++++++++------- 2 files changed, 1141 insertions(+), 862 deletions(-) diff --git a/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap b/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap index 2cc54e4350b..8ec6d9cb6ee 100644 --- a/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap +++ b/src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap @@ -2,74 +2,107 @@ exports[`EuiBottomBar is rendered 1`] = ` Array [ -

- There is a new menu opening with page level controls at the end of the document. -

, -
+

+ Page level controls +

Content -
, +
, +

+ There is a new region landmark with page level controls at the end of the document. +

, ] `; exports[`EuiBottomBar props paddingSize l is rendered 1`] = ` Array [ +
+

+ Page level controls +

+
,

- There is a new menu opening with page level controls at the end of the document. + There is a new region landmark with page level controls at the end of the document.

, -
, ] `; exports[`EuiBottomBar props paddingSize m is rendered 1`] = ` Array [ +
+

+ Page level controls +

+
,

- There is a new menu opening with page level controls at the end of the document. + There is a new region landmark with page level controls at the end of the document.

, -
, ] `; exports[`EuiBottomBar props paddingSize none is rendered 1`] = ` Array [ +
+

+ Page level controls +

+
,

- There is a new menu opening with page level controls at the end of the document. + There is a new region landmark with page level controls at the end of the document.

, -
, ] `; exports[`EuiBottomBar props paddingSize s is rendered 1`] = ` Array [ +
+

+ Page level controls +

+
,

- There is a new menu opening with page level controls at the end of the document. + There is a new region landmark with page level controls at the end of the document.

, -
, ] `; diff --git a/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap b/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap index a191eb9a04f..b87cc4ba648 100644 --- a/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap +++ b/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap @@ -1,12 +1,17 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`EuiControlBar is rendered 1`] = ` -
+

+ Page level controls +

@@ -75,7 +80,7 @@ exports[`EuiControlBar is rendered 1`] = ` Flight 815
-
+ `; exports[`EuiControlBar props leftOffset is rendered 1`] = ` @@ -139,10 +144,16 @@ exports[`EuiControlBar props leftOffset is rendered 1`] = ` -
+

+ Page level controls +

@@ -211,149 +222,181 @@ exports[`EuiControlBar props leftOffset is rendered 1`] = ` Flight 815
-
+ +

+ There is a new region landmark with page level controls at the end of the document. +

} > -
-
- + +

+ Page level controls +

+ +
- - - - + + + +
+ + + + components + + + + + - - Sound the Alarm + + Sound the Alarm + - - - -
- Close the Hatch -
-
- - + +
+ Close the Hatch +
+
+ - - - -
- +
+ + + +

+ - Flight 815 - -

-
+ There is a new region landmark with page level controls at the end of the document. + +

+ @@ -421,10 +464,16 @@ exports[`EuiControlBar props maxHeight is rendered 1`] = ` -
+

+ Page level controls +

@@ -493,149 +542,181 @@ exports[`EuiControlBar props maxHeight is rendered 1`] = ` Flight 815
-
+ +

+ There is a new region landmark with page level controls at the end of the document. +

} > -
-
- + +

+ Page level controls +

+ +
- - - - + + + +
+ + + + components + + + + + - - Sound the Alarm + + Sound the Alarm + - - - -
- Close the Hatch -
-
- - + +
+ Close the Hatch +
+
+ - - - -
- +
+ + + +

+ - Flight 815 - -

-
+ There is a new region landmark with page level controls at the end of the document. + +

+ @@ -702,10 +783,16 @@ exports[`EuiControlBar props mobile is rendered 1`] = ` -
+

+ Page level controls +

@@ -774,149 +861,181 @@ exports[`EuiControlBar props mobile is rendered 1`] = ` Flight 815
-
+ +

+ There is a new region landmark with page level controls at the end of the document. +

} > -
-
- + +

+ Page level controls +

+ +
- - - - + + + +
+ + + + components + + + + + - - Sound the Alarm + + Sound the Alarm + - - - -
- Close the Hatch -
-
- - + +
+ Close the Hatch +
+
+ - - - -
- +
+ + + +

+ - Flight 815 - -

-
+ There is a new region landmark with page level controls at the end of the document. + +

+ @@ -979,145 +1098,158 @@ exports[`EuiControlBar props position is rendered 1`] = ` showOnMobile={false} size="l" > -
-
- +

+ Page level controls +

+ +
- - - - + + + +
+ + + + components + + + + + - - Sound the Alarm + + Sound the Alarm + - - - -
- Close the Hatch -
-
- - + +
+ Close the Hatch +
+
+ - - - -
- -
-
+ > + + + +
+ +
+ + `; @@ -1182,10 +1314,16 @@ exports[`EuiControlBar props rightOffset is rendered 1`] = ` -
+

+ Page level controls +

@@ -1254,149 +1392,181 @@ exports[`EuiControlBar props rightOffset is rendered 1`] = ` Flight 815
-
+ +

+ There is a new region landmark with page level controls at the end of the document. +

} > -
-
- + +

+ Page level controls +

+ +
- - - - + + + +
+ + + + components + + + + + - - Sound the Alarm + + Sound the Alarm + - - - -
- Close the Hatch -
-
- - + +
+ Close the Hatch +
+
+ - - - -
- +
+ + + +

+ - Flight 815 - -

-
+ There is a new region landmark with page level controls at the end of the document. + +

+ @@ -1463,10 +1633,16 @@ exports[`EuiControlBar props showContent is rendered 1`] = ` -
+

+ Page level controls +

@@ -1540,154 +1716,186 @@ exports[`EuiControlBar props showContent is rendered 1`] = ` > Content
-
+ +

+ There is a new region landmark with page level controls at the end of the document. +

} > -
-
- + +

+ Page level controls +

+ +
- - - - + + + +
+ + + + components + + + + + - - Sound the Alarm + + Sound the Alarm + - - - -
- Close the Hatch -
-
- - + +
+ Close the Hatch +
+
+ - - - + > + + + +
+ +
- -
-
+ + + +

- Content -

-
+ + There is a new region landmark with page level controls at the end of the document. + +

+ @@ -1754,10 +1962,16 @@ exports[`EuiControlBar props size is rendered 1`] = ` -
+

+ Page level controls +

@@ -1826,149 +2040,181 @@ exports[`EuiControlBar props size is rendered 1`] = ` Flight 815
-
+ +

+ There is a new region landmark with page level controls at the end of the document. +

} > -
-
- + +

+ Page level controls +

+ +
- - - - + + + +
+ + + + components + + + + + - - Sound the Alarm + + Sound the Alarm + - - - -
- Close the Hatch -
-
- - + +
+ Close the Hatch +
+
+ - - - -
- +
+ + + +

+ - Flight 815 - -

-
+ There is a new region landmark with page level controls at the end of the document. + +

+ From 785de7e6416daaf9282c2ddb269bc73f1af932f2 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Tue, 18 Feb 2020 18:49:39 -0500 Subject: [PATCH 4/5] fixing i18n string id --- src/components/bottom_bar/bottom_bar.tsx | 20 ++++++----- .../context_menu_panel.test.tsx.snap | 36 ++++++++++++++----- .../__snapshots__/control_bar.test.tsx.snap | 2 +- src/components/control_bar/control_bar.tsx | 16 ++++----- 4 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/components/bottom_bar/bottom_bar.tsx b/src/components/bottom_bar/bottom_bar.tsx index 5ef9b148079..e8e5ecfdd31 100644 --- a/src/components/bottom_bar/bottom_bar.tsx +++ b/src/components/bottom_bar/bottom_bar.tsx @@ -31,7 +31,7 @@ interface Props extends CommonProps { /** * Customize the screen reader heading that helps users find this control. Default is "Page level controls". */ - title?: string; + landmarkHeading?: string; } export class EuiBottomBar extends Component { @@ -58,7 +58,7 @@ export class EuiBottomBar extends Component { className, paddingSize = 'm', bodyClassName, - title, + landmarkHeading, ...rest } = this.props; @@ -77,14 +77,18 @@ export class EuiBottomBar extends Component { // Though it would be better to use aria-labelledby than aria-label and not repeat the same string twice // A bug in voiceover won't list some landmarks in the rotor without an aria-label
{ this.bar = node; }} {...rest}> -

{title ? title : screenReaderHeading}

+

+ {landmarkHeading ? landmarkHeading : screenReaderHeading} +

{children}
@@ -92,11 +96,11 @@ export class EuiBottomBar extends Component {

- {title ? ( + {landmarkHeading ? ( ) : ( + > + + @@ -193,14 +198,19 @@ exports[`EuiContextMenu props panels and initialPanelId allows you to click the + > + +

diff --git a/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap b/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap index b87cc4ba648..5cfb2a88ec0 100644 --- a/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap +++ b/src/components/control_bar/__snapshots__/control_bar.test.tsx.snap @@ -2,7 +2,7 @@ exports[`EuiControlBar is rendered 1`] = `
& /** * Customize the screen reader heading that helps users find this control. Default is "Page level controls". */ - title?: string; + landmarkHeading?: string; }; interface EuiControlBarState { @@ -251,7 +251,7 @@ export class EuiControlBar extends Component< style, position, bodyClassName, - title, + landmarkHeading, ...rest } = this.props; @@ -411,11 +411,11 @@ export class EuiControlBar extends Component< // A bug in voiceover won't list some landmarks in the rotor without an aria-label
+ style={styles}> -

{title ? title : screenReaderHeading}

+

{landmarkHeading ? landmarkHeading : screenReaderHeading}

- {title ? ( + {landmarkHeading ? ( ) : ( Date: Tue, 25 Feb 2020 13:34:59 -0500 Subject: [PATCH 5/5] i18n fix --- src/components/control_bar/control_bar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/control_bar/control_bar.tsx b/src/components/control_bar/control_bar.tsx index 7610255f024..139debaeed2 100644 --- a/src/components/control_bar/control_bar.tsx +++ b/src/components/control_bar/control_bar.tsx @@ -441,7 +441,7 @@ export class EuiControlBar extends Component<

{landmarkHeading ? (