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 <[email protected]>
carbon-design-system · Apr 9, 2024 · 0376df7 · 0376df7
1 parent b21f8ea
commit 0376df7
Show file tree

Hide file tree

Showing 3 changed files with 196 additions and 97 deletions.
diff --git 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', () => {
       </Tabs>
     );
     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', () => {
       </Tabs>
     );
     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
@@ -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<HTMLElement, TabProps>(function Tab(
     onTabCloseRequest,
   } = React.useContext(TabsContext);
   const { index, hasSecondaryLabel, contained } = React.useContext(TabContext);
-  const dismissIconRef = useRef<HTMLDivElement>(null);
+  const dismissIconRef = useRef<HTMLButtonElement>(null);
   const tabRef = useRef<HTMLElement>(null);
   const ref = useMergedRefs([forwardRef, tabRef]);
   const [ignoreHover, setIgnoreHover] = useState(false);
@@ -849,6 +850,32 @@ const Tab = forwardRef<HTMLElement, TabProps>(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<HTMLElement, TabProps>(function Tab(
 
   const DismissIcon = (
     <div
-      tabIndex={-1}
-      aria-hidden={true}
-      className={cx(`${prefix}--tabs__nav-item--close-icon`, {
-        [`${prefix}--visually-hidden`]: !dismissable,
-      })}
-      onClick={handleClose}
-      title="Close tab"
-      ref={dismissIconRef}>
-      <Close
-        aria-hidden={dismissable ? 'false' : 'true'}
-        aria-label="Press delete to close tab"
-      />
+      className={cx({
+        [`${prefix}--tabs__nav-item--close`]: dismissable,
+        [`${prefix}--tabs__nav-item--close--hidden`]: !dismissable,
+      })}>
+      <button
+        type="button"
+        tabIndex={selectedIndex === index && dismissable ? 0 : -1}
+        aria-disabled={disabled}
+        aria-hidden={selectedIndex === index && dismissable ? 'false' : 'true'}
+        disabled={disabled}
+        className={cx({
+          [`${prefix}--tabs__nav-item--close-icon`]: dismissable,
+          [`${prefix}--visually-hidden`]: !dismissable,
+          [`${prefix}--tabs__nav-item--close-icon--selected`]:
+            selectedIndex === index,
+          [`${prefix}--tabs__nav-item--close-icon--disabled`]: disabled,
+        })}
+        onClick={handleClose}
+        title={`Remove ${typeof children === 'string' ? children : ''} tab`}
+        ref={dismissIconRef}>
+        <Close
+          aria-hidden={
+            selectedIndex === index && dismissable ? 'false' : 'true'
+          }
+          aria-label={`Press delete to remove ${
+            typeof children === 'string' ? children : ''
+          } tab`}
+        />
+      </button>
     </div>
   );
 
   const hasIcon = Icon ?? dismissable;
 
   return (
-    <BaseComponent
-      {...rest}
-      aria-controls={panelId}
-      aria-disabled={disabled}
-      aria-selected={selectedIndex === index}
-      ref={ref}
-      id={id}
-      role="tab"
-      className={className}
-      disabled={disabled}
-      onClick={(evt) => {
-        if (disabled) {
-          return;
-        }
-        setSelectedIndex(index);
-        onClick?.(evt);
-      }}
-      onKeyDown={handleKeyDown}
-      tabIndex={selectedIndex === index ? '0' : '-1'}
-      type="button">
-      <div className={`${prefix}--tabs__nav-item-label-wrapper`}>
-        {dismissable && Icon && (
-          <div className={`${prefix}--tabs__nav-item--icon-left`}>
-            {<Icon size={16} />}
-          </div>
-        )}
-        <Text className={`${prefix}--tabs__nav-item-label`}>{children}</Text>
-        {/* always rendering dismissIcon so we don't lose reference to it, otherwise events do not work when switching from/to dismissable state */}
-        <div
-          className={cx(`${prefix}--tabs__nav-item--icon`, {
-            [`${prefix}--visually-hidden`]: !hasIcon,
-          })}>
-          {DismissIcon}
-          {!dismissable && Icon && <Icon size={16} />}
+    <>
+      <BaseComponent
+        {...rest}
+        aria-controls={panelId}
+        aria-disabled={disabled}
+        aria-selected={selectedIndex === index}
+        ref={ref}
+        id={id}
+        role="tab"
+        className={className}
+        disabled={disabled}
+        onClick={(evt) => {
+          if (disabled) {
+            return;
+          }
+          setSelectedIndex(index);
+          onClick?.(evt);
+        }}
+        onKeyDown={handleKeyDown}
+        tabIndex={selectedIndex === index ? '0' : '-1'}
+        type="button">
+        <div className={`${prefix}--tabs__nav-item-label-wrapper`}>
+          {dismissable && Icon && (
+            <div className={`${prefix}--tabs__nav-item--icon-left`}>
+              {<Icon size={16} />}
+            </div>
+          )}
+          <Text className={`${prefix}--tabs__nav-item-label`}>{children}</Text>
+          {!dismissable && Icon && (
+            <div
+              className={cx(`${prefix}--tabs__nav-item--icon`, {
+                [`${prefix}--visually-hidden`]: !hasIcon,
+              })}>
+              {!dismissable && Icon && <Icon size={16} />}
+            </div>
+          )}
         </div>
-      </div>
-      {hasSecondaryLabel && secondaryLabel && (
-        <Text
-          as="div"
-          className={`${prefix}--tabs__nav-item-secondary-label`}
-          title={secondaryLabel}>
-          {secondaryLabel}
-        </Text>
-      )}
-    </BaseComponent>
+        {hasSecondaryLabel && secondaryLabel && (
+          <Text
+            as="div"
+            className={`${prefix}--tabs__nav-item-secondary-label`}
+            title={secondaryLabel}>
+            {secondaryLabel}
+          </Text>
+        )}
+      </BaseComponent>
+      {/* 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 = {