chore(Tabs): rework component (#2991)

Related to #2944 and #2449 Fixes issues found in #2355
digdir · Jan 14, 2025 · 4276d94 · 4276d94
1 parent a42f6f7
commit 4276d94
Show file tree

Hide file tree

Showing 7 changed files with 136 additions and 79 deletions.
diff --git a/.changeset/red-gorillas-argue.md b/.changeset/red-gorillas-argue.md
@@ -0,0 +1,5 @@
+---
+"@digdir/designsystemet-react": patch
+---
+
+Tabs: Content will get focus when it has no focusable elements
diff --git a/.changeset/unlucky-hairs-sit.md b/.changeset/unlucky-hairs-sit.md
@@ -0,0 +1,5 @@
+---
+"@digdir/designsystemet-css": patch
+---
+
+Tabs: Rework component CSS
diff --git a/packages/css/src/tabs.css b/packages/css/src/tabs.css
@@ -5,80 +5,88 @@
   --dsc-tabs-content-padding: var(--ds-size-5);
   --dsc-tabs-content-color: var(--ds-color-neutral-text-default);
   --dsc-tabs-list-border-color: var(--ds-color-neutral-border-subtle);
-}
 
-.ds-tabs__panel {
-  padding: var(--dsc-tabs-content-padding);
-  color: var(--dsc-tabs-content-color);
-}
+  & > [role='tabpanel'] {
+    padding: var(--dsc-tabs-content-padding);
+    color: var(--dsc-tabs-content-color);
 
-.ds-tabs__tablist {
-  display: flex;
-  flex-direction: row;
-  border-bottom: var(--ds-border-width-default) solid var(--dsc-tabs-list-border-color);
-  position: relative;
-}
+    @composes ds-focus from './base.css';
+  }
 
-.ds-tabs__tab {
-  align-items: center;
-  background: none;
-  border: 0;
-  box-sizing: border-box;
-  color: var(--dsc-tabs-tab-color);
-  cursor: pointer;
-  display: flex;
-  flex-direction: row;
-  font-family: inherit;
-  font-size: inherit;
-  gap: var(--ds-size-1);
-  justify-content: center;
-  line-height: var(--ds-line-height-sm);
-  margin: 0;
-  padding: var(--dsc-tabs-tab-padding);
-  position: relative;
-  text-align: center;
+  & > [role='tablist'] {
+    flex-direction: row;
+    border-bottom: var(--ds-border-width-default) solid var(--dsc-tabs-list-border-color);
+    position: relative;
 
-  &:not([data-size]) {
-    font-size: inherit; /* Ensure inheriting font-size when <button> */
-  }
+    &:not([hidden]) {
+      display: flex;
+    }
 
-  & :where(img, svg) {
-    flex-shrink: 0; /* Never shrink icon */
-    font-size: 1.25em; /* Auto scale icon based on font-size */
-  }
+    & > button {
+      align-items: center;
+      background: none;
+      border: 0;
+      box-sizing: border-box;
+      color: var(--dsc-tabs-tab-color);
+      cursor: pointer;
+      flex-direction: row;
+      font-family: inherit;
+      font-size: inherit;
+      gap: var(--ds-size-1);
+      justify-content: center;
+      line-height: var(--ds-line-height-sm);
+      margin: 0;
+      padding: var(--dsc-tabs-tab-padding);
+      position: relative;
+      text-align: center;
 
-  &[aria-selected='true'] {
-    --dsc-tabs-tab-bottom-border-color: var(--ds-color-base-default);
-    --dsc-tabs-tab-color: var(--ds-color-text-subtle);
+      &:not([hidden]) {
+        display: flex;
+      }
 
-    @media (forced-colors: active) {
-      --dsc-tabs-tab-color: CanvasText;
-      border-bottom: 2px solid CanvasText;
-    }
-  }
+      &:not([data-size]) {
+        font-size: inherit; /* Ensure inheriting font-size when <button> */
+      }
 
-  @composes ds-focus from './base.css';
+      & :where(img, svg) {
+        flex-shrink: 0; /* Never shrink icon */
+        font-size: 1.25em; /* Auto scale icon based on font-size */
+      }
 
-  /* We set z-index to make sure the active line does not bleed over the focus indicator */
-  &:focus-visible {
-    z-index: 2;
-  }
+      &[aria-selected='true'] {
+        --dsc-tabs-tab-bottom-border-color: var(--ds-color-base-default);
+        --dsc-tabs-tab-color: var(--ds-color-text-subtle);
 
-  &::after {
-    content: '';
-    display: block;
-    height: .15em; /* Scale with font */
-    width: 100%;
-    background-color: var(--dsc-tabs-tab-bottom-border-color);
-    position: absolute;
-    bottom: 0;
-    left: 0;
-  }
+        @media (forced-colors: active) {
+          --dsc-tabs-tab-color: CanvasText;
+          border-bottom: 2px solid CanvasText;
+        }
+      }
+
+      @composes ds-focus from './base.css';
+
+      /* We set z-index to make sure the active line does not bleed over the focus indicator */
+      &:focus-visible {
+        z-index: 2;
+      }
+
+      &::after {
+        content: '';
+        display: block;
+        height: .15em; /* Scale with font */
+        width: 100%;
+        background-color: var(--dsc-tabs-tab-bottom-border-color);
+        position: absolute;
+        bottom: 0;
+        left: 0;
+      }
 
-  @media (hover: hover) and (pointer: fine) {
-    &:hover:not([aria-selected='true']) {
-      --dsc-tabs-tab-bottom-border-color: var(--ds-color-neutral-border-subtle);
-      --dsc-tabs-tab-color: var(--ds-color-neutral-text-default);
+      @media (hover: hover) and (pointer: fine) {
+        &:hover:not([aria-selected='true']) {
+          --dsc-tabs-tab-bottom-border-color: var(--ds-color-neutral-border-subtle);
+          --dsc-tabs-tab-color: var(--ds-color-neutral-text-default);
+        }
+      }
     }
   }
 }
diff --git a/packages/react/src/components/Tabs/Tabs.test.tsx b/packages/react/src/components/Tabs/Tabs.test.tsx
@@ -6,7 +6,7 @@ import { Tabs } from '.';
 const user = userEvent.setup();
 
 describe('Tabs', () => {
-  test('can navigate tabs with keyboard', async () => {
+  it('can navigate tabs with keyboard', async () => {
     render(
       <Tabs>
         <Tabs.List>
@@ -28,7 +28,7 @@ describe('Tabs', () => {
     expect(tab1).toHaveFocus();
   });
 
-  test('renders content based on value', async () => {
+  it('renders content based on value', async () => {
     render(
       <Tabs defaultValue='value1'>
         <Tabs.List>
@@ -47,7 +47,7 @@ describe('Tabs', () => {
     expect(screen.queryByText('content 1')).not.toBeInTheDocument();
   });
 
-  test('item renders with correct aria attributes', async () => {
+  it('item renders with correct aria attributes', async () => {
     render(
       <Tabs defaultValue='value1'>
         <Tabs.List>
@@ -63,7 +63,7 @@ describe('Tabs', () => {
     expect(tab).toHaveAttribute('aria-selected', 'true');
   });
 
-  test('renders ReactNodes as children when TabsPanels value is selected', () => {
+  it('renders ReactNodes as children when TabsPanels value is selected', () => {
     render(
       <Tabs defaultValue='value1'>
         <Tabs.Panel value='value1'>
@@ -76,7 +76,7 @@ describe('Tabs', () => {
     expect(content).toBeInTheDocument();
   });
 
-  test('can navigate tabs with keyboard', async () => {
+  it('can navigate tabs with keyboard', async () => {
     render(
       <Tabs.List>
         <Tabs.Tab value='value1'>Tab 1</Tabs.Tab>
@@ -93,4 +93,28 @@ describe('Tabs', () => {
     await user.type(tab2, '{arrowleft}');
     expect(tab1).toHaveFocus();
   });
+
+  it('has tabindex 0 on tabpanel', () => {
+    render(
+      <Tabs defaultValue='value1'>
+        <Tabs.Panel value='value1'>content 1</Tabs.Panel>
+      </Tabs>,
+    );
+
+    const panel = screen.getByRole('tabpanel');
+    expect(panel).toHaveAttribute('tabindex', '0');
+  });
+
+  it('has no tabindex when tabpanel has focusable element', () => {
+    render(
+      <Tabs defaultValue='value1'>
+        <Tabs.Panel value='value1'>
+          <input type='text' />
+        </Tabs.Panel>
+      </Tabs>,
+    );
+
+    const panel = screen.getByRole('tabpanel');
+    expect(panel).not.toHaveAttribute('tabindex', '0');
+  });
 });
diff --git a/packages/react/src/components/Tabs/TabsList.tsx b/packages/react/src/components/Tabs/TabsList.tsx
@@ -1,4 +1,3 @@
-import cl from 'clsx/lite';
 import type { HTMLAttributes } from 'react';
 import { forwardRef, useContext } from 'react';
 
@@ -19,14 +18,13 @@ export type TabsListProps = HTMLAttributes<HTMLDivElement>;
  * ```
  */
 export const TabsList = forwardRef<HTMLDivElement, TabsListProps>(
-  function TabsList({ children, className, ...rest }, ref) {
+  function TabsList({ children, ...rest }, ref) {
     const { value } = useContext(Context);
 
     return (
       <RovingFocusRoot
         role='tablist'
         activeValue={value}
-        className={cl('ds-tabs__tablist', className)}
         orientation='ambiguous'
         ref={ref}
         {...rest}

diff --git a/packages/react/src/components/Tabs/TabsPanel.tsx b/packages/react/src/components/Tabs/TabsPanel.tsx
@@ -1,7 +1,7 @@
-import cl from 'clsx/lite';
 import type { HTMLAttributes } from 'react';
-import { forwardRef, useContext } from 'react';
+import { forwardRef, useContext, useEffect, useRef, useState } from 'react';
 
+import { useMergeRefs } from '@floating-ui/react';
 import { Context } from './Tabs';
 
 export type TabsPanelProps = {
@@ -17,14 +17,33 @@ export type TabsPanelProps = {
  * ```
  */
 export const TabsPanel = forwardRef<HTMLDivElement, TabsPanelProps>(
-  function TabsPanel({ children, value, className, ...rest }, ref) {
+  function TabsPanel({ children, value, ...rest }, ref) {
     const { value: tabsValue } = useContext(Context);
     const active = value === tabsValue;
 
+    const [hasTabbableElement, setHasTabbableElement] = useState(false);
+
+    const internalRef = useRef<HTMLDivElement>(null);
+    const mergedRef = useMergeRefs([ref, internalRef]);
+
+    /* Check if the panel has any tabbable elements */
+    useEffect(() => {
+      if (!internalRef.current) return;
+      const tabbableElements = internalRef.current.querySelectorAll(
+        'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])',
+      );
+      setHasTabbableElement(tabbableElements.length > 0);
+    }, [children]);
+
     return (
       <>
         {active && (
-          <div className={cl('ds-tabs__panel', className)} ref={ref} {...rest}>
+          <div
+            ref={mergedRef}
+            role='tabpanel'
+            tabIndex={hasTabbableElement ? undefined : 0}
+            {...rest}
+          >
             {children}
           </div>
         )}

diff --git a/packages/react/src/components/Tabs/TabsTab.tsx b/packages/react/src/components/Tabs/TabsTab.tsx
@@ -1,4 +1,3 @@
-import cl from 'clsx/lite';
 import type { HTMLAttributes } from 'react';
 import { forwardRef, useContext, useId } from 'react';
 
@@ -16,16 +15,15 @@ export type TabsTabProps = {
  * <Tabs.Tab value='1'>Tab 1</Tabs.Tab>
  */
 export const TabsTab = forwardRef<HTMLButtonElement, TabsTabProps>(
-  function TabsTab({ className, value, ...rest }, ref) {
+  function TabsTab({ value, id, ...rest }, ref) {
     const tabs = useContext(Context);
-    const buttonId = `tab-${useId()}`;
+    const buttonId = id ?? `tab-${useId()}`;
 
     return (
       <RovingFocusItem value={value} {...rest} asChild>
         <button
           {...rest}
           aria-selected={tabs.value === value}
-          className={cl('ds-tabs__tab', className)}
           id={buttonId}
           onClick={() => tabs.onChange?.(value)}
           ref={ref}