From aff5dbd2d177acee7e92531e67c99a0a72e1f2ab Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Fri, 14 Jul 2023 12:48:32 -0700 Subject: [PATCH] [EuiFocusTrap] Allow `gapMode` and `crossFrame` props to be configured via `EuiProvider.componentDefaults` (#6942) * [cleanup] Fix EuiFocusTrap's props / props table - it's kind of a dumpster fire right now with many props not being picked up so I'm wholesale rearranging many things - storybook props/controls also throws its own special brand of fun into this * [cleanup] Update existing Storybook stories to account for new props controls - we can remove the specified `crossFrame` arg now that it actually exists as a prop - default `returnFocus` to a boolean, since it can also take a function - add an `onDeactivation` state update (useful when testing `clickOutsideDisables=true`) * [cleanup] Remove skipped focus trap Jest tests - these were already converted to Cypress tests, so there's no point / no need to keep them * [cleanup] Convert Jest tests to RTL + add a `shouldRenderCustomStyles` to confirm that react-focus-on accepts className/css/style props * Configure `EuiFocusTrap` to accept component defaults * Add Cypress test for `gapMode` * Add Storybook story for `crossFrame` & misc EuiProvider QA * changelog --- .../__snapshots__/focus_trap.test.tsx.snap | 12 +- src/components/focus_trap/focus_trap.spec.tsx | 23 ++- .../focus_trap/focus_trap.stories.tsx | 24 ++- src/components/focus_trap/focus_trap.test.tsx | 187 ++---------------- src/components/focus_trap/focus_trap.tsx | 71 +++++-- .../component_defaults/component_defaults.tsx | 7 +- upcoming_changelogs/6942.md | 1 + 7 files changed, 130 insertions(+), 195 deletions(-) create mode 100644 upcoming_changelogs/6942.md diff --git a/src/components/focus_trap/__snapshots__/focus_trap.test.tsx.snap b/src/components/focus_trap/__snapshots__/focus_trap.test.tsx.snap index 585aafd24f9..7739a1b314d 100644 --- a/src/components/focus_trap/__snapshots__/focus_trap.test.tsx.snap +++ b/src/components/focus_trap/__snapshots__/focus_trap.test.tsx.snap @@ -42,22 +42,22 @@ exports[`EuiFocusTrap can be disabled 1`] = ` `; -exports[`EuiFocusTrap is rendered 1`] = ` -Array [ +exports[`EuiFocusTrap renders 1`] = ` +
, + />
-
, +
, -] + /> +
`; diff --git a/src/components/focus_trap/focus_trap.spec.tsx b/src/components/focus_trap/focus_trap.spec.tsx index 148a86b6193..eb3e68470ef 100644 --- a/src/components/focus_trap/focus_trap.spec.tsx +++ b/src/components/focus_trap/focus_trap.spec.tsx @@ -11,9 +11,11 @@ /// import React, { ComponentType, useRef, useState } from 'react'; -import { EuiFocusTrap } from './focus_trap'; + import { EuiPortal } from '../portal'; +import { EuiFocusTrap } from './focus_trap'; + describe('EuiFocusTrap', () => { describe('focus', () => { it('is set on the first focusable element by default', () => { @@ -359,6 +361,25 @@ describe('EuiFocusTrap', () => { expect(styles.getPropertyValue('padding-right')).to.equal('0px'); }); }); + + it('allows customizing gapMode via EuiProvider.componentDefaults', () => { + cy.mount(, { + providerProps: { + componentDefaults: { EuiFocusTrap: { gapMode: 'margin' } }, + }, + }); + skipIfNoScrollbars(); + cy.get('[data-test-subj="openFocusTrap"]').click(); + + cy.get('body').then(($body) => { + const styles = window.getComputedStyle($body[0]); + + const margin = parseFloat(styles.getPropertyValue('margin-right')); + expect(margin).to.be.gt(0); + + expect(styles.getPropertyValue('padding-right')).to.equal('0px'); + }); + }); }); }); }); diff --git a/src/components/focus_trap/focus_trap.stories.tsx b/src/components/focus_trap/focus_trap.stories.tsx index 3cf4971498b..dca4e061921 100644 --- a/src/components/focus_trap/focus_trap.stories.tsx +++ b/src/components/focus_trap/focus_trap.stories.tsx @@ -14,6 +14,7 @@ import { EuiFieldText } from '../form'; import { EuiSpacer } from '../spacer'; import { EuiPanel } from '../panel'; +import { EuiProvider } from '../provider'; import { EuiFocusTrap, EuiFocusTrapProps } from './focus_trap'; const meta: Meta = { @@ -21,9 +22,7 @@ const meta: Meta = { // @ts-ignore This still works for Storybook controls, even though Typescript complains component: EuiFocusTrap, argTypes: { - crossFrame: { - control: { type: 'boolean' }, - }, + returnFocus: { type: 'boolean' }, }, }; @@ -39,7 +38,11 @@ const StatefulFocusTrap = (props: Partial) => { - + setDisabled(true)} + > Focus trap is currently {disabled ? 'disabled' : 'enabled'} Button inside focus trap @@ -74,3 +77,16 @@ export const Iframe: Story = { ), args: { disabled: true, crossFrame: false }, }; + +export const EuiProviderComponentDefaults: Story = { + render: ({ ...args }) => ( + + + + This story is passing all controls and their arguments to EuiProvider's + `componentDefaults` instead of to EuiFocusTrap directly. It's primarily + useful for testing that configured defaults behave the same way as + individual props. + + ), +}; diff --git a/src/components/focus_trap/focus_trap.test.tsx b/src/components/focus_trap/focus_trap.test.tsx index e5baa0d94cd..1f177dc3abc 100644 --- a/src/components/focus_trap/focus_trap.test.tsx +++ b/src/components/focus_trap/focus_trap.test.tsx @@ -6,29 +6,26 @@ * Side Public License, v 1. */ -import React, { EventHandler } from 'react'; -import { mount } from 'enzyme'; +import React from 'react'; import { render } from '../../test/rtl'; -import { findTestSubject, takeMountedSnapshot } from '../../test'; +import { shouldRenderCustomStyles } from '../../test/internal'; -import { EuiEvent } from '../outside_click_detector/outside_click_detector'; import { EuiFocusTrap } from './focus_trap'; -import { EuiPortal } from '../portal'; describe('EuiFocusTrap', () => { - test('is rendered', () => { - const component = mount( + shouldRenderCustomStyles(Test); + + it('renders', () => { + const { container } = render(
); - expect( - takeMountedSnapshot(component, { hasArrayOutput: true }) - ).toMatchSnapshot(); + expect(container).toMatchSnapshot(); }); - test('can be disabled', () => { + it('can be disabled', () => { const { container } = render(
@@ -38,7 +35,7 @@ describe('EuiFocusTrap', () => { expect(container).toMatchSnapshot(); }); - test('accepts className and style', () => { + it('accepts className and style', () => { const { container } = render(
@@ -50,8 +47,8 @@ describe('EuiFocusTrap', () => { describe('behavior', () => { describe('focus', () => { - test('is set on the first focusable element by default', () => { - const component = mount( + it('is set on the first focusable element by default', () => { + const { getByTestSubject } = render(
@@ -63,13 +60,11 @@ describe('EuiFocusTrap', () => {
); - expect(findTestSubject(component, 'input').getDOMNode()).toBe( - document.activeElement - ); + expect(getByTestSubject('input')).toBe(document.activeElement); }); - test('will blur focus when negating `autoFocus`', () => { - mount( + it('will blur focus when negating `autoFocus`', () => { + render(
@@ -84,8 +79,8 @@ describe('EuiFocusTrap', () => { expect(document.body).toBe(document.activeElement); }); - test('is set on the element identified by `data-autofocus`', () => { - const component = mount( + it('is set on the element identified by `data-autofocus`', () => { + const { getByTestSubject } = render(
@@ -97,155 +92,7 @@ describe('EuiFocusTrap', () => {
); - expect(findTestSubject(component, 'input2').getDOMNode()).toBe( - document.activeElement - ); - }); - }); - - // skipping because react-focus-on / react-focus-lock uses two handlers, - // one on the container to record what element was clicked and a second - // on the document, checking if the event target is the same on both - // because enzyme doesn't bubble the event, it is difficult to simulate - // the browser behaviour - we can revisit these tests when we have an - // actual browser environment - describe.skip('clickOutsideDisables', () => { - // enzyme doesn't mount the components into the global jsdom `document` - // but that's where the click detector listener is, - // pass the top-level mounted component's click event on to document - const triggerDocumentMouseDown: EventHandler = ( - e: React.MouseEvent - ) => { - const event = new Event('mousedown') as EuiEvent; - event.euiGeneratedBy = ( - e.nativeEvent as unknown as EuiEvent - ).euiGeneratedBy; - document.dispatchEvent(event); - }; - - const triggerDocumentMouseUp: EventHandler = ( - e: React.MouseEvent - ) => { - const event = new Event('mousedown') as EuiEvent; - event.euiGeneratedBy = ( - e.nativeEvent as unknown as EuiEvent - ).euiGeneratedBy; - document.dispatchEvent(event); - }; - - test('trap remains enabled when false', () => { - const component = mount( -
- -
- - -
- -
- ); - - // The existence of `data-focus-lock-disabled=false` indicates that the trap is enabled. - expect( - component.find('[data-focus-lock-disabled=false]').length - ).not.toBeLessThan(1); - findTestSubject(component, 'outside').simulate('mousedown'); - findTestSubject(component, 'outside').simulate('mouseup'); - // `react-focus-lock` relies on real DOM events to move focus about. - // Exposed attributes are the most consistent way to attain its state. - // See https://github.com/theKashey/react-focus-lock/blob/master/_tests/FocusLock.spec.js for the lib in use - // Trap remains enabled - expect( - component.find('[data-focus-lock-disabled=false]').length - ).not.toBeLessThan(1); - }); - - test('trap remains enabled after internal clicks', () => { - const component = mount( -
- -
- - -
- -
- ); - - expect( - component.find('[data-focus-lock-disabled=false]').length - ).not.toBeLessThan(1); - findTestSubject(component, 'input2').simulate('mousedown'); - findTestSubject(component, 'input2').simulate('mouseup'); - // Trap remains enabled - expect( - component.find('[data-focus-lock-disabled=false]').length - ).not.toBeLessThan(1); - }); - - test('trap remains enabled after internal portal clicks', () => { - const component = mount( -
- -
- - - - - -
- -
- ); - - expect( - component.find('[data-focus-lock-disabled=false]').length - ).not.toBeLessThan(1); - findTestSubject(component, 'input3').simulate('mousedown'); - findTestSubject(component, 'input3').simulate('mouseup'); - // Trap remains enabled - expect( - component.find('[data-focus-lock-disabled=false]').length - ).not.toBeLessThan(1); - }); - - test('trap becomes disabled on outside clicks', () => { - const component = mount( -
- -
- - -
- -
- ); - - expect( - component.find('[data-focus-lock-disabled=false]').length - ).not.toBeLessThan(1); - findTestSubject(component, 'outside').simulate('mousedown'); - findTestSubject(component, 'outside').simulate('mouseup'); - // Trap becomes disabled - expect(component.find('[data-focus-lock-disabled=false]').length).toBe( - 0 - ); + expect(getByTestSubject('input2')).toBe(document.activeElement); }); }); }); diff --git a/src/components/focus_trap/focus_trap.tsx b/src/components/focus_trap/focus_trap.tsx index 337ea03938f..52a68bca9eb 100644 --- a/src/components/focus_trap/focus_trap.tsx +++ b/src/components/focus_trap/focus_trap.tsx @@ -6,17 +6,40 @@ * Side Public License, v 1. */ -import React, { Component, CSSProperties } from 'react'; +import React, { Component, FunctionComponent, CSSProperties } from 'react'; import { FocusOn } from 'react-focus-on'; import { ReactFocusOnProps } from 'react-focus-on/dist/es5/types'; import { RemoveScrollBar } from 'react-remove-scroll-bar'; import { CommonProps } from '../common'; import { findElementBySelectorOrRef, ElementTarget } from '../../services'; +import { useEuiComponentDefaults } from '../provider/component_defaults'; export type FocusTarget = ElementTarget; -interface EuiFocusTrapInterface { +export type EuiFocusTrapProps = Omit< + ReactFocusOnProps, + // Inverted `disabled` prop used instead + | 'enabled' + // Omitted so that our props table & storybook actually register these props + | 'style' + | 'className' + | 'css' + // Props that differ from react-focus-on's default settings + | 'gapMode' + | 'crossFrame' + | 'scrollLock' + | 'noIsolation' + | 'returnFocus' +> & { + // For some reason, Storybook doesn't register these props if they're Pick<>'d + className?: CommonProps['className']; + css?: CommonProps['css']; + style?: CSSProperties; + /** + * @default false + */ + disabled?: boolean; /** * Whether `onClickOutside` should be called on mouseup instead of mousedown. * This flag can be used to prevent conflicts with outside toggle buttons by delaying the closing click callback. @@ -24,32 +47,58 @@ interface EuiFocusTrapInterface { closeOnMouseup?: boolean; /** * Clicking outside the trap area will disable the trap + * @default false */ clickOutsideDisables?: boolean; /** * Reference to element that will get focus when the trap is initiated */ initialFocus?: FocusTarget; - style?: CSSProperties; /** * if `scrollLock` is set to true, the body's scrollbar width will be preserved on lock * via the `gapMode` CSS property. Depending on your custom CSS, you may prefer to use * `margin` instead of `padding`. + * @default padding */ gapMode?: 'padding' | 'margin'; - disabled?: boolean; -} - -export interface EuiFocusTrapProps - extends CommonProps, - Omit, // Inverted `disabled` prop used instead - EuiFocusTrapInterface {} + /** + * Configures focus trapping between iframes. + * By default, EuiFocusTrap allows focus to leave iframes and move to elements outside of it. + * Set to `true` if you want focus to remain trapped within the iframe. + * @default false + */ + crossFrame?: ReactFocusOnProps['crossFrame']; + /** + * @default false + */ + scrollLock?: ReactFocusOnProps['scrollLock']; + /** + * @default true + */ + noIsolation?: ReactFocusOnProps['noIsolation']; + /** + * @default true + */ + returnFocus?: ReactFocusOnProps['returnFocus']; +}; + +export const EuiFocusTrap: FunctionComponent = ({ + children, + ...props +}) => { + const { EuiFocusTrap: defaults } = useEuiComponentDefaults(); + return ( + + {children} + + ); +}; interface State { hasBeenDisabledByClick: boolean; } -export class EuiFocusTrap extends Component { +class EuiFocusTrapClass extends Component { static defaultProps = { clickOutsideDisables: false, disabled: false, diff --git a/src/components/provider/component_defaults/component_defaults.tsx b/src/components/provider/component_defaults/component_defaults.tsx index 954508b323c..752fdbd8f3e 100644 --- a/src/components/provider/component_defaults/component_defaults.tsx +++ b/src/components/provider/component_defaults/component_defaults.tsx @@ -9,16 +9,17 @@ import React, { createContext, useContext, FunctionComponent } from 'react'; import { EuiPortalProps } from '../../portal'; +import { EuiFocusTrapProps } from '../../focus_trap'; export type EuiComponentDefaults = { /** - * Provide a global setting for EuiPortal's default insertion position. + * Provide a global configuration for EuiPortal's default insertion position. */ EuiPortal?: { insert: EuiPortalProps['insert'] }; /** - * TODO + * Provide a global configuration for EuiFocusTrap's `gapMode` and `crossFrame` props */ - EuiFocusTrap?: unknown; + EuiFocusTrap?: Pick; /** * TODO */ diff --git a/upcoming_changelogs/6942.md b/upcoming_changelogs/6942.md new file mode 100644 index 00000000000..1ae2f2c4d1a --- /dev/null +++ b/upcoming_changelogs/6942.md @@ -0,0 +1 @@ +- `EuiFocusTrap`'s `crossFrame` and `gapMode` props can now be configured globally via `EuiProvider.componentDefaults`