From bf65dda1cabd82e7aa4cee4e80c8b13a5ddca3b5 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 8 Feb 2024 13:56:00 -0800 Subject: [PATCH 01/19] Split out `EuiSideNavHeading` sub-component - [perf] removes the need for `let headingNode` - cleans up and organizes code --- src-docs/src/views/side_nav/props.tsx | 2 +- .../__snapshots__/side_nav.test.tsx.snap | 8 +- src/components/side_nav/_side_nav.scss | 4 - src/components/side_nav/_side_nav_heading.tsx | 62 ++++++++++++++ src/components/side_nav/side_nav.stories.tsx | 2 +- src/components/side_nav/side_nav.tsx | 83 ++++--------------- 6 files changed, 87 insertions(+), 74 deletions(-) create mode 100644 src/components/side_nav/_side_nav_heading.tsx diff --git a/src-docs/src/views/side_nav/props.tsx b/src-docs/src/views/side_nav/props.tsx index d459d01a20c..f5afb109231 100644 --- a/src-docs/src/views/side_nav/props.tsx +++ b/src-docs/src/views/side_nav/props.tsx @@ -5,7 +5,7 @@ export const EuiSideNavItem: FunctionComponent< EuiSideNavItemType > = () =>
; -import { EuiSideNavHeadingProps } from '../../../../src/components/side_nav/side_nav'; +import { EuiSideNavHeadingProps } from '../../../../src/components/side_nav/_side_nav_heading'; export const EuiSideNavHeading: FunctionComponent< EuiSideNavHeadingProps > = () =>
; diff --git a/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap b/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap index 31b7ef46d28..06d8db991bd 100644 --- a/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap +++ b/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap @@ -20,7 +20,7 @@ exports[`EuiSideNav props heading accepts more headingProps 1`] = ` >

@@ -39,7 +39,7 @@ exports[`EuiSideNav props heading is hidden with screenReaderOnly 1`] = ` class="euiSideNav" >

Side Nav Heading @@ -57,7 +57,7 @@ exports[`EuiSideNav props heading is rendered 1`] = ` class="euiSideNav" >

Side Nav Heading @@ -539,10 +539,12 @@ exports[`EuiSideNav props mobileBreakpoints can be adjusted is rendered 1`] = ` class="euiSideNav" >

); - const { - screenReaderOnly: headingScreenReaderOnly = false, - element: HeadingElement = 'h2', - ...titleProps - } = headingProps!; - const hasMobileVersion = mobileBreakpoints && mobileBreakpoints.length > 0; - const hasHeader = !!heading; - let headingNode; - - const sharedHeadingProps = { - id: headingProps?.id || this.generateId('heading'), - className: headingProps?.className, - 'data-test-subj': headingProps?.['data-test-subj'], - 'aria-label': headingProps?.['aria-label'], - }; - - if (hasHeader) { - headingNode = ( - {heading} - ); - - if (headingScreenReaderOnly) { - headingNode = {headingNode}; - } else { - headingNode = ( - - {heading} - - ); - } - } + const headingId = headingProps?.id || this.generateId('heading'); let mobileNode; const breakpoints: EuiBreakpointSize[] | undefined = mobileBreakpoints; if (hasMobileVersion) { mobileNode = ( - From d08999329d471a5f8a662622eb3d978e4d634136 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Thu, 8 Feb 2024 14:35:43 -0800 Subject: [PATCH 03/19] Remove unnecssary `let mobileNode` in favor of inline conditional JSX + remove extra unnecessary `breakpoints` const --- src/components/side_nav/side_nav.tsx | 86 +++++++++++++--------------- 1 file changed, 40 insertions(+), 46 deletions(-) diff --git a/src/components/side_nav/side_nav.tsx b/src/components/side_nav/side_nav.tsx index e72708abdfa..94e555c5bbd 100644 --- a/src/components/side_nav/side_nav.tsx +++ b/src/components/side_nav/side_nav.tsx @@ -188,54 +188,48 @@ export class EuiSideNav extends Component> { const headingId = headingProps?.id || this.generateId('heading'); const headingScreenReaderOnly = !!headingProps?.screenReaderOnly; - let mobileNode; - const breakpoints: EuiBreakpointSize[] | undefined = mobileBreakpoints; - if (hasMobileVersion) { - mobileNode = ( - - - - ); - } - return ( <> - {mobileNode} - + {hasMobileVersion && ( + + + + )} + @@ -27,7 +27,7 @@ exports[`EuiSideNav props heading accepts more headingProps 1`] = ` Side Nav Heading
@@ -45,7 +45,7 @@ exports[`EuiSideNav props heading is hidden with screenReaderOnly 1`] = ` Side Nav Heading
@@ -63,7 +63,7 @@ exports[`EuiSideNav props heading is rendered 1`] = ` Side Nav Heading
@@ -71,10 +71,34 @@ exports[`EuiSideNav props heading is rendered 1`] = ` exports[`EuiSideNav props isOpenOnMobile defaults to false 1`] = ` @@ -82,10 +106,35 @@ exports[`EuiSideNav props isOpenOnMobile defaults to false 1`] = ` exports[`EuiSideNav props isOpenOnMobile is rendered when specified as true 1`] = ` @@ -96,7 +145,7 @@ exports[`EuiSideNav props items is rendered 1`] = ` class="euiSideNav" >

@@ -573,7 +622,7 @@ exports[`EuiSideNav props mobileBreakpoints can be adjusted null is rendered 1`] class="euiSideNav" >
diff --git a/src/components/side_nav/side_nav.stories.tsx b/src/components/side_nav/side_nav.stories.tsx index 15d269c1026..35c00e7d736 100644 --- a/src/components/side_nav/side_nav.stories.tsx +++ b/src/components/side_nav/side_nav.stories.tsx @@ -29,7 +29,7 @@ const meta: Meta = { }, decorators: [ (Story) => ( -
+
{/* The side nav is visually easier to see with the width set */}
@@ -98,13 +98,13 @@ export const MobileSideNav: Story = { isOpenOnMobile: true, items: sharedSideNavItems, mobileTitle: 'Toggle isOpenOnMobile in the controls panel', + headingProps: { size: 'xxs' }, }, // This story demos the side nav on smaller screens; removing other props to streamline controls argTypes: hideStorybookControls([ 'aria-label', 'children', 'heading', - 'headingProps', 'items', 'renderItem', 'truncate', diff --git a/src/components/side_nav/side_nav.styles.ts b/src/components/side_nav/side_nav.styles.ts new file mode 100644 index 00000000000..cb58d335505 --- /dev/null +++ b/src/components/side_nav/side_nav.styles.ts @@ -0,0 +1,62 @@ +/* + * 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 { euiCanAnimate, logicalCSS } from '../../global_styling'; + +export const euiSideNavMobileStyles = (euiThemeContext: UseEuiTheme) => { + const { euiTheme } = euiThemeContext; + + const fastTransition = `${euiTheme.animation.extraFast} ${euiTheme.animation.resistance}`; + const slowTransition = `${euiTheme.animation.extraSlow} ${euiTheme.animation.resistance}`; + + return { + // Mobile button + euiSideNav__mobileToggle: css` + ${logicalCSS('width', '100%')} + ${logicalCSS('height', 'auto')} + + /* Inherit from the wrapping heading element */ + padding: 1em; + font-size: inherit; + text-align: start; + + border-radius: 0; + ${logicalCSS('border-bottom', euiTheme.border.thin)} + `, + euiSideNav__mobileToggleContent: css` + justify-content: space-between; + `, + + // Mobile content + content: { + euiSideNav__mobileContent: css` + ${euiCanAnimate} { + transition: max-block-size ${fastTransition}, + padding-block ${fastTransition}, opacity ${slowTransition}, + visibility ${slowTransition}; + } + `, + hidden: css` + overflow: hidden; + visibility: hidden; + opacity: 0; + max-block-size: 0; + padding-inline: ${euiTheme.size.l}; + `, + open: css` + visibility: visible; + opacity: 1; + max-block-size: 5000px; + padding: ${euiTheme.size.l}; + `, + }, + }; +}; diff --git a/src/components/side_nav/side_nav.test.tsx b/src/components/side_nav/side_nav.test.tsx index c6f88aee996..1df520b3d34 100644 --- a/src/components/side_nav/side_nav.test.tsx +++ b/src/components/side_nav/side_nav.test.tsx @@ -14,6 +14,8 @@ import { render } from '../../test/rtl'; import { EuiSideNav } from './side_nav'; import { RenderItem } from './side_nav_item'; +const JSDOM_BREAKPOINT = ['l']; + describe('EuiSideNav', () => { shouldRenderCustomStyles(, { childProps: ['headingProps'], @@ -29,7 +31,7 @@ describe('EuiSideNav', () => { describe('isOpenOnMobile', () => { test('defaults to false', () => { const { container } = render( - + ); expect(container.firstChild).toMatchSnapshot(); @@ -39,6 +41,7 @@ describe('EuiSideNav', () => { const { container } = render( ); @@ -102,8 +105,6 @@ describe('EuiSideNav', () => { }); describe('mobile behavior', () => { - const JSDOM_BREAKPOINT = ['l']; - it('is overridden by `mobileTitle`', () => { const { getByTestSubject } = render( = T & CommonProps & { @@ -174,24 +175,21 @@ export class EuiSideNavClass extends Component< const classes = classNames('euiSideNav', className, { 'euiSideNav-isOpenMobile': isOpenOnMobile, }); + const styles = euiSideNavMobileStyles(theme); - // To support the extra CSS needed to show/hide/animate the content, - // We add a className for every breakpoint supported - const contentClasses = classNames( - 'euiSideNav__content', - mobileBreakpoints?.map( - (breakpointName) => `euiSideNav__contentMobile-${breakpointName}` - ) - ); + const contentClasses = classNames('euiSideNav__content'); const sideNavContentId = this.generateId('content'); - const navContent = ( -
- {this.renderTree(items)} -
- ); + const mobileContentStyles = [ + styles.content.euiSideNav__mobileContent, + isOpenOnMobile ? styles.content.open : styles.content.hidden, + ]; const hasMobileVersion = mobileBreakpoints && mobileBreakpoints.length > 0; const mobileToggleText = mobileTitle || heading; + const mobileHeadingUnset = { + marginBlockEnd: 0, + label: 'mobile', + }; const headingId = headingProps?.id || this.generateId('heading'); const headingScreenReaderOnly = !!headingProps?.screenReaderOnly; @@ -205,6 +203,7 @@ export class EuiSideNavClass extends Component< id={headingId} {...headingProps} screenReaderOnly={false} + css={mobileHeadingUnset} > extends Component< {(mobileToggleAriaLabel: string) => ( extends Component< )} - {navContent} +
+ {this.renderTree(items)} +
)} @@ -248,7 +254,9 @@ export class EuiSideNavClass extends Component< {heading} )} - {navContent} +
+ {this.renderTree(items)} +
From 031c60c3793bcbac4162bf9a92fb3a174faba689 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 9 Feb 2024 08:50:48 -0800 Subject: [PATCH 06/19] Set up EuiSideNavItem Emotion conversion - start with button component - reorder imports + convert function to arrow syntax --- src/components/side_nav/_side_nav_item.scss | 7 ---- .../side_nav/side_nav_item.styles.ts | 32 +++++++++++++++++++ src/components/side_nav/side_nav_item.tsx | 19 +++++++---- 3 files changed, 44 insertions(+), 14 deletions(-) create mode 100644 src/components/side_nav/side_nav_item.styles.ts diff --git a/src/components/side_nav/_side_nav_item.scss b/src/components/side_nav/_side_nav_item.scss index e4037f52ac3..af3d72101e4 100644 --- a/src/components/side_nav/_side_nav_item.scss +++ b/src/components/side_nav/_side_nav_item.scss @@ -6,13 +6,6 @@ */ .euiSideNavItemButton { - @include euiFontSizeS; - text-align: left; /* 1 */ - display: block; - width: 100%; - padding: ($euiSizeXS / 2) 0; - color: inherit; /* 2 */ - &.euiSideNavItemButton--isClickable:not(:disabled) { &:hover { cursor: pointer; diff --git a/src/components/side_nav/side_nav_item.styles.ts b/src/components/side_nav/side_nav_item.styles.ts new file mode 100644 index 00000000000..5639b18cca2 --- /dev/null +++ b/src/components/side_nav/side_nav_item.styles.ts @@ -0,0 +1,32 @@ +/* + * 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 } from '../../global_styling'; + +export const euiSideNavItemButtonStyles = (euiThemeContext: UseEuiTheme) => { + const { euiTheme } = euiThemeContext; + + return { + euiSideNavItemButton: css` + display: block; + ${logicalCSS('width', '100%')} /* Needed for nested items */ + padding-block: ${euiTheme.size.xxs}; + + font-size: ${euiFontSize(euiThemeContext, 's').fontSize}; + line-height: ${euiFontSize(euiThemeContext, 'm').lineHeight}; + + /* Text-align defaults to center, so we have to override that. */ + text-align: start; + /* Color the text at the item level and then have the button inherit so overrides are easier */ + color: inherit; + `, + }; +}; diff --git a/src/components/side_nav/side_nav_item.tsx b/src/components/side_nav/side_nav_item.tsx index 59079bd0c81..68a97b75e2c 100644 --- a/src/components/side_nav/side_nav_item.tsx +++ b/src/components/side_nav/side_nav_item.tsx @@ -16,13 +16,13 @@ import React, { } from 'react'; import classNames from 'classnames'; +import { useEuiTheme, getSecureRelForTarget } from '../../services'; +import { validateHref } from '../../services/security/href_validator'; import { CommonProps } from '../common'; - +import { EuiInnerText } from '../inner_text'; import { EuiIcon } from '../icon'; -import { getSecureRelForTarget } from '../../services'; -import { validateHref } from '../../services/security/href_validator'; -import { EuiInnerText } from '../inner_text'; +import { euiSideNavItemButtonStyles } from './side_nav_item.styles'; /** * The props that are exposed to, or altered for, the consumer @@ -145,7 +145,7 @@ const DefaultRenderItem = ({ ); }; -export function EuiSideNavItem< +export const EuiSideNavItem = < T extends _EuiSideNavItemButtonProps & _EuiSideNavItemProps & { renderItem?: (props: any) => JSX.Element } >({ @@ -167,7 +167,9 @@ export function EuiSideNavItem< buttonClassName, childrenOnly, ...rest -}: EuiSideNavItemProps) { +}: EuiSideNavItemProps) => { + const euiTheme = useEuiTheme(); + const isHrefValid = !_href || validateHref(_href); const href = isHrefValid ? _href : ''; const isClickable = onClick || href; @@ -216,6 +218,8 @@ export function EuiSideNavItem< }, buttonClassName ); + const buttonStyles = euiSideNavItemButtonStyles(euiTheme); + const buttonCssStyles = [buttonStyles.euiSideNavItemButton]; let caret; @@ -250,6 +254,7 @@ export function EuiSideNavItem< rel, target, onClick: childrenOnly ? toggleItemOpen : onClick, + css: buttonCssStyles, className: buttonClasses, children: buttonContent, }; @@ -259,4 +264,4 @@ export function EuiSideNavItem< {childItems}
); -} +}; From 9762a9850b4d6cdc2f5d2f53c43983f361a36ed3 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 9 Feb 2024 09:14:22 -0800 Subject: [PATCH 07/19] Convert item button modifier styles + remove `--isClickable` modifier (use CSS selectors instead) --- src/components/side_nav/_side_nav_item.scss | 32 ------------------- .../side_nav/side_nav_item.styles.ts | 28 ++++++++++++++++ src/components/side_nav/side_nav_item.tsx | 7 ++-- 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/components/side_nav/_side_nav_item.scss b/src/components/side_nav/_side_nav_item.scss index af3d72101e4..48c5277e55d 100644 --- a/src/components/side_nav/_side_nav_item.scss +++ b/src/components/side_nav/_side_nav_item.scss @@ -1,39 +1,7 @@ /** - * 1. Text-align defaults to center, so we have to override that. - * 2. Color the text at the item level and then have the button inherit so overrides are easier * 3. Enable ellipsis overflow to work (https://css-tricks.com/flexbox-truncated-text/) - * 4. Restrict the underline to the button __label so it doesn't affect other components that might live within */ -.euiSideNavItemButton { - &.euiSideNavItemButton--isClickable:not(:disabled) { - &:hover { - cursor: pointer; - } - - &:hover, - &:focus { - .euiSideNavItemButton__label { - // A lingering focus will stay underlined even if it doesn't get the `isSelected` prop - text-decoration: underline; /* 4 */ - } - } - } - - &.euiSideNavItemButton-isSelected { - color: $euiSideNavSelectedTextcolor; - font-weight: $euiFontWeightBold; - - .euiSideNavItemButton__label { - text-decoration: underline; /* 4 */ - } - } - - &:disabled { - @include euiDisabledState($euiSideNavDisabledTextcolor); - } -} - .euiSideNavItemButton__content { display: flex; align-items: center; diff --git a/src/components/side_nav/side_nav_item.styles.ts b/src/components/side_nav/side_nav_item.styles.ts index 5639b18cca2..ce231549433 100644 --- a/src/components/side_nav/side_nav_item.styles.ts +++ b/src/components/side_nav/side_nav_item.styles.ts @@ -27,6 +27,34 @@ export const euiSideNavItemButtonStyles = (euiThemeContext: UseEuiTheme) => { text-align: start; /* Color the text at the item level and then have the button inherit so overrides are easier */ color: inherit; + + &:is(a, button):not(:disabled) { + &:hover { + cursor: pointer; + } + + &:hover, + &:focus { + /* Restrict the underline to the button __label so it doesn't affect other components that might live within */ + .euiSideNavItemButton__label { + text-decoration: underline; + } + } + } + + &:disabled { + cursor: not-allowed; + color: ${euiTheme.colors.disabledText}; + } + `, + isSelected: css` + color: ${euiTheme.colors.primaryText}; + font-weight: ${euiTheme.font.weight.bold}; + + /* Restrict the underline to the button __label so it doesn't affect other components that might live within */ + .euiSideNavItemButton__label { + text-decoration: underline; + } `, }; }; diff --git a/src/components/side_nav/side_nav_item.tsx b/src/components/side_nav/side_nav_item.tsx index 68a97b75e2c..ba36fb68f43 100644 --- a/src/components/side_nav/side_nav_item.tsx +++ b/src/components/side_nav/side_nav_item.tsx @@ -172,7 +172,6 @@ export const EuiSideNavItem = < const isHrefValid = !_href || validateHref(_href); const href = isHrefValid ? _href : ''; - const isClickable = onClick || href; // Forcing accordion style item if not linked, but has children const [itemIsOpen, setItemIsOpen] = useState(isOpen); @@ -212,14 +211,16 @@ export const EuiSideNavItem = < const buttonClasses = classNames( 'euiSideNavItemButton', { - 'euiSideNavItemButton--isClickable': isClickable, 'euiSideNavItemButton-isOpen': depth > 0 && itemIsOpen && !isSelected, 'euiSideNavItemButton-isSelected': isSelected, }, buttonClassName ); const buttonStyles = euiSideNavItemButtonStyles(euiTheme); - const buttonCssStyles = [buttonStyles.euiSideNavItemButton]; + const buttonCssStyles = [ + buttonStyles.euiSideNavItemButton, + isSelected && buttonStyles.isSelected, + ]; let caret; From 0f15f89b9eec4fd242c103832015f67fbc097c66 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 9 Feb 2024 09:29:20 -0800 Subject: [PATCH 08/19] Convert item button child styles + lots of cleanup - Remove unnecessary `.euiSideNavItemButton__content` wrapper - just put it on the parent `
@@ -224,106 +212,86 @@ exports[`EuiSideNav props items renders items having { forceOpen: true } in open id="euiSideNav_generated-id_content" >
- - A - + A
- - B - + B
- - E - + E
@@ -346,43 +314,35 @@ exports[`EuiSideNav props items renders items using a specified callback 1`] = ` id="euiSideNav_generated-id_content" >
- - A - + A
- - B - + B
@@ -401,65 +361,53 @@ exports[`EuiSideNav props items renders items which are links 1`] = ` id="euiSideNav_generated-id_content" >
- - A - + A
- - B - + B
@@ -477,100 +425,80 @@ exports[`EuiSideNav props items renders selected item and automatically opens pa id="euiSideNav_generated-id_content" >
- - A - + A
- - B - + B
- - D - + D
- - E - + E
diff --git a/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap b/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap index 8bb99340e41..a1ae1c169bc 100644 --- a/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap +++ b/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap @@ -2,22 +2,18 @@ exports[`EuiSideNavItem can be disabled 1`] = `
@@ -25,20 +21,16 @@ exports[`EuiSideNavItem can be disabled 1`] = ` exports[`EuiSideNavItem can be emphasized 1`] = `
- - Children - + Children
@@ -46,19 +38,15 @@ exports[`EuiSideNavItem can be emphasized 1`] = ` exports[`EuiSideNavItem can have truncation turned off 1`] = `
- - Children - + Children
@@ -66,21 +54,17 @@ exports[`EuiSideNavItem can have truncation turned off 1`] = ` exports[`EuiSideNavItem href is rendered 1`] = `
- -
@@ -88,21 +72,17 @@ exports[`EuiSideNavItem href is rendered 1`] = ` exports[`EuiSideNavItem href is rendered with rel 1`] = `
- -
@@ -110,23 +90,19 @@ exports[`EuiSideNavItem href is rendered with rel 1`] = ` exports[`EuiSideNavItem is rendered 1`] = `
- -
@@ -134,19 +110,15 @@ exports[`EuiSideNavItem is rendered 1`] = ` exports[`EuiSideNavItem isSelected defaults to false 1`] = `
- -
@@ -154,19 +126,15 @@ exports[`EuiSideNavItem isSelected defaults to false 1`] = ` exports[`EuiSideNavItem isSelected is rendered when specified as true 1`] = `
- -
@@ -174,21 +142,17 @@ exports[`EuiSideNavItem isSelected is rendered when specified as true 1`] = ` exports[`EuiSideNavItem preserves child's classes 1`] = `
- -
From fa184498e58cc2839cad17dab14f311b5dd1e729 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 9 Feb 2024 11:38:18 -0800 Subject: [PATCH 17/19] Make all style-related props pass correctly to `EuiSideNavItem` doesn't need a changelog as this is not a publicly exported component --- src/components/side_nav/side_nav_item.test.tsx | 5 ++++- src/components/side_nav/side_nav_item.tsx | 10 +++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/components/side_nav/side_nav_item.test.tsx b/src/components/side_nav/side_nav_item.test.tsx index 8491174e010..9616b660550 100644 --- a/src/components/side_nav/side_nav_item.test.tsx +++ b/src/components/side_nav/side_nav_item.test.tsx @@ -7,12 +7,15 @@ */ import React from 'react'; -import { requiredProps } from '../../test/required_props'; import { render } from '../../test/rtl'; +import { shouldRenderCustomStyles } from '../../test/internal'; +import { requiredProps } from '../../test/required_props'; import { EuiSideNavItem } from './side_nav_item'; describe('EuiSideNavItem', () => { + shouldRenderCustomStyles(test); + test('is rendered', () => { const { container } = render( diff --git a/src/components/side_nav/side_nav_item.tsx b/src/components/side_nav/side_nav_item.tsx index dbf45aa7217..f5e3e8c4134 100644 --- a/src/components/side_nav/side_nav_item.tsx +++ b/src/components/side_nav/side_nav_item.tsx @@ -7,6 +7,7 @@ */ import React, { + HTMLAttributes, ReactNode, ReactElement, MouseEventHandler, @@ -75,6 +76,10 @@ export interface _EuiSideNavItemProps { * Passed to the actual `.euiSideNavItemButton` element */ buttonClassName?: string; + /** + * className, css, and style are passed to the parent wrapper, not the button + */ + style?: HTMLAttributes['style']; // Exposed as different prop type to consumer items?: ReactNode; // Not exposed to consumer @@ -165,6 +170,8 @@ export const EuiSideNavItem = < renderItem: RenderItem = DefaultRenderItem, depth = 0, className, + css, + style, truncate = true, emphasize, buttonClassName, @@ -210,6 +217,7 @@ export const EuiSideNavItem = < isTrunk && styles.trunk, isBranch && styles.branch, emphasize && styles.emphasized, + css, ]; const itemsStyles = hasChildItems && [ styles.items.euiSideNavItem__items, @@ -241,7 +249,7 @@ export const EuiSideNavItem = < ]; return ( -
+
Date: Fri, 9 Feb 2024 13:07:19 -0800 Subject: [PATCH 18/19] Restore `.euiSideNavItemButton__content` wrapper - actually needed for custom `renderItem` usages that might not apply the passed button styles down + tweak stories to show an example of correctly passed styles --- changelogs/upcoming/7521.md | 1 - .../__snapshots__/side_nav.test.tsx.snap | 208 ++++++++++++------ .../__snapshots__/side_nav_item.test.tsx.snap | 88 +++++--- src/components/side_nav/side_nav.stories.tsx | 25 ++- .../side_nav/side_nav_item.styles.ts | 9 +- src/components/side_nav/side_nav_item.tsx | 41 ++-- 6 files changed, 251 insertions(+), 121 deletions(-) diff --git a/changelogs/upcoming/7521.md b/changelogs/upcoming/7521.md index d26ad262fe8..abf1dfb39f7 100644 --- a/changelogs/upcoming/7521.md +++ b/changelogs/upcoming/7521.md @@ -17,4 +17,3 @@ - `$euiSideNavSelectedTextcolor` - `$euiSideNavDisabledTextcolor` - Removed the `euiSideNavEmbellish` Sass mixin. Use the new `EuiPageSidebar` `hasEmbellish` prop instead -- Removed the `.euiSideNavItemButton__content` DOM wrapper. Target `.euiSideNavItemButton` directly instead diff --git a/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap b/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap index fb1de6c9c7e..58e307a5f25 100644 --- a/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap +++ b/src/components/side_nav/__snapshots__/side_nav.test.tsx.snap @@ -155,10 +155,14 @@ exports[`EuiSideNav props items is rendered 1`] = ` class="euiSideNavItemButton emotion-euiSideNavItemButton-root" > - A + + A +
- B + + B +
@@ -188,13 +196,17 @@ exports[`EuiSideNav props items is rendered 1`] = ` type="button" > - C + + C + + -
@@ -218,10 +230,14 @@ exports[`EuiSideNav props items renders items having { forceOpen: true } in open class="euiSideNavItemButton emotion-euiSideNavItemButton-root" > - A + + A +
- B + + B +
@@ -249,14 +269,18 @@ exports[`EuiSideNav props items renders items having { forceOpen: true } in open type="button" > - C + + C + + -
- D + + D + + -
- E + + E +
@@ -322,10 +354,14 @@ exports[`EuiSideNav props items renders items using a specified callback 1`] = ` href="http://www.elastic.co" > - A + + A +
- B + + B +
@@ -369,10 +409,14 @@ exports[`EuiSideNav props items renders items which are links 1`] = ` rel="noreferrer" > - A + + A +
- B + + B +
@@ -400,14 +448,18 @@ exports[`EuiSideNav props items renders items which are links 1`] = ` type="button" > - C + + C + + -
@@ -431,10 +483,14 @@ exports[`EuiSideNav props items renders selected item and automatically opens pa class="euiSideNavItemButton emotion-euiSideNavItemButton-root" > - A + + A +
- B + + B +
@@ -462,14 +522,18 @@ exports[`EuiSideNav props items renders selected item and automatically opens pa type="button" > - C + + C + + -
- D + + D +
@@ -495,10 +563,14 @@ exports[`EuiSideNav props items renders selected item and automatically opens pa class="euiSideNavItemButton emotion-euiSideNavItemButton-branch" > - E + + E +
diff --git a/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap b/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap index a1ae1c169bc..aeef93a39a4 100644 --- a/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap +++ b/src/components/side_nav/__snapshots__/side_nav_item.test.tsx.snap @@ -10,10 +10,14 @@ exports[`EuiSideNavItem can be disabled 1`] = ` type="button" > - Children + + Children +
@@ -27,10 +31,14 @@ exports[`EuiSideNavItem can be emphasized 1`] = ` class="euiSideNavItemButton emotion-euiSideNavItemButton-emphasized-root" > - Children + + Children +
@@ -44,9 +52,13 @@ exports[`EuiSideNavItem can have truncation turned off 1`] = ` class="euiSideNavItemButton emotion-euiSideNavItemButton-root" > - Children + + Children +
@@ -62,9 +74,13 @@ exports[`EuiSideNavItem href is rendered 1`] = ` rel="noreferrer" > -
@@ -80,9 +96,13 @@ exports[`EuiSideNavItem href is rendered with rel 1`] = ` rel="noopener noreferrer" > -
@@ -96,13 +116,17 @@ exports[`EuiSideNavItem is rendered 1`] = ` class="euiSideNavItemButton emotion-euiSideNavItemButton-root" > -
@@ -116,9 +140,13 @@ exports[`EuiSideNavItem isSelected defaults to false 1`] = ` class="euiSideNavItemButton emotion-euiSideNavItemButton-root" > -
@@ -132,9 +160,13 @@ exports[`EuiSideNavItem isSelected is rendered when specified as true 1`] = ` class="euiSideNavItemButton euiSideNavItemButton-isSelected emotion-euiSideNavItemButton-selected-root" > -
@@ -148,11 +180,15 @@ exports[`EuiSideNavItem preserves child's classes 1`] = ` class="euiSideNavItemButton emotion-euiSideNavItemButton-root" > -
diff --git a/src/components/side_nav/side_nav.stories.tsx b/src/components/side_nav/side_nav.stories.tsx index 35c00e7d736..50effd3989b 100644 --- a/src/components/side_nav/side_nav.stories.tsx +++ b/src/components/side_nav/side_nav.stories.tsx @@ -13,6 +13,7 @@ import { disableStorybookControls, } from '../../../.storybook/utils'; +import { EuiIcon } from '../icon'; import { EuiText } from '../text'; import { EuiSideNav, EuiSideNavProps } from './side_nav'; @@ -118,19 +119,33 @@ export const MobileSideNav: Story = { export const RenderItem: Story = { args: { - renderItem: ({ children }) => {children}, + renderItem: ({ children, ...rest }) => ( + + + + ), items: [ { name: 'Kibana', id: 'kibana', - }, - { - name: 'Observability', - id: 'observability', + icon: , + items: [ + { + name: 'Observability', + id: 'observability', + items: [ + { + name: 'Test', + id: 'test', + }, + ], + }, + ], }, { name: 'Security', id: 'security', + icon: , renderItem: ({ children }) => ( {children} ), diff --git a/src/components/side_nav/side_nav_item.styles.ts b/src/components/side_nav/side_nav_item.styles.ts index 20c47b23b10..8af08da36ed 100644 --- a/src/components/side_nav/side_nav_item.styles.ts +++ b/src/components/side_nav/side_nav_item.styles.ts @@ -101,11 +101,9 @@ export const euiSideNavItemButtonStyles = (euiThemeContext: UseEuiTheme) => { return { euiSideNavItemButton: css` - display: flex; - align-items: center; + display: block; ${logicalCSS('width', '100%')} /* Needed for nested items */ padding-block: ${euiTheme.size.xxs}; - gap: ${euiTheme.size.s}; font-size: ${euiFontSize(euiThemeContext, 's').fontSize}; line-height: ${lineHeightOverride}; @@ -173,6 +171,11 @@ export const euiSideNavItemButtonStyles = (euiThemeContext: UseEuiTheme) => { `, // Child elements + euiSideNavItemButton__content: css` + display: flex; + align-items: center; + gap: ${euiTheme.size.s}; + `, label: { euiSideNavItemButton__label: css` flex-grow: 1; diff --git a/src/components/side_nav/side_nav_item.tsx b/src/components/side_nav/side_nav_item.tsx index f5e3e8c4134..ae85efe02e4 100644 --- a/src/components/side_nav/side_nav_item.tsx +++ b/src/components/side_nav/side_nav_item.tsx @@ -259,26 +259,31 @@ export const EuiSideNavItem = < onClick={childrenOnly ? toggleItemOpen : onClick} {...rest} > - {icon} + + {icon} - - {(ref, innerText) => ( - - {children} - - )} - + + {(ref, innerText) => ( + + {children} + + )} + - {hasCaret && ( - - )} + {hasCaret && ( + + )} + {hasChildItems && ( From bfbe5ee0533bdaeaf13c20968cc386aaae2eb9ab Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Fri, 9 Feb 2024 13:40:28 -0800 Subject: [PATCH 19/19] [storybook] Improve default story to show off more styles/states + fix nested caret alignment --- src/components/side_nav/side_nav.stories.tsx | 84 +++++++++++++++---- .../side_nav/side_nav_item.styles.ts | 1 + 2 files changed, 67 insertions(+), 18 deletions(-) diff --git a/src/components/side_nav/side_nav.stories.tsx b/src/components/side_nav/side_nav.stories.tsx index 50effd3989b..e16371946b7 100644 --- a/src/components/side_nav/side_nav.stories.tsx +++ b/src/components/side_nav/side_nav.stories.tsx @@ -43,43 +43,91 @@ type Story = StoryObj; const sharedSideNavItems = [ { - name: 'Has nested children', - id: 'normal_children', + name: 'Root item with carets', + id: 'root_1', items: [ { - name: 'Child 1', - id: 'child_1', + name: 'Trunk 1', + id: 'trunk_1', items: [ { - name: 'Selected item', - id: 'selected_item', + name: 'Emphasized branch', + id: 'emphasized_branch', + icon: , + emphasize: true, + items: [ + { + name: 'Yet another branch', + id: 'branch_1', + onClick: () => {}, + }, + ], + }, + ], + }, + ], + }, + { + name: 'Root item without carets', + id: 'root_2', + items: [ + { + name: 'Trunk 2', + id: 'trunk_2', + onClick: () => {}, // Causes the caret to not render + items: [ + { + name: 'Branch 2', + id: 'branch_2', onClick: () => {}, - isSelected: true, - items: [], + items: [ + { + name: 'All parents are open because this branch is selected', + id: 'selected_branch', + onClick: () => {}, + isSelected: true, + truncate: false, + }, + ], }, ], }, ], }, { - name: 'Has forceOpen: true', - id: 'force_open', - forceOpen: true, + name: 'Root item with forced open children', + id: 'root_3', items: [ { - name: 'Child 3', - id: 'child_3', + name: 'Trunk 3', + id: 'trunk_3', + onClick: () => {}, + forceOpen: true, + items: [ + { + name: 'Disabled branch', + id: 'disabled_branch', + href: '#', + disabled: true, + icon: , + }, + ], }, ], }, { - name: 'Children only without link', - id: 'children_only', - onClick: undefined, + name: 'Root item with icon and truncation', + icon: , + id: 'root_4', items: [ { - name: 'Child 4', - id: 'child_4', + name: 'Very very long text, truncated by default', + id: 'truncated', + }, + { + name: 'Very very long text, truncate set to false', + id: 'truncateFalse', + truncate: false, }, ], }, diff --git a/src/components/side_nav/side_nav_item.styles.ts b/src/components/side_nav/side_nav_item.styles.ts index 8af08da36ed..ef2ba57c5dc 100644 --- a/src/components/side_nav/side_nav_item.styles.ts +++ b/src/components/side_nav/side_nav_item.styles.ts @@ -75,6 +75,7 @@ export const euiSideNavItemStyles = (euiThemeContext: UseEuiTheme) => { ${logicalCSS('margin-left', euiTheme.size.l)} `, trunk: css` + ${logicalCSS('width', '100%')} /* Needed for trunks with carets */ ${logicalCSS('margin-left', euiTheme.size.s)} `, branch: css`