[Toolbar] Add Toolbar components #1349

mj12albert · 2025-01-21T13:23:41Z

Docs:

https://deploy-preview-1349--base-ui.netlify.app/react/components/toolbar

Demos:

Summary

parts: Root, Button, Input, Link, Group, Separator
focusableWhenDisabled prop on Button & Input
orientation prop
roving focus, loop prop
~~hotkey prop on Root~~ [Toolbar] Support an optional hotkey #1454
grid layout with cols prop

Component integrations

The three demos linked above show all of these:

Closes #661

I have followed (at least) the PR section of the contributing guide.

netlify · 2025-01-21T13:25:46Z

✅ Deploy Preview for base-ui ready!

Name	Link
🔨 Latest commit	`0f358c8`
🔍 Latest deploy log	https://app.netlify.com/sites/base-ui/deploys/67bc580539f0ec0009af31c5
😎 Deploy Preview	https://deploy-preview-1349--base-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mj12albert · 2025-02-03T10:58:48Z

packages/react/src/use-button/useButton.ts

@@ -22,6 +23,8 @@ export function useButton(parameters: useButton.Parameters = {}): useButton.Retu
    rootElementName: elementNameProp,
  });

+  const isCompositeItem = useCompositeRootContext(true) !== undefined;


This is to work around using Select.Trigger as a composite item by letting CompositeItem's tabIndex override the internal one in useButton

Like Menu.Trigger, select trigger manually sets tabIndex to keep a consistent "trigger remains focused after a select/menu item is select" behavior across browsers (Safari on iPadOS and iOS): https://github.com/mui/base-ui/blob/master/packages/react/src/select/trigger/useSelectTrigger.ts#L80

I wonder if this could cause problems with buttons nested inside other components within a toolbar (for example, a button inside a dialog opened from a toolbar would have this set to true, even though it's not a composite item).

for example, a button inside a dialog opened from a toolbar

Only if that button happened to use useButton as well, so in this case, a disabled Dialog.Close isn't affected, but a MenuTrigger in that dialog would be

What this affects is that when disabled, the HTML disabled attribute is never applied. Generally I think our components that use useButton already don't rely on the disabled attr to disable functionality anyway in order to support component/element replacement so it should be OK in most cases

@michaldudak I also tried making it a isCompositeItem param instead of a check but it wasn't enough to handle this more common case: 575dc96

docs/src/app/(private)/experiments/toolbar/text-editor.tsx

aarongarciah · 2025-02-07T06:56:00Z

I found a strange behavior: the menu won't close when a submenu is open and you move the roving focus to the next toolbar element by pressing ➡️. Only the submenu is closed. See attached video.

Kapture.2025-02-07.at.07.45.58.mp4

packages/react/src/toggle-group/ToggleGroup.tsx

mj12albert · 2025-02-20T16:47:16Z

In the Triggers experiment, it seems like disabling controls doesn't work properly? When I disable the switch individually, that works. But when I disable the Dialog individually, that doesn't work. I saw this was called out before, at least in some capacity

OK it turns out various triggers have issues with the disabled state when they are not native button elements disabled with the disabled attribute (i.e. a custom element disabled using aria-disabled/data-disabled)

The fix depends on #1445

Before some of these experiments seemed to work occasionally because I was passing the disabled state 2-3 times, to the trigger, the root, and the toolbar button at the same time, but it should only be passed once to the toolbar button

michaldudak · 2025-02-21T08:16:45Z

I find restoring focus to the previously active item quite weird. I know ARIA guidelines suggest this ("If the toolbar has previously contained focus, focus is optionally set on the control that last had focus. Otherwise, it is set on the first control that is not disabled."), but the applications I checked don't work like that. cc @colmtuite

packages/react/src/toolbar/link/ToolbarLink.tsx

packages/react/src/toggle/useToggle.ts

packages/react/src/toolbar/button/ToolbarButton.tsx

michaldudak · 2025-02-21T08:40:12Z

packages/react/src/use-button/useButton.ts

@@ -22,6 +23,8 @@ export function useButton(parameters: useButton.Parameters = {}): useButton.Retu
    rootElementName: elementNameProp,
  });

+  const isCompositeItem = useCompositeRootContext(true) !== undefined;


I wonder if this could cause problems with buttons nested inside other components within a toolbar (for example, a button inside a dialog opened from a toolbar would have this set to true, even though it's not a composite item).

mj12albert · 2025-02-24T10:53:09Z

packages/react/src/use-button/useButton.ts

+      if (elementName !== 'A') {
+        additionalProps.role = 'button';


This overwrites Select.Trigger's built-in role 😩

mj12albert added the component: toolbar The React component. label Jan 21, 2025

mj12albert force-pushed the feat/toolbar branch 2 times, most recently from f2b6d91 to 9c3111f Compare January 27, 2025 08:45

mj12albert changed the title ~~feat/toolbar~~ [Toolbar] Add Toolbar components Jan 28, 2025

mj12albert mentioned this pull request Jan 28, 2025

[DataGrid] New Toolbar component mui/mui-x#14611

Open

4 tasks

mj12albert force-pushed the feat/toolbar branch from 2901a4d to 61dc192 Compare February 3, 2025 10:30

mj12albert commented Feb 3, 2025

View reviewed changes

mj12albert force-pushed the feat/toolbar branch 7 times, most recently from 5dcde0c to 5fe85d2 Compare February 6, 2025 15:47

mj12albert commented Feb 7, 2025

View reviewed changes

docs/src/app/(private)/experiments/toolbar/text-editor.tsx Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

mj12albert commented Feb 7, 2025

View reviewed changes

packages/react/src/toggle-group/ToggleGroup.tsx Show resolved Hide resolved

mj12albert mentioned this pull request Feb 8, 2025

[popups] actionsRef prop #1236

Merged

mj12albert force-pushed the feat/toolbar branch from 090aee3 to b3d1c0c Compare February 10, 2025 15:07

mj12albert mentioned this pull request Feb 11, 2025

[Toolbar] POC with context checking API #1437

Closed

1 task

mj12albert force-pushed the feat/toolbar branch 3 times, most recently from c865bad to 626f251 Compare February 13, 2025 10:02

mj12albert mentioned this pull request Feb 13, 2025

[Tabs] Arrow keys cannot navigate to disabled Tabs #1442

Closed

mj12albert force-pushed the feat/toolbar branch 4 times, most recently from 736a277 to 7b40b0c Compare February 16, 2025 06:40

mj12albert added 6 commits February 20, 2025 13:34

Update api docs

e67b81d

Add disabledIndices test for CompositeRoot

8a15765

Update docs

154aeb1

Add ToolbarInput text navigation test

65f9e9d

Fix conformance test warnings

28d6df4

Update demos

3288348

mj12albert force-pushed the feat/toolbar branch 2 times, most recently from 52068e8 to 557bbd4 Compare February 20, 2025 07:48

Fix disabled state

c7b0c97

mj12albert force-pushed the feat/toolbar branch from 557bbd4 to c7b0c97 Compare February 20, 2025 08:36

mj12albert added 3 commits February 20, 2025 17:36

Fix useButton disabled attr

3b75a0b

Update demo

099e076

Add Menu test

820b590

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 21, 2025

michaldudak reviewed Feb 21, 2025

View reviewed changes

mj12albert added 6 commits February 24, 2025 14:01

Merge remote-tracking branch 'upstream/master' into feat/toolbar

b04f8c3

Flatten mergeReactProps

ab1d0e8

Fix ToolbarButton disabled state

95e617d

Use an effect to handle composite button disabled attr

575dc96

Flatten useToggle mergeReactProps

dd5ea35

Fix typo

ec1bf4b

github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Feb 24, 2025

Update tests

4e6b8fd

mj12albert commented Feb 24, 2025

View reviewed changes

mj12albert added 2 commits February 24, 2025 19:27

Add Select test

f3de7f1

Merge remote-tracking branch 'upstream/master' into feat/toolbar

0f358c8

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Toolbar] Add Toolbar components #1349

[Toolbar] Add Toolbar components #1349

mj12albert commented Jan 21, 2025 •

edited

Loading

netlify bot commented Jan 21, 2025 •

edited

Loading

mj12albert Feb 3, 2025

michaldudak Feb 21, 2025

mj12albert Feb 24, 2025

mj12albert Feb 24, 2025 •

edited

Loading

aarongarciah commented Feb 7, 2025

This comment was marked as outdated.

mj12albert commented Feb 20, 2025 •

edited

Loading

michaldudak commented Feb 21, 2025

michaldudak Feb 21, 2025

mj12albert Feb 24, 2025

[Toolbar] Add Toolbar components #1349

Are you sure you want to change the base?

[Toolbar] Add Toolbar components #1349

Conversation

mj12albert commented Jan 21, 2025 • edited Loading

Docs:

Demos:

Summary

Component integrations

netlify bot commented Jan 21, 2025 • edited Loading

✅ Deploy Preview for base-ui ready!

mj12albert Feb 3, 2025

Choose a reason for hiding this comment

michaldudak Feb 21, 2025

Choose a reason for hiding this comment

mj12albert Feb 24, 2025

Choose a reason for hiding this comment

mj12albert Feb 24, 2025 • edited Loading

Choose a reason for hiding this comment

aarongarciah commented Feb 7, 2025

This comment was marked as outdated.

mj12albert commented Feb 20, 2025 • edited Loading

michaldudak commented Feb 21, 2025

michaldudak Feb 21, 2025

Choose a reason for hiding this comment

mj12albert Feb 24, 2025

Choose a reason for hiding this comment

mj12albert commented Jan 21, 2025 •

edited

Loading

netlify bot commented Jan 21, 2025 •

edited

Loading

mj12albert Feb 24, 2025 •

edited

Loading

mj12albert commented Feb 20, 2025 •

edited

Loading