From a73bede3a40e3c6b56898eefcc4e711cd6f5640e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Sat, 2 Sep 2023 14:51:11 -0700 Subject: [PATCH 01/12] [REFACTOR] Split up accordion trigger into button & arrow sub-components - for easier dev readability and atomic styles + rename generic `iconButton` name to a more accurate `arrow` and remove className modifiers (use `[aria-expanded]` instead of `-isOpen` + inline some very minimal styles that don't require euiTheme or modifiers (trigger wrapper & optional action - action needs a label to trigger emotion snapshot serializer logic) --- .../__snapshots__/accordion.test.tsx.snap | 96 +++++------ src/components/accordion/accordion.styles.ts | 87 +--------- src/components/accordion/accordion.tsx | 155 +++--------------- .../accordion_arrow.styles.ts | 31 ++++ .../accordion_trigger/accordion_arrow.tsx | 53 ++++++ .../accordion_button.styles.ts | 64 ++++++++ .../accordion_trigger/accordion_button.tsx | 87 ++++++++++ .../accordion_trigger/accordion_trigger.tsx | 106 ++++++++++++ .../accordion/accordion_trigger/index.ts | 9 + .../collapsible_nav_accordion.test.tsx.snap | 8 +- 10 files changed, 424 insertions(+), 272 deletions(-) create mode 100644 src/components/accordion/accordion_trigger/accordion_arrow.styles.ts create mode 100644 src/components/accordion/accordion_trigger/accordion_arrow.tsx create mode 100644 src/components/accordion/accordion_trigger/accordion_button.styles.ts create mode 100644 src/components/accordion/accordion_trigger/accordion_button.tsx create mode 100644 src/components/accordion/accordion_trigger/accordion_trigger.tsx create mode 100644 src/components/accordion/accordion_trigger/index.ts diff --git a/src/components/accordion/__snapshots__/accordion.test.tsx.snap b/src/components/accordion/__snapshots__/accordion.test.tsx.snap index 79f97e06e33..8801e747060 100644 --- a/src/components/accordion/__snapshots__/accordion.test.tsx.snap +++ b/src/components/accordion/__snapshots__/accordion.test.tsx.snap @@ -5,13 +5,13 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = ` class="euiAccordion emotion-euiAccordion" >
{ return { @@ -36,54 +31,6 @@ export const euiAccordionStyles = ({ euiTheme }: UseEuiTheme) => { }; }; -export const euiAccordionButtonStyles = (euiThemeContext: UseEuiTheme) => { - const { euiTheme } = euiThemeContext; - return { - euiAccordion__button: css` - ${euiFontSize(euiThemeContext, 's')} - align-items: center; - display: flex; - flex-grow: 1; - line-height: ${euiTheme.size.l}; - ${logicalTextAlignCSS('left')} - ${logicalCSS('width', '100%')} - - &:hover, - &:focus { - cursor: pointer; - text-decoration: underline; - } - `, - // Triggering button needs separate `disabled` key because the element that renders may not support `:disabled`; - // Hover pseudo selector for specificity - disabled: css` - &, - &:hover { - cursor: not-allowed; - color: ${euiTheme.colors.disabledText}; - text-decoration: none; - } - `, - // Optional padding sizes - s: css` - padding: ${euiTheme.size.s}; - `, - m: css` - padding: ${euiTheme.size.base}; - `, - l: css` - padding: ${euiTheme.size.l}; - `, - // Remove padding from the accordion button on the side that the arrow is on - arrowLeft: css` - ${logicalCSS('padding-left', 0)} - `, - arrowRight: css` - ${logicalCSS('padding-left', 0)} - `, - }; -}; - export const euiAccordionChildrenStyles = ({ euiTheme }: UseEuiTheme) => ({ euiAccordion__children: css``, isLoading: css` @@ -128,40 +75,8 @@ export const euiAccordionChildWrapperStyles = ({ euiTheme }: UseEuiTheme) => ({ `, }); -export const euiAccordionIconButtonStyles = ({ euiTheme }: UseEuiTheme) => ({ - euiAccordion__iconButton: css` - flex-shrink: 0; - ${logicalCSS('margin-right', euiTheme.size.xs)} - transform: rotate( - 0deg - ) !important; /* stylelint-disable-line declaration-no-important */ - `, - isOpen: css` - transform: rotate( - 90deg - ) !important; /* stylelint-disable-line declaration-no-important */ - `, - arrowRight: css` - ${logicalCSS('margin-left', euiTheme.size.xs)} - ${logicalCSS('margin-right', 0)} - `, -}); - -export const euiAccordionOptionalActionStyles = () => ({ - euiAccordion__optionalAction: css` - flex-shrink: 0; - `, -}); - export const euiAccordionSpinnerStyles = ({ euiTheme }: UseEuiTheme) => ({ euiAccordion__spinner: css` ${logicalCSS('margin-right', euiTheme.size.xs)} `, }); - -export const euiAccordionTriggerWrapperStyles = () => ({ - euiAccordion__triggerWrapper: css` - align-items: center; - display: flex; - `, -}); diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index f4ea16ef960..4d57b559865 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -21,16 +21,13 @@ import { withEuiTheme, WithEuiThemeProps, } from '../../services'; -import { EuiButtonIcon, EuiButtonIconProps } from '../button'; +import type { EuiButtonIconProps } from '../button'; +import { EuiAccordionTrigger } from './accordion_trigger'; import { euiAccordionStyles, - euiAccordionButtonStyles, euiAccordionChildrenStyles, euiAccordionChildWrapperStyles, - euiAccordionIconButtonStyles, - euiAccordionOptionalActionStyles, euiAccordionSpinnerStyles, - euiAccordionTriggerWrapperStyles, } from './accordion.styles'; import { logicalCSS } from '../../global_styling'; @@ -279,28 +276,12 @@ export class EuiAccordionClass extends Component< isLoading, isLoadingMessage, isDisabled, - buttonProps: _buttonProps, - buttonElement: _ButtonElement = 'button', + buttonProps, + buttonElement, arrowProps, theme, ...rest } = this.props; - const { - paddingSize: buttonPaddingSize, - className: buttonPropsClassName, - css: buttonPropsCss, - ...buttonProps - } = _buttonProps || {}; - - // Force button element to be a legend if the element is a fieldset - const ButtonElement = Element === 'fieldset' ? 'legend' : _ButtonElement; - const buttonElementIsFocusable = ButtonElement === 'button'; - - // Force visibility of arrow button if button element is not focusable - const _arrowDisplay = - arrowDisplay === 'none' && !buttonElementIsFocusable - ? 'left' - : arrowDisplay; const classes = classNames( 'euiAccordion', @@ -321,41 +302,6 @@ export class EuiAccordionClass extends Component< 'euiAccordion__children-isLoading': isLoading, }); - const buttonClasses = classNames( - 'euiAccordion__button', - buttonClassName, - buttonPropsClassName - ); - - const buttonContentClasses = classNames( - 'euiAccordion__buttonContent', - buttonContentClassName - ); - - const iconButtonClasses = classNames( - 'euiAccordion__iconButton', - { - 'euiAccordion__iconButton-isOpen': this.isOpen, - 'euiAccordion__iconButton--right': _arrowDisplay === 'right', - }, - arrowProps?.className - ); - - // Emotion styles - const buttonStyles = euiAccordionButtonStyles(theme); - const cssButtonStyles = [ - buttonStyles.euiAccordion__button, - isDisabled && buttonStyles.disabled, - ...(buttonPaddingSize - ? [ - buttonStyles[buttonPaddingSize], - arrowDisplay === 'left' && buttonStyles.arrowLeft, - arrowDisplay === 'right' && buttonStyles.arrowRight, - ] - : []), - buttonPropsCss, - ]; - const childrenStyles = euiAccordionChildrenStyles(theme); const cssChildrenStyles = [ childrenStyles.euiAccordion__children, @@ -369,60 +315,9 @@ export class EuiAccordionClass extends Component< this.isOpen && childWrapperStyles.isOpen, ]; - const iconButtonStyles = euiAccordionIconButtonStyles(theme); - const cssIconButtonStyles = [ - iconButtonStyles.euiAccordion__iconButton, - this.isOpen && iconButtonStyles.isOpen, - _arrowDisplay === 'right' && iconButtonStyles.arrowRight, - arrowProps?.css, - ]; - - const optionalActionStyles = euiAccordionOptionalActionStyles(); - const cssOptionalActionStyles = [ - optionalActionStyles.euiAccordion__optionalAction, - ]; - const spinnerStyles = euiAccordionSpinnerStyles(theme); const cssSpinnerStyles = [spinnerStyles.euiAccordion__spinner]; - const triggerWrapperStyles = euiAccordionTriggerWrapperStyles(); - const cssTriggerWrapperStyles = [ - triggerWrapperStyles.euiAccordion__triggerWrapper, - ]; - - let iconButton; - const buttonId = buttonProps.id ?? this.generatedId; - if (_arrowDisplay !== 'none') { - iconButton = ( - - ); - } - - let optionalAction = null; - - if (isLoading || extraAction) { - optionalAction = ( -
- {isLoading ? : extraAction} -
- ); - } - let childrenContent: any; if (isLoading && isLoadingMessage) { childrenContent = ( @@ -446,34 +341,26 @@ export class EuiAccordionClass extends Component< childrenContent = children; } - const button = ( - - {buttonContent} - - ); + const buttonId = buttonProps?.id ?? this.generatedId; return ( -
- {_arrowDisplay === 'left' && iconButton} - {button} - {optionalAction} - {_arrowDisplay === 'right' && iconButton} -
+ : extraAction} + />
({ + euiAccordion__arrow: css` + flex-shrink: 0; + ${logicalCSS('margin-right', euiTheme.size.xs)} + transform: rotate( + 0deg + ) !important; /* stylelint-disable-line declaration-no-important */ + `, + isOpen: css` + transform: rotate( + 90deg + ) !important; /* stylelint-disable-line declaration-no-important */ + `, + right: css` + ${logicalCSS('margin-left', euiTheme.size.xs)} + ${logicalCSS('margin-right', 0)} + `, +}); diff --git a/src/components/accordion/accordion_trigger/accordion_arrow.tsx b/src/components/accordion/accordion_trigger/accordion_arrow.tsx new file mode 100644 index 00000000000..0d029cd7c9d --- /dev/null +++ b/src/components/accordion/accordion_trigger/accordion_arrow.tsx @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { FunctionComponent } from 'react'; +import classNames from 'classnames'; + +import { useEuiTheme } from '../../../services'; +import { EuiButtonIcon, EuiButtonIconPropsForButton } from '../../button'; + +import { EuiAccordionProps } from '../accordion'; +import { euiAccordionArrowStyles } from './accordion_arrow.styles'; + +type _EuiAccordionArrowProps = Partial & + Pick & { + isOpen: boolean; + }; + +export const EuiAccordionArrow: FunctionComponent<_EuiAccordionArrowProps> = ({ + arrowDisplay, + arrowProps, + isOpen, + ...rest +}) => { + const euiTheme = useEuiTheme(); + + if (arrowDisplay === 'none') return null; + + const styles = euiAccordionArrowStyles(euiTheme); + const cssStyles = [ + styles.euiAccordion__arrow, + isOpen && styles.isOpen, + arrowDisplay === 'right' && styles.right, + arrowProps?.css, + ]; + + const classes = classNames('euiAccordion__arrow', arrowProps?.className); + + return ( + + ); +}; diff --git a/src/components/accordion/accordion_trigger/accordion_button.styles.ts b/src/components/accordion/accordion_trigger/accordion_button.styles.ts new file mode 100644 index 00000000000..e0e743b27c8 --- /dev/null +++ b/src/components/accordion/accordion_trigger/accordion_button.styles.ts @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { css } from '@emotion/react'; + +import { UseEuiTheme } from '../../../services'; +import { + euiFontSize, + logicalCSS, + logicalTextAlignCSS, +} from '../../../global_styling'; + +export const euiAccordionButtonStyles = (euiThemeContext: UseEuiTheme) => { + const { euiTheme } = euiThemeContext; + return { + euiAccordion__button: css` + ${euiFontSize(euiThemeContext, 's')} + align-items: center; + display: flex; + flex-grow: 1; + line-height: ${euiTheme.size.l}; + ${logicalTextAlignCSS('left')} + ${logicalCSS('width', '100%')} + + &:hover, + &:focus { + cursor: pointer; + text-decoration: underline; + } + `, + // Triggering button needs separate `disabled` key because the element that renders may not support `:disabled`; + // Hover pseudo selector for specificity + disabled: css` + &, + &:hover { + cursor: not-allowed; + color: ${euiTheme.colors.disabledText}; + text-decoration: none; + } + `, + // Optional padding sizes + s: css` + padding: ${euiTheme.size.s}; + `, + m: css` + padding: ${euiTheme.size.base}; + `, + l: css` + padding: ${euiTheme.size.l}; + `, + // Remove padding from the accordion button on the side that the arrow is on + arrowLeft: css` + ${logicalCSS('padding-left', 0)} + `, + arrowRight: css` + ${logicalCSS('padding-left', 0)} + `, + }; +}; diff --git a/src/components/accordion/accordion_trigger/accordion_button.tsx b/src/components/accordion/accordion_trigger/accordion_button.tsx new file mode 100644 index 00000000000..b620d83af62 --- /dev/null +++ b/src/components/accordion/accordion_trigger/accordion_button.tsx @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { + FunctionComponent, + PropsWithChildren, + HTMLAttributes, +} from 'react'; +import classNames from 'classnames'; + +import { useEuiTheme } from '../../../services'; + +import { EuiAccordionProps } from '../accordion'; +import { euiAccordionButtonStyles } from './accordion_button.styles'; + +type _EuiAccordionButtonProps = PropsWithChildren & + HTMLAttributes & + Required> & + Pick< + EuiAccordionProps, + | 'buttonClassName' + | 'buttonProps' + | 'buttonContentClassName' + | 'isDisabled' + | 'arrowDisplay' + >; + +export const EuiAccordionButton: FunctionComponent< + _EuiAccordionButtonProps +> = ({ + buttonElement: ButtonElement, + buttonProps: _buttonProps, + buttonClassName, + buttonContentClassName, + isDisabled, + arrowDisplay, + children, + ...rest +}: _EuiAccordionButtonProps) => { + const { paddingSize, ...buttonProps } = _buttonProps || {}; + + const classes = classNames( + 'euiAccordion__button', + buttonClassName, + buttonProps.className + ); + + const buttonContentClasses = classNames( + 'euiAccordion__buttonContent', + buttonContentClassName + ); + + const euiTheme = useEuiTheme(); + const styles = euiAccordionButtonStyles(euiTheme); + const cssStyles = [ + styles.euiAccordion__button, + isDisabled && styles.disabled, + ...(paddingSize + ? [ + styles[paddingSize], + arrowDisplay === 'left' && styles.arrowLeft, + arrowDisplay === 'right' && styles.arrowRight, + ] + : []), + buttonProps.css, + ]; + + const elementIsButton = ButtonElement === 'button'; + + return ( + + {children} + + ); +}; diff --git a/src/components/accordion/accordion_trigger/accordion_trigger.tsx b/src/components/accordion/accordion_trigger/accordion_trigger.tsx new file mode 100644 index 00000000000..9f0cc29efef --- /dev/null +++ b/src/components/accordion/accordion_trigger/accordion_trigger.tsx @@ -0,0 +1,106 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { FunctionComponent, MouseEventHandler } from 'react'; + +import { EuiAccordionProps } from '../accordion'; +import { EuiAccordionButton } from './accordion_button'; +import { EuiAccordionArrow } from './accordion_arrow'; + +type _EuiAccordionTriggerProps = Pick< + EuiAccordionProps, + | 'arrowDisplay' + | 'arrowProps' + | 'buttonElement' + | 'buttonClassName' + | 'buttonProps' + | 'buttonContent' + | 'buttonContentClassName' + | 'extraAction' + | 'isDisabled' +> & { + isOpen: boolean; + ariaControlsId: string; + buttonId: string; + onToggle: MouseEventHandler; +}; + +export const EuiAccordionTrigger: FunctionComponent< + _EuiAccordionTriggerProps +> = ({ + arrowDisplay: _arrowDisplay, + arrowProps, + buttonElement = 'button', + buttonProps, + buttonClassName, + buttonContent, + buttonContentClassName, + buttonId, + ariaControlsId, + isDisabled, + isOpen, + onToggle, + extraAction, +}) => { + // Force visibility of arrow button icon if button element is not interactive + const buttonIsInteractive = buttonElement === 'button'; + const arrowDisplay = + _arrowDisplay === 'none' && !buttonIsInteractive ? 'left' : _arrowDisplay; + + const arrow = ( + + ); + + const button = ( + + {buttonContent} + + ); + + const optionalAction = extraAction && ( +
+ {extraAction} +
+ ); + + return ( +
+ {arrowDisplay === 'left' && arrow} + {button} + {optionalAction} + {arrowDisplay === 'right' && arrow} +
+ ); +}; diff --git a/src/components/accordion/accordion_trigger/index.ts b/src/components/accordion/accordion_trigger/index.ts new file mode 100644 index 00000000000..93bf06698f4 --- /dev/null +++ b/src/components/accordion/accordion_trigger/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export { EuiAccordionTrigger } from './accordion_trigger'; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap index 2879f5e504b..e31ff47fa4a 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap @@ -5,7 +5,7 @@ exports[`EuiCollapsibleNavAccordion renders as a sub item 1`] = ` class="euiAccordion euiCollapsibleNavAccordion emotion-euiAccordion-euiCollapsibleNavAccordion-isSubItem" >
@@ -205,14 +199,12 @@ exports[`EuiAccordion behavior opens when div is clicked if element is a div 1`] role="region" tabindex="-1" > -
-
-

- You can see me. -

-
+
+

+ You can see me. +

@@ -314,11 +306,9 @@ exports[`EuiAccordion is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -366,11 +356,9 @@ exports[`EuiAccordion isDisabled is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -401,11 +389,9 @@ exports[`EuiAccordion props arrowDisplay none is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -451,11 +437,9 @@ exports[`EuiAccordion props arrowDisplay right is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -503,11 +487,9 @@ exports[`EuiAccordion props arrowProps is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -557,11 +539,9 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -607,11 +587,9 @@ exports[`EuiAccordion props buttonContentClassName is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -655,11 +633,9 @@ exports[`EuiAccordion props buttonElement is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -707,11 +683,9 @@ exports[`EuiAccordion props buttonProps is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -758,11 +732,9 @@ exports[`EuiAccordion props buttonProps paddingSize l 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -809,11 +781,9 @@ exports[`EuiAccordion props buttonProps paddingSize m 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -860,11 +830,9 @@ exports[`EuiAccordion props buttonProps paddingSize s 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -908,11 +876,9 @@ exports[`EuiAccordion props element is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -965,11 +931,9 @@ exports[`EuiAccordion props extraAction is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -1015,14 +979,12 @@ exports[`EuiAccordion props forceState closed is rendered 1`] = ` role="region" tabindex="-1" > -
-
-

- You can not see me -

-
+
+

+ You can not see me +

@@ -1069,14 +1031,12 @@ exports[`EuiAccordion props forceState open is rendered 1`] = ` role="region" tabindex="-1" > -
-
-

- You can see me -

-
+
+

+ You can see me +

@@ -1123,14 +1083,12 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = ` role="region" tabindex="-1" > -
-
-

- You can see me. -

-
+
+

+ You can see me. +

@@ -1186,11 +1144,9 @@ exports[`EuiAccordion props isLoading is rendered 1`] = ` role="region" tabindex="-1" > -
-
-
+
`; @@ -1245,22 +1201,20 @@ exports[`EuiAccordion props isLoadingMessage is rendered 1`] = ` role="region" tabindex="-1" > -
+
+
diff --git a/src/components/accordion/accordion.styles.ts b/src/components/accordion/accordion.styles.ts index cb61bbe2603..5b497e77b9d 100644 --- a/src/components/accordion/accordion.styles.ts +++ b/src/components/accordion/accordion.styles.ts @@ -7,8 +7,9 @@ */ import { css } from '@emotion/react'; + import { UseEuiTheme } from '../../services'; -import { logicals, logicalCSS } from '../../global_styling'; +import { logicalCSS } from '../../global_styling'; export const euiAccordionStyles = ({ euiTheme }: UseEuiTheme) => { return { @@ -30,53 +31,3 @@ export const euiAccordionStyles = ({ euiTheme }: UseEuiTheme) => { }, }; }; - -export const euiAccordionChildrenStyles = ({ euiTheme }: UseEuiTheme) => ({ - euiAccordion__children: css``, - isLoading: css` - align-items: center; - display: flex; - `, - xs: css` - padding: ${euiTheme.size.xs}; - `, - s: css` - padding: ${euiTheme.size.s}; - `, - m: css` - padding: ${euiTheme.size.base}; - `, - l: css` - padding: ${euiTheme.size.l}; - `, - xl: css` - padding: ${euiTheme.size.xl}; - `, -}); - -export const euiAccordionChildWrapperStyles = ({ euiTheme }: UseEuiTheme) => ({ - euiAccordion__childWrapper: css` - ${logicalCSS('height', 0)} - opacity: 0; - overflow: hidden; - transition: ${logicals.height} ${euiTheme.animation.normal} - ${euiTheme.animation.resistance}, - opacity ${euiTheme.animation.normal} ${euiTheme.animation.resistance}; - visibility: hidden; - - &:focus { - outline: none; /* Hide focus ring because of tabindex=-1 on Safari */ - } - `, - isOpen: css` - ${logicalCSS('height', 'auto')} - opacity: 1; - visibility: visible; - `, -}); - -export const euiAccordionSpinnerStyles = ({ euiTheme }: UseEuiTheme) => ({ - euiAccordion__spinner: css` - ${logicalCSS('margin-right', euiTheme.size.xs)} - `, -}); diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index 6a6b5d0f537..d51c3a9947b 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -8,9 +8,7 @@ import React, { Component, HTMLAttributes, ReactNode } from 'react'; import classNames from 'classnames'; -import { tabbable, FocusableElement } from 'tabbable'; -import { logicalCSS } from '../../global_styling'; import { htmlIdGenerator, withEuiTheme, @@ -18,18 +16,11 @@ import { } from '../../services'; import { CommonProps } from '../common'; import { EuiLoadingSpinner } from '../loading'; -import { EuiResizeObserver } from '../observer/resize_observer'; -import { EuiText } from '../text'; -import { EuiI18n } from '../i18n'; import type { EuiButtonIconProps } from '../button'; import { EuiAccordionTrigger } from './accordion_trigger'; -import { - euiAccordionStyles, - euiAccordionChildrenStyles, - euiAccordionChildWrapperStyles, - euiAccordionSpinnerStyles, -} from './accordion.styles'; +import { EuiAccordionChildren } from './accordion_children'; +import { euiAccordionStyles } from './accordion.styles'; export const PADDING_SIZES = ['none', 'xs', 's', 'm', 'l', 'xl'] as const; export type EuiAccordionPaddingSize = (typeof PADDING_SIZES)[number]; @@ -135,10 +126,6 @@ export class EuiAccordionClass extends Component< buttonElement: 'button' as const, }; - childContent: HTMLDivElement | null = null; - childWrapper: HTMLDivElement | null = null; - tabbableChildren: FocusableElement[] | null = null; - state = { isOpen: this.props.forceState ? this.props.forceState === 'open' @@ -151,43 +138,6 @@ export class EuiAccordionClass extends Component< : this.state.isOpen; } - setChildContentHeight = () => { - requestAnimationFrame(() => { - const height = - this.childContent && this.isOpen ? this.childContent.clientHeight : 0; - this.childWrapper && - this.childWrapper.setAttribute( - 'style', - logicalCSS('height', `${height}px`) - ); - }); - }; - - componentDidMount() { - this.setChildContentHeight(); - if (!this.isOpen) this.preventTabbing(); - } - - componentDidUpdate( - prevProps: EuiAccordionProps, - prevState: EuiAccordionState - ) { - this.setChildContentHeight(); - - if ( - (prevProps.forceState === 'open' && this.props.forceState === 'closed') || - (prevState.isOpen === true && this.state.isOpen === false) - ) { - this.preventTabbing(); - } - if ( - (prevProps.forceState === 'closed' && this.props.forceState === 'open') || - (prevState.isOpen === false && this.state.isOpen === true) - ) { - this.enableTabbing(); - } - } - onToggle = () => { const { forceState } = this.props; if (forceState) { @@ -198,66 +148,14 @@ export class EuiAccordionClass extends Component< isOpen: !prevState.isOpen, }), () => { - if (this.state.isOpen && this.childWrapper) { - this.childWrapper.focus(); - } this.props.onToggle?.(this.state.isOpen); } ); } }; - // When accordions are closed, tabbable children should not be present in the tab order - preventTabbing = () => { - if (this.childContent) { - // Re-check for children on every close - content can change dynamically - this.tabbableChildren = tabbable(this.childContent); - - this.tabbableChildren.forEach((element) => { - // If the element has an existing `tabIndex` set, make sure we can restore it - const originalTabIndex = element.getAttribute('tabIndex'); - if (originalTabIndex) { - element.setAttribute('data-original-tabindex', originalTabIndex); - } - - element.setAttribute('tabIndex', '-1'); - }); - } - }; - - enableTabbing = () => { - // If no tabbable children were set, we don't need to re-enable anything - if (this.tabbableChildren) { - this.tabbableChildren.forEach((element) => { - const originalTabIndex = element.getAttribute('data-original-tabindex'); - if (originalTabIndex) { - // If the element originally had an existing `tabIndex` set, restore it - element.setAttribute('tabIndex', originalTabIndex); - element.removeAttribute('data-original-tabindex'); - } else { - // If not, remove the tabIndex property - element.removeAttribute('tabIndex'); - } - }); - // Cleanup - unset the list of children - this.tabbableChildren = null; - } - }; - - setChildContentRef = (node: HTMLDivElement | null) => { - this.childContent = node; - }; - generatedId = htmlIdGenerator()(); - // Storing resize/observer refs as an instance variable is a performance optimization - // and also resolves https://github.com/elastic/eui/issues/5903 - resizeRef: (e: HTMLElement | null) => void = () => {}; - observerRef = (ref: HTMLDivElement) => { - this.setChildContentRef(ref); - this.resizeRef(ref); - }; - render() { const { children, @@ -296,49 +194,6 @@ export class EuiAccordionClass extends Component< borders !== 'none' && styles.borders[borders!], ]; - const childrenClasses = classNames('euiAccordion__children', { - 'euiAccordion__children-isLoading': isLoading, - }); - - const childrenStyles = euiAccordionChildrenStyles(theme); - const cssChildrenStyles = [ - childrenStyles.euiAccordion__children, - isLoading && childrenStyles.isLoading, - paddingSize && paddingSize !== 'none' && childrenStyles[paddingSize], - ]; - - const childWrapperStyles = euiAccordionChildWrapperStyles(theme); - const cssChildWrapperStyles = [ - childWrapperStyles.euiAccordion__childWrapper, - this.isOpen && childWrapperStyles.isOpen, - ]; - - const spinnerStyles = euiAccordionSpinnerStyles(theme); - const cssSpinnerStyles = [spinnerStyles.euiAccordion__spinner]; - - let childrenContent: any; - if (isLoading && isLoadingMessage) { - childrenContent = ( - <> - - -

- {isLoadingMessage !== true ? ( - isLoadingMessage - ) : ( - - )} -

- - - ); - } else { - childrenContent = children; - } - const buttonId = buttonProps?.id ?? this.generatedId; return ( @@ -360,30 +215,16 @@ export class EuiAccordionClass extends Component< extraAction={isLoading ? : extraAction} /> -
{ - this.childWrapper = node; - }} - tabIndex={-1} - role="region" - aria-labelledby={buttonId} + - - {(resizeRef) => { - this.resizeRef = resizeRef; - return ( -
-
- {childrenContent} -
-
- ); - }} - -
+ {children} + ); } diff --git a/src/components/accordion/accordion_children/accordion_children.styles.ts b/src/components/accordion/accordion_children/accordion_children.styles.ts new file mode 100644 index 00000000000..814fa6a7d06 --- /dev/null +++ b/src/components/accordion/accordion_children/accordion_children.styles.ts @@ -0,0 +1,56 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { css } from '@emotion/react'; + +import { UseEuiTheme } from '../../../services'; +import { logicals, logicalCSS } from '../../../global_styling'; + +export const euiAccordionChildrenStyles = ({ euiTheme }: UseEuiTheme) => ({ + euiAccordion__children: css``, + isLoading: css` + align-items: center; + display: flex; + `, + xs: css` + padding: ${euiTheme.size.xs}; + `, + s: css` + padding: ${euiTheme.size.s}; + `, + m: css` + padding: ${euiTheme.size.base}; + `, + l: css` + padding: ${euiTheme.size.l}; + `, + xl: css` + padding: ${euiTheme.size.xl}; + `, +}); + +export const euiAccordionChildWrapperStyles = ({ euiTheme }: UseEuiTheme) => ({ + euiAccordion__childWrapper: css` + ${logicalCSS('height', 0)} + opacity: 0; + overflow: hidden; + transition: ${logicals.height} ${euiTheme.animation.normal} + ${euiTheme.animation.resistance}, + opacity ${euiTheme.animation.normal} ${euiTheme.animation.resistance}; + visibility: hidden; + + &:focus { + outline: none; /* Hide focus ring because of tabindex=-1 on Safari */ + } + `, + isOpen: css` + ${logicalCSS('height', 'auto')} + opacity: 1; + visibility: visible; + `, +}); diff --git a/src/components/accordion/accordion_children/accordion_children.tsx b/src/components/accordion/accordion_children/accordion_children.tsx new file mode 100644 index 00000000000..ee27a84b12e --- /dev/null +++ b/src/components/accordion/accordion_children/accordion_children.tsx @@ -0,0 +1,171 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { + FunctionComponent, + HTMLAttributes, + useRef, + useCallback, + useEffect, +} from 'react'; +import classNames from 'classnames'; +import { tabbable, FocusableElement } from 'tabbable'; + +import { useEuiTheme } from '../../../services'; +import { EuiResizeObserver } from '../../observer/resize_observer'; + +import { EuiAccordionProps } from '../accordion'; +import { EuiAccordionChildrenLoading } from './accordion_children_loading'; +import { + euiAccordionChildrenStyles, + euiAccordionChildWrapperStyles, +} from './accordion_children.styles'; + +type _EuiAccordionChildrenProps = HTMLAttributes & + Pick< + EuiAccordionProps, + 'children' | 'paddingSize' | 'isLoading' | 'isLoadingMessage' + > & { + isOpen: boolean; + }; +export const EuiAccordionChildren: FunctionComponent< + _EuiAccordionChildrenProps +> = ({ + children, + paddingSize, + isLoading, + isLoadingMessage, + isOpen, + ...rest +}) => { + /** + * Children + */ + const classes = classNames('euiAccordion__children', { + 'euiAccordion__children-isLoading': isLoading, + }); + + const euiTheme = useEuiTheme(); + const styles = euiAccordionChildrenStyles(euiTheme); + const cssStyles = [ + styles.euiAccordion__children, + isLoading && styles.isLoading, + paddingSize && paddingSize !== 'none' && styles[paddingSize], + ]; + + const contentRef = useRef(null); + + /** + * Wrapper + */ + const wrapperStyles = euiAccordionChildWrapperStyles(euiTheme); + const wrapperCssStyles = [ + wrapperStyles.euiAccordion__childWrapper, + isOpen && wrapperStyles.isOpen, + ]; + + const wrapperRef = useRef(null); + + /** + * Update the accordion wrapper height whenever the accordion opens, and also + * whenever the child content updates (which will change the height) + */ + const setAccordionHeight = useCallback(() => { + requestAnimationFrame(() => { + if (!contentRef.current || !wrapperRef.current) return; + + const height = isOpen ? contentRef.current!.clientHeight : 0; + wrapperRef.current.style.blockSize = `${height}px`; + }); + }, [isOpen]); + + useEffect(() => { + setAccordionHeight(); + }, [setAccordionHeight, children]); + + /** + * Focus the children wrapper on open + */ + useEffect(() => { + if (isOpen) wrapperRef.current?.focus(); + }, [isOpen]); + + /** + * Ensure accordion children are correctly removed from tabindex order + * when accordions are closed, and correctly restored on open + */ + const tabbableChildren = useRef(null); + + useEffect(() => { + // When accordions are closed, tabbable children should not be present in the tab order + if (!isOpen) { + // Re-check for children on every close - content can change dynamically + tabbableChildren.current = tabbable(contentRef.current!); + + tabbableChildren.current.forEach((element) => { + // If the element has an existing `tabIndex` set, make sure we can restore it + const originalTabIndex = element.getAttribute('tabIndex'); + if (originalTabIndex) { + element.setAttribute('data-original-tabindex', originalTabIndex); + } + + element.setAttribute('tabIndex', '-1'); + }); + } else { + // On open, restore tabbable children + // If no tabbable children were set, we don't need to re-enable anything + if (!tabbableChildren.current) return; + + tabbableChildren.current.forEach((element) => { + const originalTabIndex = element.getAttribute('data-original-tabindex'); + if (originalTabIndex) { + // If the element originally had an existing `tabIndex` set, restore it + element.setAttribute('tabIndex', originalTabIndex); + element.removeAttribute('data-original-tabindex'); + } else { + // If not, remove the tabIndex property + element.removeAttribute('tabIndex'); + } + }); + // Cleanup - unset the list of children + tabbableChildren.current = null; + } + }, [isOpen]); + + return ( +
+ + {(resizeRef) => ( +
{ + resizeRef(node); + contentRef.current = node; + }} + className={classes} + css={cssStyles} + > + {isLoading && isLoadingMessage ? ( + + ) : ( + children + )} +
+ )} + +
+ ); +}; diff --git a/src/components/accordion/accordion_children/accordion_children_loading.tsx b/src/components/accordion/accordion_children/accordion_children_loading.tsx new file mode 100644 index 00000000000..2574d08725f --- /dev/null +++ b/src/components/accordion/accordion_children/accordion_children_loading.tsx @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { FunctionComponent } from 'react'; + +import { useEuiTheme } from '../../../services'; +import { EuiI18n } from '../../i18n'; +import { EuiText } from '../../text'; +import { EuiLoadingSpinner } from '../../loading'; + +import { EuiAccordionProps } from '../accordion'; + +type _EuiAccordionChildrenLoadingProps = Pick< + EuiAccordionProps, + 'isLoadingMessage' +>; + +export const EuiAccordionChildrenLoading: FunctionComponent< + _EuiAccordionChildrenLoadingProps +> = ({ isLoadingMessage }) => { + const { euiTheme } = useEuiTheme(); + + return ( + <> + + +

+ {isLoadingMessage !== true ? ( + isLoadingMessage + ) : ( + + )} +

+ + + ); +}; diff --git a/src/components/accordion/accordion_children/index.ts b/src/components/accordion/accordion_children/index.ts new file mode 100644 index 00000000000..5690e4d6509 --- /dev/null +++ b/src/components/accordion/accordion_children/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export { EuiAccordionChildren } from './accordion_children'; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap index e31ff47fa4a..0647ee26638 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap @@ -45,23 +45,21 @@ exports[`EuiCollapsibleNavAccordion renders as a sub item 1`] = ` role="region" tabindex="-1" > -
+
-
- - sub item - + sub item -
+
@@ -115,23 +113,21 @@ exports[`EuiCollapsibleNavAccordion renders as a top level item 1`] = ` role="region" tabindex="-1" > -
+
-
- - sub item - + sub item -
+
From f7773f4da175edea7bd91a57b128035e8ffa4f3e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 4 Sep 2023 11:35:58 -0700 Subject: [PATCH 05/12] [refactor] Switch to styles obj instead of modifying a DOM element directly - this feels more react-y and has the benefit of not requiring an extra useEffect on mount --- .../__snapshots__/accordion.test.tsx.snap | 23 +++++++++++ .../accordion_children/accordion_children.tsx | 39 +++++++------------ .../collapsible_nav_accordion.test.tsx.snap | 2 + 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/components/accordion/__snapshots__/accordion.test.tsx.snap b/src/components/accordion/__snapshots__/accordion.test.tsx.snap index 0ba05d6ed9c..7e5f6ece005 100644 --- a/src/components/accordion/__snapshots__/accordion.test.tsx.snap +++ b/src/components/accordion/__snapshots__/accordion.test.tsx.snap @@ -39,6 +39,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = ` class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper" id="25" role="region" + style="block-size: 0;" tabindex="-1" >
(null); - /** * Wrapper */ @@ -75,18 +75,15 @@ export const EuiAccordionChildren: FunctionComponent< * Update the accordion wrapper height whenever the accordion opens, and also * whenever the child content updates (which will change the height) */ - const setAccordionHeight = useCallback(() => { - requestAnimationFrame(() => { - if (!contentRef.current || !wrapperRef.current) return; - - const height = isOpen ? contentRef.current!.clientHeight : 0; - wrapperRef.current.style.blockSize = `${height}px`; - }); - }, [isOpen]); - - useEffect(() => { - setAccordionHeight(); - }, [setAccordionHeight, children]); + const [contentHeight, setContentHeight] = useState(0); + const onResize = useCallback( + ({ height }: { height: number }) => setContentHeight(Math.round(height)), + [] + ); + const heightInlineStyle = useMemo( + () => ({ blockSize: isOpen ? contentHeight : 0 }), + [isOpen, contentHeight] + ); /** * Focus the children wrapper on open @@ -105,7 +102,7 @@ export const EuiAccordionChildren: FunctionComponent< // When accordions are closed, tabbable children should not be present in the tab order if (!isOpen) { // Re-check for children on every close - content can change dynamically - tabbableChildren.current = tabbable(contentRef.current!); + tabbableChildren.current = tabbable(wrapperRef.current!); tabbableChildren.current.forEach((element) => { // If the element has an existing `tabIndex` set, make sure we can restore it @@ -142,20 +139,14 @@ export const EuiAccordionChildren: FunctionComponent< {...rest} className="euiAccordion__childWrapper" css={wrapperCssStyles} + style={heightInlineStyle} ref={wrapperRef} tabIndex={-1} role="region" > - + {(resizeRef) => ( -
{ - resizeRef(node); - contentRef.current = node; - }} - className={classes} - css={cssStyles} - > +
{isLoading && isLoadingMessage ? (
Date: Mon, 4 Sep 2023 12:27:56 -0700 Subject: [PATCH 06/12] [refactor] Remove manual tabbable controls and use `inert` attribute instead - see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/inert - this property completely does what we want and full browser support was added in early 2023 - also allows us to remove `visibility` CSS which we were using to hide content from screen readers --- .../__snapshots__/accordion.test.tsx.snap | 72 +++++-------------- src/components/accordion/accordion.test.tsx | 36 +--------- .../accordion_children.styles.ts | 2 - .../accordion_children/accordion_children.tsx | 47 +----------- .../collapsible_nav_accordion.test.tsx.snap | 2 + 5 files changed, 25 insertions(+), 134 deletions(-) diff --git a/src/components/accordion/__snapshots__/accordion.test.tsx.snap b/src/components/accordion/__snapshots__/accordion.test.tsx.snap index 7e5f6ece005..f6594d73be1 100644 --- a/src/components/accordion/__snapshots__/accordion.test.tsx.snap +++ b/src/components/accordion/__snapshots__/accordion.test.tsx.snap @@ -38,6 +38,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = ` aria-labelledby="generated-id" class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper" id="25" + inert="" role="region" style="block-size: 0;" tabindex="-1" @@ -93,6 +94,7 @@ exports[`EuiAccordion behavior does not open when isDisabled 1`] = ` aria-labelledby="generated-id" class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper" id="23" + inert="" role="region" style="block-size: 0;" tabindex="-1" @@ -214,59 +216,6 @@ exports[`EuiAccordion behavior opens when div is clicked if element is a div 1`]
`; -exports[`EuiAccordion behavior restores tabbable children on accordion open 1`] = ` -
- - - - tabbable item three - -
- tabbable item four -
-
-`; - -exports[`EuiAccordion behavior sets tabbable children to \`tabIndex={-1}\` when accordions are closed 1`] = ` -
- - - - tabbable item three - -
- tabbable item four -
-
-`; - exports[`EuiAccordion is rendered 1`] = `
{ ); expect(container.firstChild).toMatchSnapshot(); + expect(container.firstChild).not.toHaveAttribute('inert'); }); }); @@ -323,39 +323,5 @@ describe('EuiAccordion', () => { expect(childWrapper).toBe(document.activeElement); }); - - it('sets tabbable children to `tabIndex={-1}` when accordions are closed', () => { - const { container } = render( - - - - tabbable item three -
tabbable item four
- - ); - const children = container.querySelector('.euiAccordion__children')!; - - expect(children.querySelectorAll('[tabindex="-1"]')).toHaveLength(4); - expect(children).toMatchSnapshot(); - }); - - it('restores tabbable children on accordion open', () => { - const { container, getByTestSubject } = render( - - - - tabbable item three -
tabbable item four
- - ); - fireEvent.click(getByTestSubject('trigger')); - const children = container.querySelector('.euiAccordion__children')!; - - expect(children.querySelectorAll('[tabindex="-1"]')).toHaveLength(0); - expect(children).toMatchSnapshot(); - }); }); }); diff --git a/src/components/accordion/accordion_children/accordion_children.styles.ts b/src/components/accordion/accordion_children/accordion_children.styles.ts index 814fa6a7d06..e85f46c238e 100644 --- a/src/components/accordion/accordion_children/accordion_children.styles.ts +++ b/src/components/accordion/accordion_children/accordion_children.styles.ts @@ -42,7 +42,6 @@ export const euiAccordionChildWrapperStyles = ({ euiTheme }: UseEuiTheme) => ({ transition: ${logicals.height} ${euiTheme.animation.normal} ${euiTheme.animation.resistance}, opacity ${euiTheme.animation.normal} ${euiTheme.animation.resistance}; - visibility: hidden; &:focus { outline: none; /* Hide focus ring because of tabindex=-1 on Safari */ @@ -51,6 +50,5 @@ export const euiAccordionChildWrapperStyles = ({ euiTheme }: UseEuiTheme) => ({ isOpen: css` ${logicalCSS('height', 'auto')} opacity: 1; - visibility: visible; `, }); diff --git a/src/components/accordion/accordion_children/accordion_children.tsx b/src/components/accordion/accordion_children/accordion_children.tsx index bf94495b17e..c85d11df45c 100644 --- a/src/components/accordion/accordion_children/accordion_children.tsx +++ b/src/components/accordion/accordion_children/accordion_children.tsx @@ -16,7 +16,6 @@ import React, { useState, } from 'react'; import classNames from 'classnames'; -import { tabbable, FocusableElement } from 'tabbable'; import { useEuiTheme } from '../../../services'; import { EuiResizeObserver } from '../../observer/resize_observer'; @@ -92,48 +91,6 @@ export const EuiAccordionChildren: FunctionComponent< if (isOpen) wrapperRef.current?.focus(); }, [isOpen]); - /** - * Ensure accordion children are correctly removed from tabindex order - * when accordions are closed, and correctly restored on open - */ - const tabbableChildren = useRef(null); - - useEffect(() => { - // When accordions are closed, tabbable children should not be present in the tab order - if (!isOpen) { - // Re-check for children on every close - content can change dynamically - tabbableChildren.current = tabbable(wrapperRef.current!); - - tabbableChildren.current.forEach((element) => { - // If the element has an existing `tabIndex` set, make sure we can restore it - const originalTabIndex = element.getAttribute('tabIndex'); - if (originalTabIndex) { - element.setAttribute('data-original-tabindex', originalTabIndex); - } - - element.setAttribute('tabIndex', '-1'); - }); - } else { - // On open, restore tabbable children - // If no tabbable children were set, we don't need to re-enable anything - if (!tabbableChildren.current) return; - - tabbableChildren.current.forEach((element) => { - const originalTabIndex = element.getAttribute('data-original-tabindex'); - if (originalTabIndex) { - // If the element originally had an existing `tabIndex` set, restore it - element.setAttribute('tabIndex', originalTabIndex); - element.removeAttribute('data-original-tabindex'); - } else { - // If not, remove the tabIndex property - element.removeAttribute('tabIndex'); - } - }); - // Cleanup - unset the list of children - tabbableChildren.current = null; - } - }, [isOpen]); - return (
{(resizeRef) => ( diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap index 475e3a120d5..047648833f1 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap @@ -42,6 +42,7 @@ exports[`EuiCollapsibleNavAccordion renders as a sub item 1`] = ` aria-labelledby="generated-id" class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper" id="generated-id" + inert="" role="region" style="block-size: 0;" tabindex="-1" @@ -111,6 +112,7 @@ exports[`EuiCollapsibleNavAccordion renders as a top level item 1`] = ` aria-labelledby="generated-id" class="euiAccordion__childWrapper emotion-euiAccordion__childWrapper" id="generated-id" + inert="" role="region" style="block-size: 0;" tabindex="-1" From 8567a31bf6e3e698dab6f775f5d12dbee3d6c84d Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 4 Sep 2023 12:45:20 -0700 Subject: [PATCH 07/12] [opinionated refactor] Clean up child wrapper styles - remove CSS overrides in favor of two separate states - add missing motion media query around height/opacity transition - remove complete focus removal in favor of `:focus-visible` selector and existing mixin --- .../__snapshots__/accordion.test.tsx.snap | 38 ++++++------- .../accordion_children.styles.ts | 53 ++++++++++++------- .../accordion_children/accordion_children.tsx | 2 +- .../collapsible_nav_accordion.test.tsx.snap | 4 +- 4 files changed, 57 insertions(+), 40 deletions(-) diff --git a/src/components/accordion/__snapshots__/accordion.test.tsx.snap b/src/components/accordion/__snapshots__/accordion.test.tsx.snap index f6594d73be1..5b82699100e 100644 --- a/src/components/accordion/__snapshots__/accordion.test.tsx.snap +++ b/src/components/accordion/__snapshots__/accordion.test.tsx.snap @@ -36,7 +36,7 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
({ euiAccordion__children: css``, @@ -34,21 +39,33 @@ export const euiAccordionChildrenStyles = ({ euiTheme }: UseEuiTheme) => ({ `, }); -export const euiAccordionChildWrapperStyles = ({ euiTheme }: UseEuiTheme) => ({ - euiAccordion__childWrapper: css` - ${logicalCSS('height', 0)} - opacity: 0; - overflow: hidden; - transition: ${logicals.height} ${euiTheme.animation.normal} - ${euiTheme.animation.resistance}, - opacity ${euiTheme.animation.normal} ${euiTheme.animation.resistance}; +export const euiAccordionChildWrapperStyles = ( + euiThemeContext: UseEuiTheme +) => { + const { euiTheme } = euiThemeContext; + return { + euiAccordion__childWrapper: css` + overflow: hidden; - &:focus { - outline: none; /* Hide focus ring because of tabindex=-1 on Safari */ - } - `, - isOpen: css` - ${logicalCSS('height', 'auto')} - opacity: 1; - `, -}); + ${euiCanAnimate} { + transition: ${logicals.height} ${euiTheme.animation.normal} + ${euiTheme.animation.resistance}, + opacity ${euiTheme.animation.normal} ${euiTheme.animation.resistance}; + } + + /* NOTE: Safari is slightly flaky about showing the focus-visible outline + on click when it should only show on keyboard enter. However, the minor + visual impact of this is not worth the accessibility loss to keyboard + users on Chrome & FF */ + ${euiFocusRing(euiThemeContext)} + `, + isClosed: css` + ${logicalCSS('height', 0)} + opacity: 0; + `, + isOpen: css` + ${logicalCSS('height', 'auto')} + opacity: 1; + `, + }; +}; diff --git a/src/components/accordion/accordion_children/accordion_children.tsx b/src/components/accordion/accordion_children/accordion_children.tsx index c85d11df45c..f9845690973 100644 --- a/src/components/accordion/accordion_children/accordion_children.tsx +++ b/src/components/accordion/accordion_children/accordion_children.tsx @@ -65,7 +65,7 @@ export const EuiAccordionChildren: FunctionComponent< const wrapperStyles = euiAccordionChildWrapperStyles(euiTheme); const wrapperCssStyles = [ wrapperStyles.euiAccordion__childWrapper, - isOpen && wrapperStyles.isOpen, + isOpen ? wrapperStyles.isOpen : wrapperStyles.isClosed, ]; const wrapperRef = useRef(null); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap index 047648833f1..2ce394d09a8 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_accordion.test.tsx.snap @@ -40,7 +40,7 @@ exports[`EuiCollapsibleNavAccordion renders as a sub item 1`] = `
Date: Mon, 4 Sep 2023 15:41:04 -0700 Subject: [PATCH 08/12] [regression] Fix children wrapper `.focus()` to only occur after mount --- .../accordion/accordion_children/accordion_children.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/accordion/accordion_children/accordion_children.tsx b/src/components/accordion/accordion_children/accordion_children.tsx index f9845690973..0dca8d2b433 100644 --- a/src/components/accordion/accordion_children/accordion_children.tsx +++ b/src/components/accordion/accordion_children/accordion_children.tsx @@ -11,13 +11,12 @@ import React, { HTMLAttributes, useRef, useCallback, - useEffect, useMemo, useState, } from 'react'; import classNames from 'classnames'; -import { useEuiTheme } from '../../../services'; +import { useEuiTheme, useUpdateEffect } from '../../../services'; import { EuiResizeObserver } from '../../observer/resize_observer'; import { EuiAccordionProps } from '../accordion'; @@ -85,9 +84,10 @@ export const EuiAccordionChildren: FunctionComponent< ); /** - * Focus the children wrapper on open + * Focus the children wrapper when the accordion is opened, + * but not if the accordion is initially open on mount */ - useEffect(() => { + useUpdateEffect(() => { if (isOpen) wrapperRef.current?.focus(); }, [isOpen]); From 3e82e7e2a6680d00e8864b1c4448939c670b1ed7 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 4 Sep 2023 14:57:27 -0700 Subject: [PATCH 09/12] More downstream snapshot updates --- .../collapsible_nav_group.test.tsx.snap | 32 ++++++++-------- .../collapsible_nav_item.test.tsx.snap | 32 ++++++++-------- .../notification_event.test.tsx.snap | 38 +++++++++---------- 3 files changed, 51 insertions(+), 51 deletions(-) diff --git a/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap b/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap index b4bab73763f..d26df326f72 100644 --- a/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap +++ b/src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap @@ -203,7 +203,7 @@ exports[`EuiCollapsibleNavGroup when isCollapsible is true accepts accordion pro data-test-subj="test subject string" >
-
-
-
+
`; @@ -270,7 +270,7 @@ exports[`EuiCollapsibleNavGroup when isCollapsible is true will render an accord class="euiAccordion euiCollapsibleNavGroup emotion-euiAccordion-euiCollapsibleNavGroup-isCollapsible-none" >
-
-
-
+
`; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap index 14dcc726e5e..c391909cbbb 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/__snapshots__/collapsible_nav_item.test.tsx.snap @@ -29,7 +29,7 @@ exports[`EuiCollapsibleNavItem renders a top level accordion if items exist 1`] data-test-subj="test subject string" >
-
+
-
- - Sub-item - + Sub-item -
+
diff --git a/src/components/notification/__snapshots__/notification_event.test.tsx.snap b/src/components/notification/__snapshots__/notification_event.test.tsx.snap index d531a3442d1..090ea4760d8 100644 --- a/src/components/notification/__snapshots__/notification_event.test.tsx.snap +++ b/src/components/notification/__snapshots__/notification_event.test.tsx.snap @@ -509,7 +509,7 @@ exports[`EuiNotificationEvent props multiple messages are rendered 1`] = ` class="euiAccordion euiNotificationEventMessages__accordion emotion-euiAccordion" >