From ad6eb8a5e669c32c31c8a790ae50f582ce9a1c56 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Wed, 8 Jul 2020 16:20:25 -0400 Subject: [PATCH 1/3] making more a11y callouts in docs --- src-docs/src/views/flyout/flyout.js | 1 + src-docs/src/views/flyout/flyout_banner.js | 5 +- .../src/views/flyout/flyout_complicated.js | 1 + src-docs/src/views/flyout/flyout_example.js | 59 +++++++++++------ src-docs/src/views/flyout/flyout_large.js | 1 + src-docs/src/views/flyout/flyout_max_width.js | 1 + src-docs/src/views/flyout/flyout_small.js | 1 - src-docs/src/views/modal/modal_example.js | 2 +- src-docs/src/views/popover/popover.js | 1 + src-docs/src/views/popover/popover_block.js | 1 + .../src/views/popover/popover_container.js | 1 + src-docs/src/views/popover/popover_example.js | 65 ++++++++++++------- src-docs/src/views/popover/popover_fixed.js | 1 + .../popover/popover_htmlelement_anchor.js | 1 + src-docs/src/views/popover/trap_focus.js | 1 - .../src/views/tool_tip/tool_tip_example.js | 31 +++++++++ 16 files changed, 126 insertions(+), 47 deletions(-) diff --git a/src-docs/src/views/flyout/flyout.js b/src-docs/src/views/flyout/flyout.js index e2190c51a85..ff9fd049086 100644 --- a/src-docs/src/views/flyout/flyout.js +++ b/src-docs/src/views/flyout/flyout.js @@ -30,6 +30,7 @@ export default () => { if (isFlyoutVisible) { flyout = ( setIsFlyoutVisible(false)} aria-labelledby="flyoutTitle"> diff --git a/src-docs/src/views/flyout/flyout_banner.js b/src-docs/src/views/flyout/flyout_banner.js index 8b4be85cc2e..a787f4631be 100644 --- a/src-docs/src/views/flyout/flyout_banner.js +++ b/src-docs/src/views/flyout/flyout_banner.js @@ -44,7 +44,10 @@ export default () => { if (isFlyoutVisible) { flyout = ( - +

A flyout with a banner

diff --git a/src-docs/src/views/flyout/flyout_complicated.js b/src-docs/src/views/flyout/flyout_complicated.js index d5e8aab5703..cce6ee80904 100644 --- a/src-docs/src/views/flyout/flyout_complicated.js +++ b/src-docs/src/views/flyout/flyout_complicated.js @@ -178,6 +178,7 @@ export default () => { if (isFlyoutVisible) { flyout = ( diff --git a/src-docs/src/views/flyout/flyout_example.js b/src-docs/src/views/flyout/flyout_example.js index f94c6cbb0a5..877a9d82a86 100644 --- a/src-docs/src/views/flyout/flyout_example.js +++ b/src-docs/src/views/flyout/flyout_example.js @@ -37,7 +37,7 @@ import FlyoutWithBanner from './flyout_banner'; const flyoutWithBannerSource = require('!!raw-loader!./flyout_banner'); const flyoutWithBannerHtml = renderToHtml(FlyoutWithBanner); -const flyOutSnippet = ` +const flyOutSnippet = `

@@ -49,7 +49,7 @@ const flyOutSnippet = ` `; -const flyoutComplicatedSnippet = ` +const flyoutComplicatedSnippet = `

@@ -67,7 +67,7 @@ const flyoutComplicatedSnippet = ` `; -const flyoutSmallSnippet = ` +const flyoutSmallSnippet = `

@@ -79,7 +79,7 @@ const flyoutSmallSnippet = ` `; -const flyoutMaxWidthSnippet = ` +const flyoutMaxWidthSnippet = `

@@ -91,7 +91,7 @@ const flyoutMaxWidthSnippet = ` `; -const flyoutLargeSnippet = ` +const flyoutLargeSnippet = `

@@ -103,7 +103,7 @@ const flyoutLargeSnippet = ` `; -const flyoutWithBannerSnippet = ` +const flyoutWithBannerSnippet = `

@@ -130,19 +130,24 @@ export const FlyoutExample = { }, ], text: ( -
+ <>

EuiFlyout is a fixed position panel that pops in from the right side of the screen. It should be used any time you need to perform quick, individual actions to a larger page or list.

-

- For accessibility, use{' '} - {'aria-labelledby={headingId}'} to announce the - flyout to screen readers when the user opens it. -

-
+ + Use {'aria-labelledby={headingId}'} and{' '} + ownFocus to announce the flyout to screen + readers when the user opens it. + + } + /> + ), props: { EuiFlyout, EuiFlyoutHeader, EuiFlyoutBody }, snippet: flyOutSnippet, @@ -195,7 +200,7 @@ export const FlyoutExample = { demo: , }, { - title: 'Small flyout, ownFocus', + title: 'Small flyout, without ownFocus', source: [ { type: GuideSectionTypes.JS, @@ -207,12 +212,26 @@ export const FlyoutExample = { }, ], text: ( -

- In this example, we set size to{' '} - s and apply the ownFocus prop. - The latter not only traps the focus of our flyout, but also adds - background overlay to reinforce your boundaries. -

+ <> +

+ In this example, we set size to{' '} + s and remove the ownFocus{' '} + prop. The latter not only remove the focus trap around the flyout, + but also remove the background overlay that reinforces your + boundaries. +

+ + Removing ownFocus makes it difficult for + keyboard-only and screen reader users to navigate to and from + your flyout. + + } + /> + ), snippet: flyoutSmallSnippet, demo: , diff --git a/src-docs/src/views/flyout/flyout_large.js b/src-docs/src/views/flyout/flyout_large.js index f1c2ce02929..45c16ed6a87 100644 --- a/src-docs/src/views/flyout/flyout_large.js +++ b/src-docs/src/views/flyout/flyout_large.js @@ -20,6 +20,7 @@ export default () => { if (isFlyoutVisible) { flyout = ( diff --git a/src-docs/src/views/flyout/flyout_max_width.js b/src-docs/src/views/flyout/flyout_max_width.js index 5fafe0c0fe9..d781bb615de 100644 --- a/src-docs/src/views/flyout/flyout_max_width.js +++ b/src-docs/src/views/flyout/flyout_max_width.js @@ -47,6 +47,7 @@ export default () => { flyout = ( { if (isFlyoutVisible) { flyout = ( diff --git a/src-docs/src/views/modal/modal_example.js b/src-docs/src/views/modal/modal_example.js index 8c08fb43de5..d601a9de739 100644 --- a/src-docs/src/views/modal/modal_example.js +++ b/src-docs/src/views/modal/modal_example.js @@ -77,7 +77,7 @@ export const ModalExample = {

Use a modal to temporarily interrupt a user’s current task and block interactions to the content below it. Be sure to read the full{' '} - modal usage guidelines. + modal usage guidelines.

), props: { EuiModal, EuiOverlayMask }, diff --git a/src-docs/src/views/popover/popover.js b/src-docs/src/views/popover/popover.js index b1412892282..8798fedd8f7 100644 --- a/src-docs/src/views/popover/popover.js +++ b/src-docs/src/views/popover/popover.js @@ -16,6 +16,7 @@ export default () => { return ( diff --git a/src-docs/src/views/popover/popover_block.js b/src-docs/src/views/popover/popover_block.js index 327486d094a..6462d4295e0 100644 --- a/src-docs/src/views/popover/popover_block.js +++ b/src-docs/src/views/popover/popover_block.js @@ -16,6 +16,7 @@ export default () => { return ( { return ( @@ -65,13 +67,13 @@ const popOverSnippet = ` `; const popoverAnchorSnippet = ``; const popoverWithTitleSnippet = ` @@ -99,6 +102,7 @@ const popoverPanelClassNameSnippet = ``; const popoverWithTitlePaddingSnippet = ``; const popoverContainerSnippet = ``; const popoverFixedSnippet = ``; const popoverBlockSnippet = `, }, - { - title: 'Trap focus', - source: [ - { - type: GuideSectionTypes.JS, - code: trapFocusSource, - }, - { - type: GuideSectionTypes.HTML, - code: trapFocusHtml, - }, - ], - text: ( -

- If the popover should be responsible for trapping the focus within - itself (as opposed to a child component), then you should set{' '} - ownFocus. -

- ), - snippet: trapFocusSnippet, - demo: , - }, { title: 'Anchor position', source: [ @@ -435,5 +420,39 @@ export const PopoverExample = { snippet: inputPopoverSnippet, demo: , }, + { + title: 'Removing the focus focus', + source: [ + { + type: GuideSectionTypes.JS, + code: trapFocusSource, + }, + { + type: GuideSectionTypes.HTML, + code: trapFocusHtml, + }, + ], + text: ( + <> +

+ If the popover cannot be responsible for trapping focus within + itself, then you can remove ownFocus. +

+ + Removing ownFocus makes it difficult for + keyboard-only and screen reader users to navigate to and from + your popover. + + } + /> + + ), + snippet: trapFocusSnippet, + demo: , + }, ], }; diff --git a/src-docs/src/views/popover/popover_fixed.js b/src-docs/src/views/popover/popover_fixed.js index e94932679dd..6fa4e9e482a 100644 --- a/src-docs/src/views/popover/popover_fixed.js +++ b/src-docs/src/views/popover/popover_fixed.js @@ -27,6 +27,7 @@ export default () => { Toggle Example {isExampleShown && ( { return ( diff --git a/src-docs/src/views/popover/trap_focus.js b/src-docs/src/views/popover/trap_focus.js index 73d8688cfc1..5f1a8db035f 100644 --- a/src-docs/src/views/popover/trap_focus.js +++ b/src-docs/src/views/popover/trap_focus.js @@ -22,7 +22,6 @@ export default () => { return ( + + + Anchoring a tooltip to a non-interactive element will make it + difficult for keyboard-only and screen reader users to read it. + + } + /> + + + + + Putting anything other than plain text into a tooltip will be lost + to screen readers. Consider switching to{' '} + + EuiPopover + {' '} + if you need more content inside a tooltip. + + } + /> + + ), sections: [ From fbdeaf5089c2707b654bfe7d0a3ce2c73729c399 Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Mon, 13 Jul 2020 21:56:34 -0400 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com> --- src-docs/src/views/flyout/flyout_example.js | 4 ++-- src-docs/src/views/popover/popover_example.js | 2 +- src-docs/src/views/tool_tip/tool_tip_example.js | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src-docs/src/views/flyout/flyout_example.js b/src-docs/src/views/flyout/flyout_example.js index 877a9d82a86..e341fe23d79 100644 --- a/src-docs/src/views/flyout/flyout_example.js +++ b/src-docs/src/views/flyout/flyout_example.js @@ -216,8 +216,8 @@ export const FlyoutExample = {

In this example, we set size to{' '} s and remove the ownFocus{' '} - prop. The latter not only remove the focus trap around the flyout, - but also remove the background overlay that reinforces your + prop. The latter not only removes the focus trap around the flyout, + but also removes the background overlay that reinforces your boundaries.

- If the popover cannot be responsible for trapping focus within + If the popover cannot trap focus within itself, then you can remove ownFocus.

- Anchoring a tooltip to a non-interactive element will make it - difficult for keyboard-only and screen reader users to read it. + Anchoring a tooltip to a non-interactive element makes it + difficult for keyboard-only and screen reader users to read. } /> @@ -106,8 +106,8 @@ export const ToolTipExample = { color="warning" title={ <> - Putting anything other than plain text into a tooltip will be lost - to screen readers. Consider switching to{' '} + Putting anything other than plain text in a tooltip is lost + on screen readers. Consider switching to{' '} EuiPopover {' '} From 89a274ced806b2267eb0626b274ad486930844fc Mon Sep 17 00:00:00 2001 From: Michail Yasonik Date: Wed, 15 Jul 2020 12:25:27 -0400 Subject: [PATCH 3/3] prettier update --- src-docs/src/views/popover/popover_example.js | 4 ++-- src-docs/src/views/tool_tip/tool_tip_example.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src-docs/src/views/popover/popover_example.js b/src-docs/src/views/popover/popover_example.js index 38d0e687ec1..1211336be6c 100644 --- a/src-docs/src/views/popover/popover_example.js +++ b/src-docs/src/views/popover/popover_example.js @@ -435,8 +435,8 @@ export const PopoverExample = { text: ( <>

- If the popover cannot trap focus within - itself, then you can remove ownFocus. + If the popover cannot trap focus within itself, then you can remove{' '} + ownFocus.

- Anchoring a tooltip to a non-interactive element makes it - difficult for keyboard-only and screen reader users to read. + Anchoring a tooltip to a non-interactive element makes it difficult + for keyboard-only and screen reader users to read. } /> @@ -106,8 +106,8 @@ export const ToolTipExample = { color="warning" title={ <> - Putting anything other than plain text in a tooltip is lost - on screen readers. Consider switching to{' '} + Putting anything other than plain text in a tooltip is lost on + screen readers. Consider switching to{' '} EuiPopover {' '}