From 4b06208ce9f4219b1a0f268e3130e2115fa91e8d Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 1 Feb 2023 08:59:32 -0800 Subject: [PATCH 1/7] EuiFlyout - auto-shard all fixed EuiHeaders NOT YET WORKING: autoFocus on flyout open and returnFocus --- src/components/flyout/flyout.tsx | 30 ++++++++++++++++++- .../header/__snapshots__/header.test.tsx.snap | 1 + src/components/header/header.tsx | 6 +++- upcoming_changelogs/6566.md | 1 + 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 upcoming_changelogs/6566.md diff --git a/src/components/flyout/flyout.tsx b/src/components/flyout/flyout.tsx index f1a8c75cff6..013355cd9cf 100644 --- a/src/components/flyout/flyout.tsx +++ b/src/components/flyout/flyout.tsx @@ -148,6 +148,14 @@ interface _EuiFlyoutProps { * `closeOnMouseup` will delay the close callback, allowing time for external toggle buttons to handle close behavior. */ focusTrapProps?: Pick; + /** + * By default, EuiFlyout will consider any fixed `EuiHeader`s that sit alongside or above the EuiFlyout + * as part of the flyout's focus trap. This prevents focus fighting with interactive elements + * within fixed headers. + * + * Set this to `false` if you need to disable this behavior for a specific reason. + */ + includeFixedHeadersInFocusTrap?: boolean; } const defaultElement = 'div'; @@ -186,7 +194,8 @@ export const EuiFlyout = forwardRef( outsideClickCloses, role = 'dialog', pushMinBreakpoint = 'l', - focusTrapProps, + focusTrapProps: _focusTrapProps = {}, + includeFixedHeadersInFocusTrap = true, ...rest }: EuiFlyoutProps, ref: @@ -317,6 +326,25 @@ export const EuiFlyout = forwardRef( ); } + /* + * If not disabled, automatically add fixed EuiHeaders as shards + * to EuiFlyout focus traps, to prevent focus fighting + */ + const [fixedHeaders, setFixedHeaders] = useState([]); + useEffect(() => { + if (includeFixedHeadersInFocusTrap) { + const fixedHeaderEls = document.querySelectorAll( + '.euiHeader[data-fixed-header]' + ); + setFixedHeaders(Array.from(fixedHeaderEls)); + } + }, [includeFixedHeadersInFocusTrap]); + + const focusTrapProps: EuiFlyoutProps['focusTrapProps'] = { + ..._focusTrapProps, + shards: [...fixedHeaders, ...(_focusTrapProps.shards || [])], + }; + const hasOverlayMask = ownFocus && !isPushed; const onClickOutside = (event: MouseEvent | TouchEvent) => { // Do not close the flyout for any external click diff --git a/src/components/header/__snapshots__/header.test.tsx.snap b/src/components/header/__snapshots__/header.test.tsx.snap index d2b31571712..35c4a3256dd 100644 --- a/src/components/header/__snapshots__/header.test.tsx.snap +++ b/src/components/header/__snapshots__/header.test.tsx.snap @@ -27,6 +27,7 @@ exports[`EuiHeader renders dark theme 1`] = ` exports[`EuiHeader renders in fixed position 1`] = `
Hello! diff --git a/src/components/header/header.tsx b/src/components/header/header.tsx index b0c39251d17..c5d856ba79e 100644 --- a/src/components/header/header.tsx +++ b/src/components/header/header.tsx @@ -148,7 +148,11 @@ export const EuiHeader: FunctionComponent = ({ } return ( -
+
{contents}
); diff --git a/upcoming_changelogs/6566.md b/upcoming_changelogs/6566.md new file mode 100644 index 00000000000..014fbb26063 --- /dev/null +++ b/upcoming_changelogs/6566.md @@ -0,0 +1 @@ +- `EuiFlyout`s now automatically shard all fixed `EuiHeader`s on the page. This means that interactions (mouse & keyboard) with items inside `EuiHeader`s when flyouts are open will no longer trigger focus fighting From f38b991e493687261b157518f7620d93c019c5e8 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 1 Feb 2023 16:32:43 -0800 Subject: [PATCH 2/7] Fix flyouts toggled from header shards not correctly focusing headers + a11y improvement: mirroring popovers & modals, make the flyout wrapper focusable via `tabIndex={0}`, so that screen readers take out a beat to read out the `labelledby` and (upcoming `describedby`) instead of jumping to the close button or the first focusable child (if no close button exists) --- .../collapsible_nav.test.tsx.snap | 27 ++-- .../flyout/__snapshots__/flyout.test.tsx.snap | 136 ++++++++++++++---- src/components/flyout/flyout.spec.tsx | 24 ++-- src/components/flyout/flyout.tsx | 15 +- 4 files changed, 153 insertions(+), 49 deletions(-) diff --git a/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap b/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap index bae474c35c2..6f2704c3141 100644 --- a/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap +++ b/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap @@ -13,9 +13,10 @@ Array [ >
,
,
+
+
+
+ + +`; diff --git a/src/components/flyout/flyout.spec.tsx b/src/components/flyout/flyout.spec.tsx index 881733b5be6..464df891200 100644 --- a/src/components/flyout/flyout.spec.tsx +++ b/src/components/flyout/flyout.spec.tsx @@ -42,23 +42,17 @@ const Flyout = ({ children = childrenDefault, ...rest }) => { describe('EuiFlyout', () => { describe('Focus behavior', () => { - it('focuses the close button by default', () => { + it('focuses the flyout wrapper by default', () => { cy.mount(); - cy.focused().should( - 'have.attr', - 'data-test-subj', - 'euiFlyoutCloseButton' - ); + cy.focused().should('have.class', 'euiFlyout'); + cy.focused().should('have.attr', 'data-autofocus', 'true'); }); it('traps focus and cycles tabbable items', () => { cy.mount(); - cy.realPress('Tab'); - cy.realPress('Tab'); - cy.realPress('Tab'); + cy.repeatRealPress('Tab', 4); cy.focused().should('have.attr', 'data-test-subj', 'itemC'); - cy.realPress('Tab'); - cy.realPress('Tab'); + cy.repeatRealPress('Tab', 3); cy.focused().should( 'have.attr', 'data-test-subj', @@ -70,9 +64,11 @@ describe('EuiFlyout', () => { describe('Close behavior: standard', () => { it('closes the flyout when the close button is clicked', () => { cy.mount(); - cy.realPress('Enter').then(() => { - expect(cy.get('[data-test-subj="flyoutSpec"]').should('not.exist')); - }); + cy.get('[data-test-subj="euiFlyoutCloseButton"]') + .click() + .then(() => { + expect(cy.get('[data-test-subj="flyoutSpec"]').should('not.exist')); + }); }); it('closes the flyout with `escape` key', () => { diff --git a/src/components/flyout/flyout.tsx b/src/components/flyout/flyout.tsx index 013355cd9cf..90f05ad2ae3 100644 --- a/src/components/flyout/flyout.tsx +++ b/src/components/flyout/flyout.tsx @@ -330,15 +330,25 @@ export const EuiFlyout = forwardRef( * If not disabled, automatically add fixed EuiHeaders as shards * to EuiFlyout focus traps, to prevent focus fighting */ + const flyoutToggle = useRef(document.activeElement); const [fixedHeaders, setFixedHeaders] = useState([]); + useEffect(() => { if (includeFixedHeadersInFocusTrap) { const fixedHeaderEls = document.querySelectorAll( '.euiHeader[data-fixed-header]' ); setFixedHeaders(Array.from(fixedHeaderEls)); + + // Flyouts that are toggled from fixed headers do not have working + // focus trap autoFocus, so we need to focus the flyout wrapper ourselves + fixedHeaderEls.forEach((header) => { + if (header.contains(flyoutToggle.current)) { + resizeRef?.focus(); + } + }); } - }, [includeFixedHeadersInFocusTrap]); + }, [includeFixedHeadersInFocusTrap, resizeRef]); const focusTrapProps: EuiFlyoutProps['focusTrapProps'] = { ..._focusTrapProps, @@ -383,7 +393,8 @@ export const EuiFlyout = forwardRef( {...(rest as ComponentPropsWithRef)} role={role} className={classes} - tabIndex={-1} + tabIndex={0} + data-autofocus style={newStyle} ref={setRef} > From 13285ddf0c765c67cfd62924c1cd248719313cc7 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Thu, 2 Feb 2023 09:41:36 -0800 Subject: [PATCH 3/7] Add screen reader instructions on interacting with the flyout - allow for custom `role`s that consumers pass in that aren't `dialog`s (likely very rare, may need more testing) --- .../collapsible_nav.test.tsx.snap | 63 ++++++- .../flyout/__snapshots__/flyout.test.tsx.snap | 165 +++++++++++++++++- src/components/flyout/flyout.tsx | 73 ++++++-- upcoming_changelogs/6566.md | 1 + 4 files changed, 278 insertions(+), 24 deletions(-) diff --git a/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap b/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap index 6f2704c3141..c1646d2866a 100644 --- a/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap +++ b/src/components/collapsible_nav/__snapshots__/collapsible_nav.test.tsx.snap @@ -12,14 +12,21 @@ Array [ data-focus-lock-disabled="false" >