From 0376df7e71958354474882e1c1b11aa13cda88c3 Mon Sep 17 00:00:00 2001 From: Alison Joseph Date: Tue, 9 Apr 2024 10:01:02 -0500 Subject: [PATCH] fix(tabs): refactor dismissable tab html (#16033) * fix(tabs): refactor dismissable tab html * fix: typescript * feat: add button reset, active state and fix number values * fix: disabled styles * fix: styling issues * fix: fullwidth tabs * fix: update close to remove & add tab name & hide on non dismissible * chore: code comments * fix: tests * fix: set focus after removing tab * fix: focus * fix: set focus to active tab * feat: update focus state after removing tab * chore: add code comments * fix: focus updates * fix: focus states * Update packages/react/src/components/Tabs/Tabs.tsx --------- Co-authored-by: Taylor Jones --- .../react/src/components/Tabs/Tabs-test.js | 14 +- packages/react/src/components/Tabs/Tabs.tsx | 162 ++++++++++++------ .../styles/scss/components/tabs/_tabs.scss | 117 +++++++++---- 3 files changed, 196 insertions(+), 97 deletions(-) diff --git a/packages/react/src/components/Tabs/Tabs-test.js b/packages/react/src/components/Tabs/Tabs-test.js index a384ea368ac3..32ee5f474509 100644 --- a/packages/react/src/components/Tabs/Tabs-test.js +++ b/packages/react/src/components/Tabs/Tabs-test.js @@ -264,7 +264,8 @@ describe('Tab', () => { expect( // eslint-disable-next-line testing-library/no-node-access - screen.getAllByLabelText('Press delete to close tab')[0].parentElement + screen.getAllByLabelText('Press delete to remove Tab Label 1 tab')[0] + .parentElement ).not.toHaveClass(`${prefix}--visually-hidden`); }); @@ -286,7 +287,8 @@ describe('Tab', () => { expect( // eslint-disable-next-line testing-library/no-node-access - screen.queryAllByLabelText('Press delete to close tab')[0].parentElement + screen.queryAllByLabelText('Press delete to remove Tab Label 1 tab')[0] + .parentElement ).toHaveClass(`${prefix}--visually-hidden`); }); @@ -307,7 +309,7 @@ describe('Tab', () => { ); await userEvent.click( - screen.getAllByLabelText('Press delete to close tab')[0] + screen.getAllByLabelText('Press delete to remove Tab Label 1 tab')[0] ); expect(onTabCloseRequest).toHaveBeenCalledTimes(1); }); @@ -329,7 +331,7 @@ describe('Tab', () => { ); await userEvent.click( - screen.getAllByLabelText('Press delete to close tab')[0] + screen.getAllByLabelText('Press delete to remove Tab Label 1 tab')[0] ); expect(onTabCloseRequest).not.toHaveBeenCalled(); }); @@ -421,7 +423,7 @@ describe('Tab', () => { ); expect( - screen.getAllByLabelText('Press delete to close tab')[0] + screen.getAllByLabelText('Press delete to remove Tab Label 1 tab')[0] ).not.toHaveClass(`${prefix}--visaully-hidden`); }); @@ -442,7 +444,7 @@ describe('Tab', () => { ); expect( - screen.getAllByLabelText('Press delete to close tab')[0] + screen.getAllByLabelText('Press delete to remove Tab Label 1 tab')[0] ).not.toHaveClass(`${prefix}--visaully-hidden`); expect(screen.getByTestId('svg')).toBeInTheDocument(); // eslint-disable-next-line testing-library/no-node-access diff --git a/packages/react/src/components/Tabs/Tabs.tsx b/packages/react/src/components/Tabs/Tabs.tsx index c058ce31a4dc..fab0367b0588 100644 --- a/packages/react/src/components/Tabs/Tabs.tsx +++ b/packages/react/src/components/Tabs/Tabs.tsx @@ -353,6 +353,7 @@ function TabList({ [`${prefix}--layout--size-lg`]: iconSize === 'lg', [`${prefix}--tabs--tall`]: hasSecondaryLabelTabs, [`${prefix}--tabs--full-width`]: distributeWidth, + [`${prefix}--tabs--dismissable`]: dismissable, }, customClassName ); @@ -809,7 +810,7 @@ const Tab = forwardRef(function Tab( onTabCloseRequest, } = React.useContext(TabsContext); const { index, hasSecondaryLabel, contained } = React.useContext(TabContext); - const dismissIconRef = useRef(null); + const dismissIconRef = useRef(null); const tabRef = useRef(null); const ref = useMergedRefs([forwardRef, tabRef]); const [ignoreHover, setIgnoreHover] = useState(false); @@ -849,6 +850,32 @@ const Tab = forwardRef(function Tab( const handleClose = (evt) => { evt.stopPropagation(); onTabCloseRequest?.(index); + + // set focus after removing tab + if (tabRef.current && tabRef.current.parentElement) { + // determine number of tabs, excluding disabled + const tabCount = Array.from( + tabRef.current.parentElement.childNodes + ).filter((node) => { + const element = node as HTMLElement; + return ( + element.classList.contains('cds--tabs__nav-link') && + !element.classList.contains('cds--tabs__nav-item--disabled') + ); + }).length; + + // if not removing last tab focus on next tab + if (tabRef.current && index + 1 !== tabCount) { + tabRef.current.focus(); + } + // if removing last tab focus on previous tab + else { + const prevTabIndex = (tabCount - 2) * 2; + ( + tabRef.current.parentElement.childNodes[prevTabIndex] as HTMLElement + )?.focus(); + } + } }; const handleKeyDown = (event) => { @@ -860,69 +887,90 @@ const Tab = forwardRef(function Tab( const DismissIcon = (
- + className={cx({ + [`${prefix}--tabs__nav-item--close`]: dismissable, + [`${prefix}--tabs__nav-item--close--hidden`]: !dismissable, + })}> +
); const hasIcon = Icon ?? dismissable; return ( - { - if (disabled) { - return; - } - setSelectedIndex(index); - onClick?.(evt); - }} - onKeyDown={handleKeyDown} - tabIndex={selectedIndex === index ? '0' : '-1'} - type="button"> -
- {dismissable && Icon && ( -
- {} -
- )} - {children} - {/* always rendering dismissIcon so we don't lose reference to it, otherwise events do not work when switching from/to dismissable state */} -
- {DismissIcon} - {!dismissable && Icon && } + <> + { + if (disabled) { + return; + } + setSelectedIndex(index); + onClick?.(evt); + }} + onKeyDown={handleKeyDown} + tabIndex={selectedIndex === index ? '0' : '-1'} + type="button"> +
+ {dismissable && Icon && ( +
+ {} +
+ )} + {children} + {!dismissable && Icon && ( +
+ {!dismissable && Icon && } +
+ )}
-
- {hasSecondaryLabel && secondaryLabel && ( - - {secondaryLabel} - - )} - + {hasSecondaryLabel && secondaryLabel && ( + + {secondaryLabel} + + )} + + {/* always rendering dismissIcon so we don't lose reference to it, otherwise events do not work when switching from/to dismissable state */} + {DismissIcon} + ); }); Tab.propTypes = { diff --git a/packages/styles/scss/components/tabs/_tabs.scss b/packages/styles/scss/components/tabs/_tabs.scss index a1a2b372eda0..a60d611f954f 100644 --- a/packages/styles/scss/components/tabs/_tabs.scss +++ b/packages/styles/scss/components/tabs/_tabs.scss @@ -277,14 +277,16 @@ &.#{$prefix}--tabs--contained .#{$prefix}--tabs__nav-item { background-color: $layer-accent; + // Draws the border without affecting the inner-content + box-shadow: convert.to-rem(-1px) 0 0 0 $border-strong; + margin-inline-start: 0; } &.#{$prefix}--tabs--contained - .#{$prefix}--tabs__nav-item + .#{$prefix}--tabs__nav-item--selected + + div + .#{$prefix}--tabs__nav-item { - // Draws the border without affecting the inner-content - box-shadow: convert.to-rem(-1px) 0 0 0 $border-strong; - margin-inline-start: 0; + box-shadow: convert.to-rem(-1px) 0 0 0 transparent; } .#{$prefix}--tabs__nav-item .#{$prefix}--tabs__nav-link { @@ -293,57 +295,104 @@ outline $duration-fast-01 motion(standard, productive); } + &.#{$prefix}--tabs--dismissable .#{$prefix}--tabs__nav-link { + padding-inline-end: $spacing-08; + } + + &.#{$prefix}--tabs--dismissable.#{$prefix}--tabs--contained + .#{$prefix}--tabs__nav-link { + padding-inline-end: calc($spacing-09 - 1px); + } + //----------------------------- // Icon //----------------------------- - &.#{$prefix}--tabs--contained - .#{$prefix}--tabs__nav-item:not(.#{$prefix}--tabs__nav-item--disabled) - .#{$prefix}--tabs__nav-item--icon - .#{$prefix}--tabs__nav-item--close-icon { - &:hover { - background-color: inherit; - } + .#{$prefix}--tabs__nav-item--close { + position: relative; + display: flex; + align-items: center; + inset-inline-start: calc(-#{$spacing-04} - 1px); + margin-inline-start: calc(-#{$spacing-06} + 1px); + } + + &:not(.#{$prefix}--tabs--contained) + .#{$prefix}--tabs__nav-item--close--hidden { + @include visually-hidden; + + position: static; + inline-size: convert.to-rem(3px); + } + + &.#{$prefix}--tabs--contained.#{$prefix}--tabs--full-width + .#{$prefix}--tabs__nav-item--close--hidden { + display: none; + } + + .#{$prefix}--tabs__nav-item--close-icon { + @include button-reset.reset(); + + block-size: convert.to-rem(24px); + inline-size: convert.to-rem(24px); + padding-block: $spacing-02; + padding-inline: $spacing-02; + pointer-events: auto; svg { - padding: $spacing-02; - margin: -$spacing-02; - block-size: 24px; - inline-size: 24px; + block-size: convert.to-rem(16px); + fill: $text-secondary; + inline-size: convert.to-rem(16px); } svg:hover { - background-color: $layer-accent-hover; + fill: $text-primary; + } + + &:hover { + background-color: $layer-hover; + } + + &:focus, + &:active { + @include focus-outline('outline'); } } - &.#{$prefix}--tabs--contained - .#{$prefix}--tabs__nav-item:not( - .#{$prefix}--tabs__nav-item--disabled - ).#{$prefix}--tabs__nav-item--selected - .#{$prefix}--tabs__nav-item--icon + .#{$prefix}--tabs__nav-item:hover + + .#{$prefix}--tabs__nav-item--close .#{$prefix}--tabs__nav-item--close-icon - svg:hover { - background-color: $layer-hover; + svg { + fill: $text-primary; } - .#{$prefix}--tabs__nav-item:not(.#{$prefix}--tabs__nav-item--disabled) - .#{$prefix}--tabs__nav-item--icon - .#{$prefix}--tabs__nav-item--close-icon:hover { - background-color: $background-hover; + .#{$prefix}--tabs__nav-item--close-icon--selected { + svg { + fill: $text-primary; + } + } + + .#{$prefix}--tabs__nav-item:hover + + .#{$prefix}--tabs__nav-item--close + .#{$prefix}--tabs__nav-item--close-icon--disabled, + .#{$prefix}--tabs__nav-item--close-icon--disabled, + .#{$prefix}--tabs__nav-item--close-icon--disabled:hover { + background-color: inherit; + cursor: not-allowed; + + svg { + fill: $tab-text-disabled; + } + + &:focus, + &:active { + @include focus-outline('reset'); + } } .#{$prefix}--tabs__nav-item--icon { display: flex; align-items: center; padding-inline-start: $spacing-03; - - .#{$prefix}--tabs__nav-item--close-icon { - padding: $spacing-02; - margin: -$spacing-02; - line-height: 0; - pointer-events: auto; - } } .#{$prefix}--tabs__nav-item--icon-left {