Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Toolbar] Add Toolbar components #1349

Merged
merged 95 commits into from
Mar 18, 2025
Merged

Conversation

mj12albert
Copy link
Member

@mj12albert mj12albert commented Jan 21, 2025

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:

  • Dialog/AlertDialog
  • Menu
  • Menu with submenu
  • Popover
  • Select
  • Toggle/ToggleGroup
  • Tooltip
  • NumberField using Toolbar.Input
  • DirectionProvider
  • Input this will be a Toolbar.Input instead
  • Switch

Closes #661

@mj12albert mj12albert added the component: toolbar This is the name of the generic UI component, not the React module! label Jan 21, 2025
Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 7ce819e
🔍 Latest deploy log https://app.netlify.com/sites/base-ui/deploys/67d9335865c5a4000825fa8e
😎 Deploy Preview https://deploy-preview-1349--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

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 mj12albert force-pushed the feat/toolbar branch 2 times, most recently from f2b6d91 to 9c3111f Compare January 27, 2025 08:45
@mj12albert mj12albert changed the title feat/toolbar [Toolbar] Add Toolbar components Jan 28, 2025
@@ -22,6 +23,8 @@ export function useButton(parameters: useButton.Parameters = {}): useButton.Retu
rootElementName: elementNameProp,
});

const isCompositeItem = useCompositeRootContext(true) !== undefined;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@mj12albert mj12albert Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

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

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

@mj12albert

This comment was marked as outdated.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 13, 2025
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 13, 2025
@mj12albert
Copy link
Member Author

image

I've made the demo smaller and removed the icons/input @colmtuite

@colmtuite
Copy link
Contributor

colmtuite commented Mar 17, 2025

@mj12albert

Content

  • Change "Auto" and "Text" toggles to "Align Left" and "Align Right".
  • Remove the "Using with Toggles" example, that's covered in the hero example.
  • Change "Using with popups" to "Using with Menu", and remove the Select example.
  • Add a Select menu into the hero demo. Just 2 options: "Arial" and "Helvetica".

Styling

  • Computed font-size on all text in the hero example should be 14px.
  • Add font-family: inherit to controls/buttons, so it inherits the font stack.
  • Add font-weight: 500 to controls/buttons.
  • Change parent from inline-flex to flex. Add align-items: center to it.
  • Remove align-self: stretch from separators. Give separators height: 16px. Set their margin to 0 .25rem.
  • Remove cursor: pointer from controls/buttons.

@mj12albert
Copy link
Member Author

image

@colmtuite Updated the demo and docs: https://deploy-preview-1349--base-ui.netlify.app/react/components/toolbar

If this is ok I'll add the tailwind version as well

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 17, 2025
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 17, 2025
@mj12albert mj12albert merged commit d419cfc into mui:master Mar 18, 2025
22 checks passed
@mj12albert mj12albert deleted the feat/toolbar branch March 18, 2025 08:56
atomiks added a commit to atomiks/base-ui that referenced this pull request Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: toolbar This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[toolbar] Implement Toolbar
5 participants