Fix registering ToolbarLink as composite item

mj12albert · Jan 27, 2025 · 2901a4d · 2901a4d
1 parent f79f0e5
commit 2901a4d
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 17 deletions.
diff --git a/packages/react/src/toolbar/button/ToolbarButton.tsx b/packages/react/src/toolbar/button/ToolbarButton.tsx
@@ -5,14 +5,10 @@ import { useComponentRenderer } from '../../utils/useComponentRenderer';
 import { BaseUIComponentProps } from '../../utils/types';
 import { useButton } from '../../use-button';
 import { CompositeItem } from '../../composite/item/CompositeItem';
-import type { ToolbarRoot } from '../root/ToolbarRoot';
+import type { ToolbarRoot, ToolbarItemMetadata } from '../root/ToolbarRoot';
 import { useToolbarRootContext } from '../root/ToolbarRootContext';
 import { useToolbarGroupContext } from '../group/ToolbarGroupContext';
 
-export interface ToolbarButtonMetadata {
-  focusableWhenDisabled: boolean;
-}
-
 /**
  * A button that can be used as-is or as a trigger for other components.
  * Renders a `<button>` element.
@@ -63,7 +59,7 @@ const ToolbarButton = React.forwardRef(function ToolbarButton(
     extraProps: otherProps,
   });
 
-  return <CompositeItem<ToolbarButtonMetadata> metadata={itemMetadata} render={renderElement()} />;
+  return <CompositeItem<ToolbarItemMetadata> metadata={itemMetadata} render={renderElement()} />;
 });
 
 export namespace ToolbarButton {

diff --git a/packages/react/src/toolbar/link/ToolbarLink.test.tsx b/packages/react/src/toolbar/link/ToolbarLink.test.tsx
@@ -40,11 +40,11 @@ describe('<Toolbar.Link />', () => {
     it('renders an anchor', async () => {
       const { getByTestId } = await render(
         <Toolbar.Root>
-          <Toolbar.Link data-testid="link" />
+          <Toolbar.Link data-testid="link" href="https://base-ui.com" />
         </Toolbar.Root>,
       );
 
-      expect(getByTestId('button')).to.equal(screen.getByRole('button'));
+      expect(getByTestId('link')).to.equal(screen.getByRole('link'));
     });
   });
 });
diff --git a/packages/react/src/toolbar/link/ToolbarLink.tsx b/packages/react/src/toolbar/link/ToolbarLink.tsx
@@ -5,9 +5,13 @@ import { useComponentRenderer } from '../../utils/useComponentRenderer';
 import { BaseUIComponentProps } from '../../utils/types';
 import { useButton } from '../../use-button';
 import { CompositeItem } from '../../composite/item/CompositeItem';
-import type { ToolbarOrientation } from '../root/ToolbarRoot';
+import type { ToolbarOrientation, ToolbarItemMetadata } from '../root/ToolbarRoot';
 import { useToolbarRootContext } from '../root/ToolbarRootContext';
 
+const TOOLBAR_LINK_METADATA = {
+  focusableWhenDisabled: true,
+};
+
 const ToolbarLink = React.forwardRef(function ToolbarLink(
   props: ToolbarLink.Props,
   forwardedRef: React.ForwardedRef<HTMLAnchorElement>,
@@ -36,7 +40,9 @@ const ToolbarLink = React.forwardRef(function ToolbarLink(
     extraProps: otherProps,
   });
 
-  return <CompositeItem render={renderElement()} />;
+  return (
+    <CompositeItem<ToolbarItemMetadata> metadata={TOOLBAR_LINK_METADATA} render={renderElement()} />
+  );
 });
 
 export namespace ToolbarLink {

diff --git a/packages/react/src/toolbar/root/ToolbarRoot.tsx b/packages/react/src/toolbar/root/ToolbarRoot.tsx
@@ -67,6 +67,10 @@ const ToolbarRoot = React.forwardRef(function ToolbarRoot(
 
 export type ToolbarOrientation = 'horizontal' | 'vertical';
 
+export interface ToolbarItemMetadata {
+  focusableWhenDisabled: boolean;
+}
+
 namespace ToolbarRoot {
   export type State = {
     disabled: boolean;

diff --git a/packages/react/src/toolbar/root/ToolbarRootContext.tsx b/packages/react/src/toolbar/root/ToolbarRootContext.tsx
@@ -1,14 +1,13 @@
 'use client';
 import * as React from 'react';
 import type { CompositeMetadata } from '../../composite/list/CompositeList';
-import type { ToolbarButtonMetadata } from '../button/ToolbarButton';
-import type { ToolbarOrientation } from './ToolbarRoot';
+import type { ToolbarOrientation, ToolbarItemMetadata } from './ToolbarRoot';
 
 export interface ToolbarRootContext {
   disabled: boolean;
   orientation: ToolbarOrientation;
   setItemMap: React.Dispatch<
-    React.SetStateAction<Map<Node, CompositeMetadata<ToolbarButtonMetadata> | null>>
+    React.SetStateAction<Map<Node, CompositeMetadata<ToolbarItemMetadata> | null>>
   >;
 }
 

diff --git a/packages/react/src/toolbar/root/useToolbarRoot.ts b/packages/react/src/toolbar/root/useToolbarRoot.ts
@@ -2,14 +2,14 @@
 import * as React from 'react';
 import { mergeReactProps } from '../../utils/mergeReactProps';
 import { GenericHTMLProps } from '../../utils/types';
-import type { ToolbarButtonMetadata } from '../button/ToolbarButton';
 import type { CompositeMetadata } from '../../composite/list/CompositeList';
+import type { ToolbarItemMetadata } from './ToolbarRoot';
 
 function useToolbarRoot(parameters: useToolbarRoot.Parameters): useToolbarRoot.ReturnValue {
   const { orientation } = parameters;
 
   const [itemMap, setItemMap] = React.useState(
-    () => new Map<Node, CompositeMetadata<ToolbarButtonMetadata> | null>(),
+    () => new Map<Node, CompositeMetadata<ToolbarItemMetadata> | null>(),
   );
 
   const disabledIndices = React.useMemo(() => {
@@ -60,7 +60,7 @@ namespace useToolbarRoot {
     getRootProps: (externalProps?: GenericHTMLProps) => GenericHTMLProps;
     disabledIndices: number[];
     setItemMap: React.Dispatch<
-      React.SetStateAction<Map<Node, CompositeMetadata<ToolbarButtonMetadata> | null>>
+      React.SetStateAction<Map<Node, CompositeMetadata<ToolbarItemMetadata> | null>>
     >;
   }
 }

diff --git a/packages/react/src/use-button/useButton.ts b/packages/react/src/use-button/useButton.ts
@@ -56,8 +56,10 @@ export function useButton(parameters: useButton.Parameters = {}): useButton.Retu
     } else if (elementName !== '') {
       if (elementName !== 'A') {
         additionalProps.role = 'button';
+        additionalProps.tabIndex = tabIndex ?? 0;
+      } else if (tabIndex) {
+        additionalProps.tabIndex = tabIndex;
       }
-      additionalProps.tabIndex = tabIndex ?? 0;
       if (disabled) {
         additionalProps['aria-disabled'] = disabled as boolean;
         additionalProps.tabIndex = focusableWhenDisabled ? (tabIndex ?? 0) : -1;