Fix activateOnFocus

mui · Nov 19, 2024 · 7fada3f · 7fada3f
1 parent 9de095c
commit 7fada3f
Show file tree

Hide file tree

Showing 10 changed files with 108 additions and 267 deletions.
diff --git a/packages/mui-base/src/Tabs/Root/TabsRoot.test.tsx b/packages/mui-base/src/Tabs/Root/TabsRoot.test.tsx
@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { expect } from 'chai';
 import { spy } from 'sinon';
-import { act, describeSkipIf, fireEvent, screen } from '@mui/internal-test-utils';
+import { act, describeSkipIf, flushMicrotasks, fireEvent, screen } from '@mui/internal-test-utils';
 import { Tabs } from '@base_ui/react/Tabs';
 import { createRenderer, describeConformance } from '#test-utils';
 
@@ -214,6 +214,7 @@ describe('<Tabs.Root />', () => {
       });
 
       fireEvent.keyDown(firstTab, { key: 'ArrowRight' });
+      await flushMicrotasks();
 
       expect(handleChange.callCount).to.equal(1);
       expect(handleChange.firstCall.args[0]).to.equal(1);
@@ -301,6 +302,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(firstTab, { key: previousItemKey });
+              await flushMicrotasks();
 
               expect(lastTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(0);
@@ -332,6 +334,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(secondTab, { key: previousItemKey });
+              await flushMicrotasks();
 
               expect(firstTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(0);
@@ -365,6 +368,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(firstTab, { key: previousItemKey });
+              await flushMicrotasks();
 
               expect(lastTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(1);
@@ -397,6 +401,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(secondTab, { key: previousItemKey });
+              await flushMicrotasks();
 
               expect(firstTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(1);
@@ -428,6 +433,7 @@ describe('<Tabs.Root />', () => {
             });
 
             fireEvent.keyDown(lastTab, { key: previousItemKey });
+            await flushMicrotasks();
 
             expect(firstTab).toHaveFocus();
             expect(handleKeyDown.callCount).to.equal(1);
@@ -461,6 +467,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(lastTab, { key: nextItemKey });
+              await flushMicrotasks();
 
               expect(firstTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(0);
@@ -492,6 +499,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(secondTab, { key: nextItemKey });
+              await flushMicrotasks();
 
               expect(lastTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(0);
@@ -525,6 +533,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(lastTab, { key: nextItemKey });
+              await flushMicrotasks();
 
               expect(firstTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(1);
@@ -557,6 +566,7 @@ describe('<Tabs.Root />', () => {
               });
 
               fireEvent.keyDown(secondTab, { key: nextItemKey });
+              await flushMicrotasks();
 
               expect(lastTab).toHaveFocus();
               expect(handleChange.callCount).to.equal(1);
@@ -588,6 +598,7 @@ describe('<Tabs.Root />', () => {
             });
 
             fireEvent.keyDown(firstTab, { key: nextItemKey });
+            await flushMicrotasks();
 
             expect(lastTab).toHaveFocus();
             expect(handleKeyDown.callCount).to.equal(1);

diff --git a/packages/mui-base/src/Tabs/Tab/Tab.test.tsx b/packages/mui-base/src/Tabs/Tab/Tab.test.tsx
@@ -2,16 +2,24 @@ import * as React from 'react';
 import { Tabs } from '@base_ui/react/Tabs';
 import { createRenderer, describeConformance } from '#test-utils';
 import { NOOP } from '../../utils/noop';
+import { CompositeRootContext } from '../../Composite/Root/CompositeRootContext';
 import { TabsRootContext } from '../Root/TabsRootContext';
 import { TabsListContext } from '../TabsList/TabsListContext';
 
 describe('<Tabs.Tab />', () => {
   const { render } = createRenderer();
 
+  const testCompositeContext = {
+    activeIndex: 0,
+    onActiveIndexChange: NOOP,
+  };
+
   const testTabsListContext: TabsListContext = {
     activateOnFocus: true,
     getTabElement: () => null,
+    highlightedTabIndex: 0,
     onTabActivation: NOOP,
+    setHighlightedTabIndex: NOOP,
     tabsListRef: {
       current: null,
     },
@@ -32,7 +40,11 @@ describe('<Tabs.Tab />', () => {
     render: (node) => {
       return render(
         <TabsRootContext.Provider value={testTabsContext}>
-          <TabsListContext.Provider value={testTabsListContext}>{node}</TabsListContext.Provider>
+          <TabsListContext.Provider value={testTabsListContext}>
+            <CompositeRootContext.Provider value={testCompositeContext}>
+              {node}
+            </CompositeRootContext.Provider>
+          </TabsListContext.Provider>
         </TabsRootContext.Provider>,
       );
     },

diff --git a/packages/mui-base/src/Tabs/Tab/Tab.tsx b/packages/mui-base/src/Tabs/Tab/Tab.tsx
@@ -3,7 +3,6 @@ import * as React from 'react';
 import { useTab } from './useTab';
 import { useComponentRenderer } from '../../utils/useComponentRenderer';
 import type { BaseUIComponentProps } from '../../utils/types';
-import { useCompositeRootContext } from '../../Composite/Root/CompositeRootContext';
 import type { TabsOrientation } from '../Root/TabsRoot';
 import { useTabsRootContext } from '../Root/TabsRootContext';
 import { useTabsListContext } from '../TabsList/TabsListContext';
@@ -25,28 +24,28 @@ const Tab = React.forwardRef(function Tab(
   const { className, disabled = false, render, value: valueProp, id: idProp, ...other } = props;
 
   const {
-    value: selectedValue,
+    value: selectedTabValue,
     getTabPanelIdByTabValueOrIndex,
     orientation,
   } = useTabsRootContext();
 
-  const { activateOnFocus, onTabActivation } = useTabsListContext();
-
-  // this is the context of TabsList
-  const { activeIndex } = useCompositeRootContext();
-  // console.log('activeIndex', activeIndex);
+  const { activateOnFocus, highlightedTabIndex, onTabActivation, setHighlightedTabIndex } =
+    useTabsListContext();
 
   const { getRootProps, index, selected } = useTab({
+    activateOnFocus,
     disabled,
     getTabPanelIdByTabValueOrIndex,
+    highlightedTabIndex,
     id: idProp,
-    isSelected: valueProp === selectedValue,
     onTabActivation,
     rootRef: forwardedRef,
+    setHighlightedTabIndex,
+    selectedTabValue,
     value: valueProp,
   });
 
-  const highlighted = index > -1 && index === activeIndex;
+  const highlighted = index > -1 && index === highlightedTabIndex;
 
   const ownerState: Tab.OwnerState = React.useMemo(
     () => ({

diff --git a/packages/mui-base/src/Tabs/Tab/useTab.test.tsx b/packages/mui-base/src/Tabs/Tab/useTab.test.tsx
diff --git a/packages/mui-base/src/Tabs/Tab/useTab.ts b/packages/mui-base/src/Tabs/Tab/useTab.ts
@@ -1,12 +1,14 @@
 'use client';
 import * as React from 'react';
 import { mergeReactProps } from '../../utils/mergeReactProps';
+import { useEnhancedEffect } from '../../utils/useEnhancedEffect';
 import { useForkRef } from '../../utils/useForkRef';
 import { useId } from '../../utils/useId';
 import { useButton } from '../../useButton';
 import { useCompositeItem } from '../../Composite/Item/useCompositeItem';
 import type { TabsRootContext } from '../Root/TabsRootContext';
 import type { useTabsList } from '../TabsList/useTabsList';
+import type { TabsList } from '../TabsList/TabsList';
 
 export interface TabMetadata {
   disabled: boolean;
@@ -15,12 +17,15 @@ export interface TabMetadata {
 
 function useTab(parameters: useTab.Parameters): useTab.ReturnValue {
   const {
+    activateOnFocus,
     disabled = false,
     getTabPanelIdByTabValueOrIndex,
+    highlightedTabIndex,
     id: idParam,
-    isSelected,
     onTabActivation,
     rootRef: externalRef,
+    selectedTabValue,
+    setHighlightedTabIndex,
     value: valueParam,
   } = parameters;
 
@@ -39,6 +44,25 @@ function useTab(parameters: useTab.Parameters): useTab.ReturnValue {
 
   const tabValue = valueParam ?? index;
 
+  // the `selected` state isn't set on the server (it relies on effects to be calculated),
+  // so we fall back to checking the `value` param with the selectedValue from the TabsContext
+  const selected = React.useMemo(() => {
+    if (valueParam === undefined) {
+      return index < 0 ? false : index === selectedTabValue;
+    }
+
+    return valueParam === selectedTabValue;
+  }, [index, selectedTabValue, valueParam]);
+
+  // ensure the active item in Composite's roving focus group matches the selected Tab
+  // FIXME: something is wrong with this
+  useEnhancedEffect(() => {
+    if (activateOnFocus && selected && index > -1 && highlightedTabIndex !== index) {
+      console.log('useEnhancedEffect update index', index);
+      setHighlightedTabIndex(index);
+    }
+  }, [activateOnFocus, highlightedTabIndex, index, selected, setHighlightedTabIndex]);
+
   const { getButtonProps, buttonRef } = useButton({
     disabled,
     focusableWhenDisabled: true,
@@ -57,27 +81,45 @@ function useTab(parameters: useTab.Parameters): useTab.ReturnValue {
           {
             role: 'tab',
             'aria-controls': tabPanelId,
-            'aria-selected': false,
+            'aria-selected': selected,
             id,
             ref: handleRef,
             onClick(event) {
               onTabActivation(tabValue, event.nativeEvent);
             },
+            onFocus(event) {
+              if (!activateOnFocus) {
+                return;
+              }
+
+              if (selectedTabValue !== tabValue) {
+                onTabActivation(tabValue, event.nativeEvent);
+              }
+            },
           },
           mergeReactProps(getItemProps(), getButtonProps()),
         ),
       );
     },
-    [getButtonProps, getItemProps, handleRef, id, onTabActivation, tabPanelId, tabValue],
+    [
+      activateOnFocus,
+      getButtonProps,
+      getItemProps,
+      handleRef,
+      id,
+      onTabActivation,
+      selected,
+      selectedTabValue,
+      tabPanelId,
+      tabValue,
+    ],
   );
 
   return {
     getRootProps,
     index,
     rootRef: handleRef,
-    // the `selected` state isn't set on the server (it relies on effects to be calculated),
-    // so we fall back to checking the `value` prop with the selectedValue from the TabsContext
-    selected: /* selected || */ isSelected,
+    selected,
     // TODO: recalculate this using Composite stuff, but is it really needed?
     totalTabsCount: -1,
   };
@@ -86,6 +128,7 @@ function useTab(parameters: useTab.Parameters): useTab.ReturnValue {
 namespace useTab {
   export interface Parameters
     extends Pick<TabsRootContext, 'getTabPanelIdByTabValueOrIndex'>,
+      Pick<TabsList.Props, 'activateOnFocus'>,
       Pick<useTabsList.ReturnValue, 'onTabActivation'> {
     /**
      * The value of the tab.
@@ -101,12 +144,14 @@ namespace useTab {
      * If `true`, the tab will be disabled.
      */
     disabled?: boolean;
+    highlightedTabIndex: number;
     /**
      * The id of the tab.
      * If not provided, it will be automatically generated.
      */
     id?: string;
-    isSelected: boolean;
+    selectedTabValue: TabsRootContext['value'];
+    setHighlightedTabIndex: (index: number) => void;
     /**
      * Ref to the root slot's DOM element.
      */