diff --git a/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx b/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx index 1082c92a913..1a24ef61b7d 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_beta.stories.tsx @@ -111,7 +111,6 @@ export const KibanaExample: Story = { { const { euiTheme } = euiThemeContext; @@ -42,7 +42,7 @@ export const euiCollapsibleNavBetaStyles = (euiThemeContext: UseEuiTheme) => { ${euiShadowFlat(euiThemeContext)} `, isPushCollapsed: css` - ${euiCollapsibleNavBodyStyles._isPushCollapsed} + ${hideScrollbars} `, isOverlayFullWidth: css` /* Override EuiFlyout's max-width */ diff --git a/src/components/collapsible_nav_beta/collapsible_nav_body_footer.styles.ts b/src/components/collapsible_nav_beta/collapsible_nav_body_footer.styles.ts index 260697f2992..931fe44ed81 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_body_footer.styles.ts +++ b/src/components/collapsible_nav_beta/collapsible_nav_body_footer.styles.ts @@ -8,37 +8,40 @@ import { css } from '@emotion/react'; import { UseEuiTheme } from '../../services'; -import { logicalCSS } from '../../global_styling'; +import { logicalCSS, euiYScrollWithShadows } from '../../global_styling'; + +// Hide the scrollbar for docked mode (while still keeping the nav scrollable) +// Otherwise if scrollbars are visible, button icon visibility suffers. +export const hideScrollbars = ` + scrollbar-width: none; /* Firefox */ + + &::-webkit-scrollbar { + display: none; /* Chrome, Edge, & Safari */ + } +`; export const euiCollapsibleNavBodyStyles = { // In case things get really dire responsively, ensure the footer doesn't overtake the body euiCollapsibleNav__body: css` ${logicalCSS('min-height', '50%')} `, - get isPushCollapsed() { - return css` - .euiFlyoutBody__overflow { - ${this._isPushCollapsed} - } - `; - }, - // CSS is reused by main euiCollapsibleNav styles in case the body component isn't used - _isPushCollapsed: ` - /* Hide the scrollbar for docked mode (while still keeping the nav scrollable) - Otherwise if scrollbars are visible, button icon visibility suffers. */ - scrollbar-width: none; /* Firefox */ - - &::-webkit-scrollbar { - display: none; /* Chrome, Edge, & Safari */ + isPushCollapsed: css` + .euiFlyoutBody__overflow { + ${hideScrollbars} } `, }; -export const euiCollapsibleNavFooterStyles = ({ euiTheme }: UseEuiTheme) => { +export const euiCollapsibleNavFooterStyles = (euiThemeContext: UseEuiTheme) => { + const { euiTheme } = euiThemeContext; return { euiCollapsibleNav__footer: css` background-color: ${euiTheme.colors.emptyShade}; ${logicalCSS('border-top', euiTheme.border.thin)} + ${euiYScrollWithShadows(euiThemeContext, { side: 'end' })} + `, + isPushCollapsed: css` + ${hideScrollbars} `, }; }; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_body_footer.tsx b/src/components/collapsible_nav_beta/collapsible_nav_body_footer.tsx index 7bd6ebb9a39..a268126006d 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_body_footer.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_body_footer.tsx @@ -61,9 +61,13 @@ export const EuiCollapsibleNavFooter: EuiFlyoutFooterProps = ({ }) => { const classes = classNames('euiCollapsibleNav__footer', className); + const { isCollapsed, isPush } = useContext(EuiCollapsibleNavContext); const euiTheme = useEuiTheme(); const styles = euiCollapsibleNavFooterStyles(euiTheme); - const cssStyles = [styles.euiCollapsibleNav__footer]; + const cssStyles = [ + styles.euiCollapsibleNav__footer, + isCollapsed && isPush && styles.isPushCollapsed, + ]; return ; }; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.stories.tsx b/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.stories.tsx index e4fc402fc8b..75d55827b71 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.stories.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.stories.tsx @@ -32,7 +32,6 @@ const meta: Meta = { { title: 'Dashboards', href: '#' }, { title: 'Explore', - href: '#', items: [ { title: 'Hello', href: '#' }, { title: 'World', href: '#' }, @@ -88,7 +87,4 @@ export const EdgeCaseTesting: Story = { ), - args: { - href: '#', - }, }; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.tsx b/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.tsx index 22e8ba90158..bbb7eaf3f5b 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_group/collapsible_nav_group.tsx @@ -16,29 +16,24 @@ import { EuiCollapsibleNavContext } from '../context'; import { EuiCollapsibleNavItem, EuiCollapsibleNavSubItems, - EuiCollapsibleNavSubItemProps, - EuiCollapsibleNavItemProps, + type EuiCollapsibleNavItemProps, + type _SharedEuiCollapsibleNavItemProps, } from '../collapsible_nav_item/collapsible_nav_item'; import { EuiCollapsedNavPopover } from '../collapsible_nav_item/collapsed/collapsed_nav_popover'; import { euiCollapsibleNavGroupStyles } from './collapsible_nav_group.styles'; -export type EuiCollapsibleNavGroupProps = Omit< - EuiCollapsibleNavItemProps, - 'items' | 'accordionProps' -> & { - /** - * Will render an array of `EuiCollapsibleNavItems`. - * - * Accepts any #EuiCollapsibleNavItemProps. Or, to render completely custom - * subitem content, pass an object with a `renderItem` callback. - */ - items: EuiCollapsibleNavSubItemProps[]; - /** - * Optional props to pass to the wrapping div - */ - wrapperProps?: HTMLAttributes & CommonProps; -}; +export type EuiCollapsibleNavGroupProps = _SharedEuiCollapsibleNavItemProps & + Pick< + EuiCollapsibleNavItemProps, + 'title' | 'titleElement' | 'icon' | 'iconProps' + > & + Required> & { + /** + * Optional props to pass to the wrapping div + */ + wrapperProps?: HTMLAttributes & CommonProps; + }; /** * This component should only ever be used as a **top-level component**, and not as a sub-item. 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 9a05ff15881..11b63eafdf8 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 @@ -7,10 +7,12 @@ exports[`EuiCollapsibleNavAccordion renders as a sub item 1`] = `
- +
`; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/__snapshots__/collapsed_nav_item.test.tsx.snap b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/__snapshots__/collapsed_nav_item.test.tsx.snap index 995c41af0b3..166881b3fac 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/__snapshots__/collapsed_nav_item.test.tsx.snap +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/__snapshots__/collapsed_nav_item.test.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`EuiCollapsedNavItem renders a popover if items exist 1`] = ` +exports[`EuiCollapsedNavItem renders a popover if items are passed 1`] = `
`; -exports[`EuiCollapsedNavItem renders an icon button/link if items are missing or empty 1`] = ` +exports[`EuiCollapsedNavItem renders an icon button/link if href is passed 1`] = `
- - - Item - - + Item +
& { + hideToolTip?: boolean; // Used by EuiCollapsedNavPopover to prevent tooltip/popover overlap } > = ({ href, // eslint-disable-line local/href-with-rel @@ -37,10 +40,7 @@ export const EuiCollapsedNavButton: FunctionComponent< onClick, hideToolTip, linkProps, - // Extracted to avoid spreading to ...rest - accordionProps, - titleElement, - items, + titleElement, // Extracted to avoid spreading to ...rest ...rest }) => { const euiTheme = useEuiTheme(); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.stories.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.stories.tsx index 1a1c4d991a5..d10762203f8 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.stories.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.stories.tsx @@ -21,7 +21,7 @@ const meta: Meta = { export default meta; type Story = StoryObj; -export const Playground: Story = { +export const Link: Story = { render: ({ ...args }) => (
@@ -29,8 +29,20 @@ export const Playground: Story = { ), args: { title: 'Collapsed nav item', + icon: 'home', href: '#', linkProps: { target: '_blank' }, + }, +}; + +export const Accordion: Story = { + render: ({ ...args }) => ( +
+ +
+ ), + args: { + title: 'Collapsed nav item', icon: 'home', items: [ { title: 'Popover link A', href: '#', linkProps: { target: '_blank' } }, diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.test.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.test.tsx index e4dbfd3f509..e8bb09b7f4a 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.test.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.test.tsx @@ -13,7 +13,7 @@ import { requiredProps } from '../../../../test'; import { EuiCollapsedNavItem } from './collapsed_nav_item'; describe('EuiCollapsedNavItem', () => { - it('renders a popover if items exist', () => { + it('renders a popover if items are passed', () => { const { container } = render( { expect(container.firstChild).toMatchSnapshot(); }); - it('renders an icon button/link if items are missing or empty', () => { + it('renders an icon button/link if href is passed', () => { const { container } = render( - + ); expect(container.firstChild).toHaveClass('euiToolTipAnchor'); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.tsx index 8460ddfe13c..8bdc1192265 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_item.tsx @@ -23,14 +23,26 @@ import classNames from 'classnames'; export const EuiCollapsedNavItem: FunctionComponent< EuiCollapsibleNavItemProps -> = ({ items, className, ...props }) => { +> = ({ + className, + href, // eslint-disable-line local/href-with-rel + linkProps, + items, + // Extracted to avoid spreading to DOM node + accordionProps, + isCollapsible, + ...props +}) => { const classes = classNames('euiCollapsedNavItem', className); - const hasItems = items && items.length > 0; - - return hasItems ? ( + return items ? ( ) : ( - + ); }; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.styles.ts b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.styles.ts index 092e873cb3f..16db9d6c37f 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.styles.ts +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.styles.ts @@ -15,8 +15,6 @@ import { euiYScrollWithShadows, } from '../../../../global_styling'; -import { euiCollapsibleNavItemVariables } from '../collapsible_nav_item.styles'; - export const euiCollapsedNavPopoverStyles = (euiThemeContext: UseEuiTheme) => { const { euiTheme } = euiThemeContext; @@ -27,6 +25,11 @@ export const euiCollapsedNavPopoverStyles = (euiThemeContext: UseEuiTheme) => { mathWithUnits(euiTheme.size.base, (x) => x * 15) )} `, + euiCollapsedNavPopover__title: css` + display: block; + padding: ${euiTheme.size.m}; + font-weight: ${euiTheme.font.weight.bold}; + `, euiCollapsedNavPopover__items: css` ${euiYScrollWithShadows(euiThemeContext)} padding-block: ${euiTheme.size.s}; @@ -39,32 +42,3 @@ export const euiCollapsedNavPopoverStyles = (euiThemeContext: UseEuiTheme) => { `, }; }; - -export const euiCollapsedNavPopoverTitleStyles = ( - euiThemeContext: UseEuiTheme -) => { - const { euiTheme } = euiThemeContext; - const sharedStyles = euiCollapsibleNavItemVariables(euiThemeContext); - - return { - euiCollapsedNavPopover__title: css` - padding: ${euiTheme.size.m}; - font-weight: ${euiTheme.font.weight.bold}; - `, - link: css` - /* Vertically align the external icon */ - display: flex; - align-items: center; - - /* Horizontally align with other external link icons */ - [class*='euiLink__externalIcon'] { - ${logicalCSS('margin-left', 'auto')} - ${logicalCSS('margin-right', euiTheme.size.xxs)} - color: ${sharedStyles.rightIconColor}; - } - `, - span: css` - display: block; - `, - }; -}; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.test.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.test.tsx index 70f1f1b099f..007aa846d54 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.test.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.test.tsx @@ -22,28 +22,13 @@ describe('EuiCollapsedNavPopover', () => { shouldRenderCustomStyles( ); - shouldRenderCustomStyles( - , - { - childProps: ['linkProps'], - renderCallback: async ({ getByTestSubject }) => { - fireEvent.click(getByTestSubject('euiCollapsedNavButton')); - await waitForEuiPopoverOpen(); - }, - } - ); - it('renders the title and sub-items within the popover', async () => { - const { baseElement, getByTestSubject } = render( + it('renders', async () => { + const { baseElement, getByTestSubject, getByText } = render( { await waitForEuiPopoverOpen(); expect(baseElement).toMatchSnapshot(); - expect(getByTestSubject('popoverTitle')).toHaveTextContent('Item'); + expect(getByText('Item').nodeName).toEqual('H3'); expect(getByTestSubject('A')).toHaveTextContent('Sub-item A'); expect(getByTestSubject('B')).toHaveTextContent('Sub-item B'); fireEvent.keyDown(baseElement, { key: 'Escape' }); await waitForEuiPopoverClose(); }); - - it('renders popver titles without links', async () => { - const { getByText, getByTestSubject } = render( - - ); - fireEvent.click(getByTestSubject('euiCollapsedNavButton')); - await waitForEuiPopoverOpen(); - - expect(getByText('Popover title')).toMatchInlineSnapshot(` -

- Popover title -

- `); - }); }); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.tsx index 68f4aa4e053..f1f07a03d06 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsed/collapsed_nav_popover.tsx @@ -10,7 +10,6 @@ import React, { FunctionComponent, useState, useCallback } from 'react'; import { useEuiTheme } from '../../../../services'; -import { EuiLink, EuiLinkAnchorProps } from '../../../link'; import { EuiPopover, EuiPopoverTitle } from '../../../popover'; import { @@ -19,26 +18,20 @@ import { } from '../collapsible_nav_item'; import { EuiCollapsedNavButton } from './collapsed_nav_button'; -import { - euiCollapsedNavPopoverStyles, - euiCollapsedNavPopoverTitleStyles, -} from './collapsed_nav_popover.styles'; +import { euiCollapsedNavPopoverStyles } from './collapsed_nav_popover.styles'; export const EuiCollapsedNavPopover: FunctionComponent< - EuiCollapsibleNavItemProps & { - items: EuiCollapsibleNavItemProps['items']; - } + Omit< + EuiCollapsibleNavItemProps, + 'isCollapsible' | 'accordionProps' | 'href' | 'linkProps' + > > = ({ items, - href, // eslint-disable-line local/href-with-rel - linkProps, title, - titleElement, + titleElement: TitleElement = 'span', icon, iconProps, isSelected, - // Extracted to avoid spreading to ...rest - accordionProps, ...rest }) => { const euiTheme = useEuiTheme(); @@ -67,17 +60,18 @@ export const EuiCollapsedNavPopover: FunctionComponent< isSelected={isSelected} onClick={togglePopover} hideToolTip={isPopoverOpen} - // Note: do not pass `linkProps` to buttons that toggle popovers /> } {...rest} > - + + + {title} + +
{items!.map((item, index) => ( @@ -86,42 +80,3 @@ export const EuiCollapsedNavPopover: FunctionComponent< ); }; - -const EuiCollapsedNavPopoverTitle: FunctionComponent< - Pick< - EuiCollapsibleNavItemProps, - 'title' | 'titleElement' | 'href' | 'linkProps' - > -> = ({ - title, - titleElement: TitleElement = 'span', - href, // eslint-disable-line local/href-with-rel - linkProps, -}) => { - const euiTheme = useEuiTheme(); - const styles = euiCollapsedNavPopoverTitleStyles(euiTheme); - const cssStyles = [ - styles.euiCollapsedNavPopover__title, - href ? styles.link : styles.span, - href && linkProps?.css, - ]; - - return ( - - {href ? ( - - {title} - - ) : ( - - {title} - - )} - - ); -}; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.styles.ts b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.styles.ts index c8f7a3fc898..397f39a7b79 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.styles.ts +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.styles.ts @@ -25,22 +25,36 @@ export const euiCollapsibleNavAccordionStyles = ( euiCollapsibleNavAccordion: css` .euiAccordion__button { overflow: hidden; /* Title text truncation doesn't work otherwise */ - - /* unset accordion underline - only show for EuiLinks (which display their own underlines) - * so that behavior between link accordions and non-link accordions is consistent */ - &:hover, - &:focus { - cursor: default; - text-decoration: none; - } } - .euiAccordion__triggerWrapper { + & > .euiAccordion__triggerWrapper { border-radius: ${sharedStyles.borderRadius}; ${euiCanAnimate} { transition: background-color ${sharedStyles.animation}; } + + &:hover, + &:focus-within { + background-color: ${sharedStyles.backgroundHoverColor}; + + .euiAccordion__arrow .euiIcon { + color: ${sharedStyles.color}; + } + } + + /* Move the keyboard focus outline to the entire trigger wrapper */ + &:has(:focus-visible) { + outline-style: auto; + outline-offset: -${euiTheme.focus.width}; + + *:focus { + outline: none; + } + } + + /* Weird workaround for Safari showing a strange focus ring bump around the arrow */ + overflow: hidden; } .euiAccordion__buttonContent { @@ -54,24 +68,19 @@ export const euiCollapsibleNavAccordionStyles = ( ${logicalCSS('width', '100%')} } `, - isTopItem: css` - margin: ${sharedStyles.padding}; - - & > .euiAccordion__triggerWrapper { - &:hover { - background-color: ${sharedStyles.backgroundHoverColor}; - } - } - `, isSelected: css` & > .euiAccordion__triggerWrapper { background-color: ${sharedStyles.backgroundSelectedColor}; - &:hover { + &:hover, + &:focus-within { background-color: ${sharedStyles.backgroundSelectedColor}; } } `, + isTopItem: css` + margin: ${sharedStyles.padding}; + `, isSubItem: css` /* Adds extra spacing to the bottom of the accordion while open. Notes: 1. This uses a pseudo element instead of margin-bottom on the accordion, @@ -88,22 +97,11 @@ export const euiCollapsibleNavAccordionStyles = ( `, // Arrow element euiCollapsibleNavAccordion__arrow: css` + /* Ensure there's no non-clickable deadzones in the accordion trigger wrapper */ + margin: 0; + ${logicalCSS('height', sharedStyles.height)} /* Slight visual offset from edge of entire item */ - ${logicalCSS('margin-right', euiTheme.size.xs)} - - /* Give the arrow button its own clearer hover animation to indicate its hitbox */ - ${euiCanAnimate} { - transition: background-color ${sharedStyles.animation}; - } - - &:hover, - &:focus-visible { - background-color: ${euiTheme.colors.lightShade}; - - & > .euiIcon { - color: ${sharedStyles.color}; - } - } + ${logicalCSS('width', euiTheme.size.xl)} /* Rotate the arrow icon, not the button itself - * otherwise the background rotates and looks a bit silly */ @@ -123,6 +121,12 @@ export const euiCollapsibleNavAccordionStyles = ( color: ${sharedStyles.color}; transform: rotate(-90deg); } + + /* Unset default accordion arrow style */ + &:hover, + &:focus { + background-color: transparent; + } `, }; }; diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.test.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.test.tsx index 86418cb60bb..f2603284522 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.test.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.test.tsx @@ -22,7 +22,6 @@ describe('EuiCollapsibleNavAccordion', () => { shouldRenderCustomStyles(, { childProps: [ - 'linkProps', 'accordionProps', 'accordionProps.arrowProps', 'accordionProps.buttonProps', @@ -53,26 +52,28 @@ describe('EuiCollapsibleNavAccordion', () => { ); }); - describe('when the accordion header is a link and the link is clicked', () => { - it('does not trigger the accordion opening', () => { - const { getByTestSubject, container } = render( - - ); + it('triggers the accordion opening', () => { + const { getByTestSubject, container } = render( + + ); + expect( + container.querySelector('.euiAccordion__childWrapper') + ).toHaveStyleRule('opacity', '0'); - fireEvent.click(getByTestSubject('link')); - expect( - container.querySelector('.euiAccordion__childWrapper') - ).toHaveStyleRule('opacity', '0'); + fireEvent.click(getByTestSubject('button')); + expect( + container.querySelector('.euiAccordion__childWrapper') + ).toHaveStyleRule('opacity', '1'); - fireEvent.click(getByTestSubject('toggle')); - expect( - container.querySelector('.euiAccordion__childWrapper') - ).toHaveStyleRule('opacity', '1'); - }); + fireEvent.click(getByTestSubject('arrow')); + expect( + container.querySelector('.euiAccordion__childWrapper') + ).toHaveStyleRule('opacity', '0'); }); }); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.tsx index 7871efc7fd0..6aeb62ba4bf 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_accordion.tsx @@ -6,33 +6,26 @@ * Side Public License, v 1. */ -import React, { - FunctionComponent, - ReactNode, - MouseEvent, - useCallback, -} from 'react'; +import React, { FunctionComponent, ReactNode } from 'react'; import classNames from 'classnames'; import { useEuiTheme, useGeneratedHtmlId } from '../../../services'; import { EuiAccordion } from '../../accordion'; import { + type _SharedEuiCollapsibleNavItemProps, + type _EuiCollapsibleNavItemDisplayProps, + type EuiCollapsibleNavItemProps, EuiCollapsibleNavSubItems, - EuiCollapsibleNavSubItemProps, - _SharedEuiCollapsibleNavItemProps, - _EuiCollapsibleNavItemDisplayProps, } from './collapsible_nav_item'; import { EuiCollapsibleNavLink } from './collapsible_nav_link'; import { euiCollapsibleNavAccordionStyles } from './collapsible_nav_accordion.styles'; -type EuiCollapsibleNavAccordionProps = Omit< - _SharedEuiCollapsibleNavItemProps, - 'items' | 'isCollapsible' -> & - _EuiCollapsibleNavItemDisplayProps & { +type EuiCollapsibleNavAccordionProps = _SharedEuiCollapsibleNavItemProps & + _EuiCollapsibleNavItemDisplayProps & + Pick & + Required> & { buttonContent: ReactNode; - items: EuiCollapsibleNavSubItemProps[]; }; /** @@ -48,10 +41,8 @@ export const EuiCollapsibleNavAccordion: FunctionComponent< id, className, items, - href, // eslint-disable-line local/href-with-rel isSubItem, isSelected, - linkProps, accordionProps, buttonContent, children: _children, // Make sure this isn't spread @@ -69,32 +60,16 @@ export const EuiCollapsibleNavAccordion: FunctionComponent< accordionProps?.css, ]; - const isTitleInteractive = !!(href || linkProps?.onClick); - - // Stop propagation on the title so that the accordion toggle doesn't occur on click - // (should only occur on accordion arrow click for UX consistency) - const stopPropagationClick = useCallback( - (e: MouseEvent & MouseEvent) => { - e.stopPropagation(); - linkProps?.onClick?.(e); - }, - [linkProps?.onClick] // eslint-disable-line react-hooks/exhaustive-deps - ); - return ( {buttonContent} diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.test.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.test.tsx index a40e31af398..582593d8cea 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.test.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.test.tsx @@ -19,9 +19,7 @@ describe('EuiCollapsibleNavGroup', () => { items: [{ title: 'sub item' }], }; - shouldRenderCustomStyles(, { - childProps: ['linkProps'], - }); + shouldRenderCustomStyles(); it('renders as a top level item', () => { const { container } = render( @@ -45,12 +43,4 @@ describe('EuiCollapsibleNavGroup', () => { const header = container.querySelector('.euiCollapsibleNavLink'); expect(header?.className).toContain('isSelected'); }); - - it('renders interactive headers', () => { - const { container } = render( - - ); - const header = container.querySelector('.euiCollapsibleNavLink'); - expect(header?.nodeName).toEqual('A'); - }); }); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.tsx index b63ac6e82f7..0cafca8f208 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_group.tsx @@ -12,21 +12,18 @@ import classNames from 'classnames'; import { useEuiTheme, useGeneratedHtmlId } from '../../../services'; import { + type _SharedEuiCollapsibleNavItemProps, + type _EuiCollapsibleNavItemDisplayProps, + type EuiCollapsibleNavItemProps, EuiCollapsibleNavSubItems, - EuiCollapsibleNavSubItemProps, - _SharedEuiCollapsibleNavItemProps, - _EuiCollapsibleNavItemDisplayProps, } from './collapsible_nav_item'; import { euiCollapsibleNavItemVariables } from './collapsible_nav_item.styles'; import { EuiCollapsibleNavLink } from './collapsible_nav_link'; -type EuiCollapsibleNavGroupProps = Omit< - _SharedEuiCollapsibleNavItemProps, - 'items' | 'accordionProps' -> & - _EuiCollapsibleNavItemDisplayProps & { +type EuiCollapsibleNavGroupProps = _SharedEuiCollapsibleNavItemProps & + _EuiCollapsibleNavItemDisplayProps & + Required> & { header: ReactNode; - items: EuiCollapsibleNavSubItemProps[]; }; /** @@ -42,11 +39,9 @@ export const EuiCollapsibleNavGroup: FunctionComponent< > = ({ className, header, - href, // eslint-disable-line local/href-with-rel items, isSubItem, isSelected, - linkProps, children: _children, // Make sure this isn't spread ...rest }) => { @@ -68,12 +63,10 @@ export const EuiCollapsibleNavGroup: FunctionComponent< return (
{header} @@ -82,7 +75,7 @@ export const EuiCollapsibleNavGroup: FunctionComponent< isSubItem={isSubItem} className="euiCollapsibleNavGroup__children" role="group" - aria-labelledby={linkProps?.id || labelledById} + aria-labelledby={labelledById} />
); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx index 0845978804b..fa98a54c2e0 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.stories.tsx @@ -112,17 +112,14 @@ export const EdgeCaseTesting: Story = { { ...args, title: 'Not a link' }, { ...args, - title: 'Nested accordion - span', + title: 'Nested accordion', items: [{ title: 'grandchild' }, { title: 'grandchild 2' }], }, { ...args, - title: 'Nested accordion - link', - href: '#', - items: [ - { title: 'grandchild', href: '#' }, - { title: 'grandchild 2', href: '#' }, - ], + title: 'Nested group', + isCollapsible: false, + items: [{ title: 'grandchild' }, { title: 'grandchild 2' }], }, { renderItem: () => }, { @@ -148,42 +145,20 @@ export const EdgeCaseTesting: Story = { }, ]} /> - - { + shouldRenderCustomStyles(, { + childProps: ['linkProps'], + }); shouldRenderCustomStyles( - , - { childProps: ['linkProps', 'accordionProps'] } + , + { childProps: ['accordionProps'] } ); it('renders a top level accordion if items exist', () => { @@ -37,17 +36,31 @@ describe('EuiCollapsibleNavItem', () => { expect(container.firstChild).toMatchSnapshot(); }); - it('renders a top level link if items are missing or empty', () => { + it('does not pass the `href` prop to the accordion/group title', () => { const { container } = render( + // @ts-expect-error - should warn about `href` ); - expect(container.firstChild).toHaveClass('euiLink'); + expect(container.querySelector('a')).not.toBeInTheDocument(); + }); + + it('renders a top level group if items exist and `isCollapsible` is set to false', () => { + const { container } = render( + + ); + + expect(container.firstChild).toHaveClass('euiCollapsibleNavGroup'); expect(container.firstChild).toMatchSnapshot(); }); diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.tsx index 3b929d6a9ff..b887835c01f 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_item.tsx @@ -34,36 +34,6 @@ import { export type _SharedEuiCollapsibleNavItemProps = HTMLAttributes & CommonProps & { - /** - * The nav item link. - * If not included, and no `onClick` is specified, the nav item - * will render as an non-interactive ``. - */ - href?: string; - /** - * Will render either an accordion or group of nested child item links. - * - * Accepts any #EuiCollapsibleNavItemProps. Or, to render completely custom - * subitem content, pass an object with a `renderItem` callback. - */ - items?: EuiCollapsibleNavSubItemProps[]; - /** - * If set to false, will (visually) render an always-open accordion that cannot - * be toggled closed. Ignored if `items` is not passed. - * - * @default true - */ - isCollapsible?: boolean; - /** - * If `items` is specified, and `isCollapsible` is not set to false, you may - * use this prop to pass any prop that `EuiAccordion` accepts, including props - * that control the toggled state of the accordion (e.g. `initialIsOpen`, `forceState`) - */ - accordionProps?: Partial; - /** - * If a `href` is specified, use this prop to pass any prop that `EuiLink` accepts - */ - linkProps?: Partial; /** * Highlights whether an item is currently selected, e.g. * if the user is on the same page as the nav link @@ -71,7 +41,7 @@ export type _SharedEuiCollapsibleNavItemProps = HTMLAttributes & isSelected?: boolean; }; -export type EuiCollapsibleNavItemProps = { +export type EuiCollapsibleNavItemProps = _SharedEuiCollapsibleNavItemProps & { /** * Required text to render as the nav item title */ @@ -90,7 +60,46 @@ export type EuiCollapsibleNavItemProps = { * Optional props to pass to the title icon */ iconProps?: Partial; -} & _SharedEuiCollapsibleNavItemProps; +} & ExclusiveUnion< + { + /** + * The nav item link. + * + * If not included, and no `onClick` is specified, the nav item + * will render as an non-interactive ``. + * + * Should not be used together with `items`, as the title will + * trigger the accordion collapse/expand action instead of a link. + */ + href?: string; + /** + * If a `href` is specified, use this prop to pass any prop that `EuiLink` accepts + */ + linkProps?: Partial; + }, + { + /** + * Will render either an accordion or group of nested child item links. + * + * Accepts any #EuiCollapsibleNavItemProps. Or, to render completely custom + * subitem content, pass an object with a `renderItem` callback. + */ + items: EuiCollapsibleNavSubItemProps[]; + /** + * If set to false, will (visually) render an always-open accordion that cannot + * be toggled closed. Ignored if `items` is not passed. + * + * @default true + */ + isCollapsible?: boolean; + /** + * If `items` is specified, and `isCollapsible` is not set to false, you may + * use this prop to pass any prop that `EuiAccordion` accepts, including props + * that control the toggled state of the accordion (e.g. `initialIsOpen`, `forceState`) + */ + accordionProps?: Partial; + } + >; export type EuiCollapsibleNavCustomSubItem = { renderItem: () => ReactNode; @@ -122,9 +131,11 @@ const EuiCollapsibleNavItemDisplay: FunctionComponent< titleElement, icon, iconProps, + href, // eslint-disable-line local/href-with-rel + linkProps, items, isCollapsible = true, - accordionProps, // Ensure this isn't spread to non-accordions + accordionProps, children, // Ensure children isn't spread ...props }) => { @@ -137,7 +148,7 @@ const EuiCollapsibleNavItemDisplay: FunctionComponent< /> ); - if (items && items.length > 0) { + if (items) { if (isCollapsible) { return ( {headerContent} diff --git a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_link.tsx b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_link.tsx index 4bc70a4285f..6a8d406e2e8 100644 --- a/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_link.tsx +++ b/src/components/collapsible_nav_beta/collapsible_nav_item/collapsible_nav_link.tsx @@ -15,15 +15,14 @@ import { EuiLink, EuiLinkProps } from '../../link'; import type { _SharedEuiCollapsibleNavItemProps, _EuiCollapsibleNavItemDisplayProps, + EuiCollapsibleNavItemProps, } from './collapsible_nav_item'; import { euiCollapsibleNavLinkStyles } from './collapsible_nav_link.styles'; type EuiCollapsibleNavLinkProps = Omit & - Omit< - _SharedEuiCollapsibleNavItemProps, - 'items' | 'isCollapsible' | 'accordionProps' - > & - _EuiCollapsibleNavItemDisplayProps & { + _SharedEuiCollapsibleNavItemProps & + _EuiCollapsibleNavItemDisplayProps & + Pick & { children: ReactNode; isInteractive?: boolean; isNotAccordion?: boolean;