diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ab51d3a915..ec85a16072e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## [`master`](https://github.com/elastic/eui/tree/master) - Updated `tokenKeyword` to match the definition of keyword field type ([#5251](https://github.com/elastic/eui/pull/5251)) +- Added `element`, `buttonElement`, and `arrowProps` props to further customize `EuiAccordion` ([#5258](https://github.com/elastic/eui/pull/5258)) **Breaking changes** diff --git a/src-docs/src/views/accordion/accordion.js b/src-docs/src/views/accordion/accordion.tsx similarity index 100% rename from src-docs/src/views/accordion/accordion.js rename to src-docs/src/views/accordion/accordion.tsx diff --git a/src-docs/src/views/accordion/accordion_buttonElement.tsx b/src-docs/src/views/accordion/accordion_buttonElement.tsx new file mode 100644 index 00000000000..6571f7e31eb --- /dev/null +++ b/src-docs/src/views/accordion/accordion_buttonElement.tsx @@ -0,0 +1,29 @@ +import React from 'react'; + +import { EuiAccordion, EuiLink, EuiPanel } from '../../../../src/components'; +import { useGeneratedHtmlId } from '../../../../src/services'; + +export default () => { + const buttonElementAccordionId = useGeneratedHtmlId({ + prefix: 'buttonElementAccordion', + }); + + return ( + e.stopPropagation()} + href="#/layout/accordion#interactive-content-in-the-trigger" + > + This is a nested link + + } + > + + Any content inside of EuiAccordion will appear here. + + + ); +}; diff --git a/src-docs/src/views/accordion/accordion_example.js b/src-docs/src/views/accordion/accordion_example.js index 61d79ab3d58..ff28c277b0a 100644 --- a/src-docs/src/views/accordion/accordion_example.js +++ b/src-docs/src/views/accordion/accordion_example.js @@ -86,6 +86,9 @@ const accordionIsLoadingSnippet = [ `, ]; +import AccordionButtonElement from './accordion_buttonElement'; +const accordionButtonElementSource = require('!!raw-loader!./accordion_buttonElement'); + export const AccordionExample = { title: 'Accordion', intro: ( @@ -327,6 +330,37 @@ export const AccordionExample = { ), demo: , }, + { + title: 'Interactive content in the trigger', + source: [ + { + type: GuideSectionTypes.JS, + code: accordionButtonElementSource, + }, + ], + text: ( + <> +

+ Passing interactive content like links, buttons, or form elements as + the buttonContent, will cause issues with the + wrapping button element. To fix this, you can change this wrapping + element to a div using {'buttonElement="div"'}. +

+

+ If you don't want the interactive content to trigger the + accordion expansion, you will have to apply{' '} + e.stopPropagation() to your element + manually. +

+ + + ), + demo: , + }, { title: 'Styled for forms', source: [ @@ -358,12 +392,22 @@ export const AccordionExample = { hide it until hover or focus +

+ We also recommend creating a fieldset/legend combination for better + accessibility and DOM structure by passing{' '} + {'element="fieldset"'}. This will + set the entire accordion as a{' '} + {'

'} and automatically + change the {'buttonElement'} to a{' '} + {''}. +

), demo: , snippet: ` {
{ { const exitPath = useExitPath(); const [navIsOpen, setNavIsOpen] = useState(true); - const [navIsDocked, setNavIsDocked] = useState( - JSON.parse(String(localStorage.getItem('nav2IsDocked'))) || false - ); /** * Accordion toggling @@ -143,119 +133,142 @@ const CollapsibleNavAll = () => { const collapsibleNavId = useGeneratedHtmlId({ prefix: 'collapsibleNav' }); - const collapsibleNav = () => { - return ( - setNavIsOpen(!navIsOpen)} - > - + ); const leftSectionItems = [ collapsibleNav, diff --git a/src-docs/src/views/collapsible_nav/collapsible_nav_group.tsx b/src-docs/src/views/collapsible_nav/collapsible_nav_group.tsx index 96f971f3c0f..ccb6b6b2fa9 100644 --- a/src-docs/src/views/collapsible_nav/collapsible_nav_group.tsx +++ b/src-docs/src/views/collapsible_nav/collapsible_nav_group.tsx @@ -1,8 +1,6 @@ import React from 'react'; -import { EuiCollapsibleNavGroup } from '../../../../src/components/collapsible_nav'; -import { EuiText } from '../../../../src/components/text'; -import { EuiCode } from '../../../../src/components/code'; +import { EuiCollapsibleNavGroup, EuiText, EuiCode } from '../../../../src'; export default () => ( <> @@ -11,11 +9,7 @@ export default () => (

This is a basic group without any modifications

- +

This is a nice group with a heading supplied via{' '} @@ -24,7 +18,6 @@ export default () => ( - + +

@@ -56,7 +80,11 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = `
+ > +

+ You can not see me. +

+
@@ -67,7 +95,9 @@ exports[`EuiAccordion behavior closes when clicked twice 1`] = ` exports[`EuiAccordion behavior opens when clicked once 1`] = ` + + + + +
+ +
+
+

+ You can see me. +

+
+
+
+
+ +
+`; + +exports[`EuiAccordion behavior opens when div is clicked if element is a div 1`] = ` + +
+
+ + + +
@@ -120,7 +264,11 @@ exports[`EuiAccordion behavior opens when clicked once 1`] = `
+ > +

+ You can see me. +

+
@@ -137,6 +285,21 @@ exports[`EuiAccordion is rendered 1`] = `
+
@@ -181,32 +336,28 @@ exports[`EuiAccordion props arrowDisplay none is rendered 1`] = ` class="euiAccordion__triggerWrapper" >
-

- You can see me. -

-
+ />
@@ -220,46 +371,49 @@ exports[`EuiAccordion props arrowDisplay right is rendered 1`] = ` class="euiAccordion__triggerWrapper" > +
-

- You can see me. -

-
+ />
`; -exports[`EuiAccordion props buttonContent is rendered 1`] = ` +exports[`EuiAccordion props arrowProps is rendered 1`] = `
@@ -267,22 +421,81 @@ exports[`EuiAccordion props buttonContent is rendered 1`] = ` class="euiAccordion__triggerWrapper" > + +
+
+
+
+
+
+
+`; + +exports[`EuiAccordion props buttonContent is rendered 1`] = ` +
+
+ + + +
+
+
+
+
+
+
+`; + +exports[`EuiAccordion props buttonElement is rendered 1`] = ` +
+
+ +
@@ -357,7 +626,22 @@ exports[`EuiAccordion props buttonProps is rendered 1`] = ` class="euiAccordion__triggerWrapper" > +
@@ -394,6 +670,55 @@ exports[`EuiAccordion props buttonProps is rendered 1`] = `
`; +exports[`EuiAccordion props element is rendered 1`] = ` +
+
+ + +
+
+
+
+
+
+
+`; + exports[`EuiAccordion props extraAction is rendered 1`] = `
+
@@ -444,7 +776,7 @@ exports[`EuiAccordion props extraAction is rendered 1`] = `
`; -exports[`EuiAccordion props forceState is rendered 1`] = ` +exports[`EuiAccordion props forceState closed is rendered 1`] = `
@@ -452,29 +784,36 @@ exports[`EuiAccordion props forceState is rendered 1`] = ` class="euiAccordion__triggerWrapper" > +
@@ -491,7 +830,7 @@ exports[`EuiAccordion props forceState is rendered 1`] = `
`; -exports[`EuiAccordion props initialIsOpen is rendered 1`] = ` +exports[`EuiAccordion props forceState open is rendered 1`] = `
@@ -499,29 +838,36 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = ` class="euiAccordion__triggerWrapper" > +
@@ -530,7 +876,7 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = ` class="" >

- You can see me. + You can see me

@@ -538,50 +884,50 @@ exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
`; -exports[`EuiAccordion props isLoading is rendered 1`] = ` +exports[`EuiAccordion props initialIsOpen is rendered 1`] = `
-
-
+

You can see me. @@ -592,7 +938,7 @@ exports[`EuiAccordion props isLoading is rendered 1`] = `

`; -exports[`EuiAccordion props isLoadingMessage is rendered 1`] = ` +exports[`EuiAccordion props isLoading is rendered 1`] = `
@@ -600,36 +946,86 @@ exports[`EuiAccordion props isLoadingMessage is rendered 1`] = ` class="euiAccordion__triggerWrapper" > + +
+
+
+
+
+
+
+`; + +exports[`EuiAccordion props isLoadingMessage is rendered 1`] = ` +
+
+ - +
diff --git a/src/components/accordion/_accordion.scss b/src/components/accordion/_accordion.scss index 4d7e7c07302..aad27ccd02b 100644 --- a/src/components/accordion/_accordion.scss +++ b/src/components/accordion/_accordion.scss @@ -16,47 +16,22 @@ text-decoration: underline; cursor: pointer; } - - &:focus { - .euiAccordion__iconWrapper { - @include euiAccordionIconFocus; - outline: none; // The `outline` gets applied to the whole button, we don't need a second one on the icon - } - } -} - -.euiAccordion__buttonReverse { - // Puts the arrow on the right - flex-direction: row-reverse; - justify-content: space-between; - - .euiAccordion__iconWrapper { - @include euiAccordionIconMargin(right); - } } -.euiAccordion__iconWrapper { - @include size($euiSize); - @include euiAccordionIconMargin; - border-radius: $euiBorderRadius; +.euiAccordion__iconButton { + margin-right: $euiSizeXS; flex-shrink: 0; + // sass-lint:disable-block no-important + // Needed to override transform from EuiButtonIcon + transform: rotate(0deg) !important; - // Nested to override EuiIcon - .euiAccordion__icon { - vertical-align: top; - transition: transform $euiAnimSpeedNormal $euiAnimSlightResistance; - } - - .euiAccordion__icon-isOpen { - transform: rotate(90deg); + &-isOpen { + transform: rotate(90deg) !important; } -} -.euiAccordion__iconButton { - @include euiAccordionIconMargin(right); - - &:focus { - @include euiAccordionIconFocus; + &--right { + margin-right: 0; + margin-left: $euiSizeXS; } } diff --git a/src/components/accordion/_index.scss b/src/components/accordion/_index.scss index 934d19bee5f..4cf11eea6dc 100644 --- a/src/components/accordion/_index.scss +++ b/src/components/accordion/_index.scss @@ -1,3 +1,2 @@ -@import 'mixins'; @import 'accordion'; @import 'accordion_form'; diff --git a/src/components/accordion/_mixins.scss b/src/components/accordion/_mixins.scss deleted file mode 100644 index 9f0fabddb82..00000000000 --- a/src/components/accordion/_mixins.scss +++ /dev/null @@ -1,14 +0,0 @@ -@mixin euiAccordionIconFocus { - @include euiFocusRing; - color: $euiColorPrimary; -} - -@mixin euiAccordionIconMargin($align: left) { - @if $align == left { - margin-left: $euiSizeXS; - margin-right: $euiSizeS; - } @else { - margin-left: $euiSizeS; - margin-right: $euiSizeXS; - } -} diff --git a/src/components/accordion/accordion.test.tsx b/src/components/accordion/accordion.test.tsx index b277de46bb0..512e40a3754 100644 --- a/src/components/accordion/accordion.test.tsx +++ b/src/components/accordion/accordion.test.tsx @@ -23,6 +23,16 @@ describe('EuiAccordion', () => { }); describe('props', () => { + describe('element', () => { + it('is rendered', () => { + const component = render( + + ); + + expect(component).toMatchSnapshot(); + }); + }); + describe('buttonContentClassName', () => { it('is rendered', () => { const component = render( @@ -59,6 +69,16 @@ describe('EuiAccordion', () => { }); }); + describe('buttonElement', () => { + it('is rendered', () => { + const component = render( + + ); + + expect(component).toMatchSnapshot(); + }); + }); + describe('extraAction', () => { it('is rendered', () => { const component = render( @@ -87,9 +107,7 @@ describe('EuiAccordion', () => { describe('arrowDisplay', () => { it('right is rendered', () => { const component = render( - -

You can see me.

-
+ ); expect(component).toMatchSnapshot(); @@ -97,17 +115,25 @@ describe('EuiAccordion', () => { it('none is rendered', () => { const component = render( - -

You can see me.

-
+ ); expect(component).toMatchSnapshot(); }); }); - describe('forceState', () => { + describe('arrowProps', () => { it('is rendered', () => { + const component = render( + + ); + + expect(component).toMatchSnapshot(); + }); + }); + + describe('forceState', () => { + it('closed is rendered', () => { const component = render(

You can not see me

@@ -117,6 +143,16 @@ describe('EuiAccordion', () => { expect(component).toMatchSnapshot(); }); + it('open is rendered', () => { + const component = render( + +

You can see me

+
+ ); + + expect(component).toMatchSnapshot(); + }); + it('accepts and calls an optional callback on click', () => { const onToggleHandler = jest.fn(); const component = mount( @@ -127,7 +163,7 @@ describe('EuiAccordion', () => { /> ); - component.find('button').simulate('click'); + component.find('button').at(0).simulate('click'); expect(onToggleHandler).toBeCalled(); expect(onToggleHandler).toBeCalledWith(true); }); @@ -135,11 +171,7 @@ describe('EuiAccordion', () => { describe('isLoading', () => { it('is rendered', () => { - const component = render( - -

You can see me.

-
- ); + const component = render(); expect(component).toMatchSnapshot(); }); @@ -148,9 +180,7 @@ describe('EuiAccordion', () => { describe('isLoadingMessage', () => { it('is rendered', () => { const component = render( - -

You can't see me.

-
+ ); expect(component).toMatchSnapshot(); @@ -160,18 +190,38 @@ describe('EuiAccordion', () => { describe('behavior', () => { it('opens when clicked once', () => { - const component = mount(); + const component = mount( + +

You can see me.

+
+ ); - component.find('button').simulate('click'); + component.find('button').at(0).simulate('click'); + + expect(component).toMatchSnapshot(); + }); + + it('opens when div is clicked if element is a div', () => { + const component = mount( + +

You can see me.

+
+ ); + + component.find('.euiAccordion__button').simulate('click'); expect(component).toMatchSnapshot(); }); it('closes when clicked twice', () => { - const component = mount(); + const component = mount( + +

You can not see me.

+
+ ); - component.find('button').simulate('click'); - component.find('button').simulate('click'); + component.find('button').at(0).simulate('click'); + component.find('button').at(0).simulate('click'); expect(component).toMatchSnapshot(); }); @@ -182,11 +232,11 @@ describe('EuiAccordion', () => { ); - component.find('button').simulate('click'); + component.find('button').at(0).simulate('click'); expect(onToggleHandler).toBeCalled(); expect(onToggleHandler).toBeCalledWith(true); - component.find('button').simulate('click'); + component.find('button').at(0).simulate('click'); expect(onToggleHandler).toBeCalled(); expect(onToggleHandler).toBeCalledWith(false); }); @@ -200,7 +250,7 @@ describe('EuiAccordion', () => { expect(childWrapper).not.toBe(document.activeElement); // click button - component.find('button').simulate('click'); + component.find('button').at(0).simulate('click'); expect(childWrapper).toBe(document.activeElement); }); diff --git a/src/components/accordion/accordion.tsx b/src/components/accordion/accordion.tsx index 5a56c3cd2a4..25d797df626 100644 --- a/src/components/accordion/accordion.tsx +++ b/src/components/accordion/accordion.tsx @@ -11,11 +11,11 @@ import classNames from 'classnames'; import { CommonProps, keysOf } from '../common'; -import { EuiIcon } from '../icon'; import { EuiLoadingSpinner } from '../loading'; import { EuiResizeObserver } from '../observer/resize_observer'; import { EuiI18n } from '../i18n'; import { htmlIdGenerator } from '../../services'; +import { EuiButtonIcon, EuiButtonIconProps } from '../button'; const paddingSizeToClassNameMap = { none: '', @@ -30,8 +30,13 @@ export const PADDING_SIZES = keysOf(paddingSizeToClassNameMap); export type EuiAccordionSize = keyof typeof paddingSizeToClassNameMap; export type EuiAccordionProps = CommonProps & - Omit, 'id'> & { + Omit, 'id'> & { id: string; + /** + * Applied to the entire .euiAccordion wrapper. + * When using `fieldset`, it will enforce `buttonElement = legend` as well. + */ + element?: 'div' | 'fieldset'; /** * Class that will apply to the trigger for the accordion. */ @@ -39,7 +44,7 @@ export type EuiAccordionProps = CommonProps & /** * Apply more props to the triggering button */ - buttonProps?: CommonProps & HTMLAttributes; + buttonProps?: CommonProps & HTMLAttributes; /** * Class that will apply to the trigger content for the accordion. */ @@ -48,6 +53,17 @@ export type EuiAccordionProps = CommonProps & * The content of the clickable trigger */ buttonContent?: ReactNode; + /** + * Applied to the main button receiving the `onToggle` event. + * Anything other than the default `button` does not support removing the arrow display (for accessibility of focus). + */ + buttonElement?: 'div' | 'legend' | 'button'; + /** + * Extra props to pass to the EuiButtonIcon containing the arrow. + */ + arrowProps?: Partial< + Omit + >; /** * Will appear right aligned against the button. Useful for separate actions like deletions. */ @@ -92,6 +108,8 @@ export class EuiAccordion extends Component< arrowDisplay: 'left', isLoading: false, isLoadingMessage: false, + element: 'div', + buttonElement: 'button', }; childContent: HTMLDivElement | null = null; @@ -156,6 +174,7 @@ export class EuiAccordion extends Component< buttonContent, className, id, + element: Element = 'div', buttonClassName, buttonContentClassName, extraAction, @@ -166,11 +185,23 @@ export class EuiAccordion extends Component< isLoading, isLoadingMessage, buttonProps, + buttonElement: _ButtonElement = 'button', + arrowProps, ...rest } = this.props; const isOpen = forceState ? forceState === 'open' : this.state.isOpen; + // 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', { @@ -189,56 +220,48 @@ export class EuiAccordion extends Component< const buttonClasses = classNames( 'euiAccordion__button', - { - euiAccordion__buttonReverse: !extraAction && arrowDisplay === 'right', - }, buttonClassName, buttonProps?.className ); - const iconClasses = classNames('euiAccordion__icon', { - 'euiAccordion__icon-isOpen': isOpen, - }); - - const iconWrapperClasses = classNames('euiAccordion__iconWrapper', { - euiAccordion__iconButton: extraAction && arrowDisplay === 'right', - }); + const buttonContentClasses = classNames( + 'euiAccordion__buttonContent', + buttonContentClassName + ); - let baseIcon; - if (arrowDisplay !== 'none') { - baseIcon = ; - } + const iconButtonClasses = classNames( + 'euiAccordion__iconButton', + { + 'euiAccordion__iconButton-isOpen': isOpen, + 'euiAccordion__iconButton--right': _arrowDisplay === 'right', + }, + arrowProps?.className + ); - let icon; let iconButton; const buttonId = buttonProps?.id ?? this.generatedId; - if (extraAction && arrowDisplay === 'right') { + if (_arrowDisplay !== 'none') { iconButton = ( - + tabIndex={buttonElementIsFocusable ? -1 : 0} + /> ); - } else if (arrowDisplay !== 'none') { - icon = {baseIcon}; } let optionalAction = null; - if (extraAction && !isLoading) { - optionalAction = ( -
{extraAction}
- ); - } else if (isLoading) { + if (extraAction) { optionalAction = (
- + {isLoading ? : extraAction}
); } @@ -261,27 +284,27 @@ export class EuiAccordion extends Component< childrenContent = children; } + const button = ( + + {buttonContent} + + ); + return ( -
+
- + {_arrowDisplay === 'left' && iconButton} + {button} {optionalAction} - {iconButton} + {_arrowDisplay === 'right' && iconButton}
-
+ ); } } 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 8a6f9696580..67c9a1a409e 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 @@ -209,21 +209,13 @@ exports[`EuiCollapsibleNavGroup when isCollapsible is true accepts accordion pro aria-controls="id" aria-expanded="false" aria-label="aria-label" - class="euiAccordion__button euiAccordion__buttonReverse euiCollapsibleNavGroup__heading testClass1 testClass2" + class="euiAccordion__button euiCollapsibleNavGroup__heading testClass1 testClass2" data-test-subj="test subject string" id="generated-id" type="button" > - - -
+