feat: add experimental combo-button and menu-button components

[janhassel]

Closes #3371

After working on #13146 I couldn't help myself but wondering how much work it would actually be to utilize the Menu component for the long-standing and accepted proposal of a ComboButton as the Menu should provide all the flexibility to leverage it in many different contexts (related: #1903).

Opening as draft as I have some questions regarding the button kinds that should be supported:

• The mockup only shows primary. I included all kinds that the IconButton supports for this proof of concept and think the secondary works well. Both ghost and tertiary though don't work all that good from a visual perspective:
  • Ghost icon buttons don't share a ghost text button's color and there is no visually detachment between the icon and the button which is bad for discoverability in this case
  • Tertiary buttons have an outline and therefore create a double outline where the gap is. A workaround could be to have a gap of -1px to overlay the right outline of the primary action and the left outline of the trigger. Not sure if it's intended from design though.

Changelog

New

• Added new experimental component ComboButton based on Menu

Testing / Reviewing

Storybook for now mainly. If the core Carbon team is happy with this approach, I will add the remaining topics:

• Unit testing
• VRT and AVT
• Prop-controllable trigger tooltip alignment
• i18n support "Additional actions" label
• Updating supported button kinds based on answer to my question above
• support for props.className
• (whatever else I'm forgetting right now)

@laurenmrice there are some slight visual differences between your last shared design spec and the implementation as it uses the design spec of the Menu component which, for example, doesn't have the indented dividing lines in between each option. I think it's best to keep these styles consistent so that a MenuItem looks and feels the same everywhere it is used, but do let me know if you disagree!

[netlify]

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	d0df7d3
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-components-react/deploys/6410bf202fe6e40008c9800a
😎 Deploy Preview	https://deploy-preview-13224--carbon-components-react.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 settings.

[tay1orjones]

Very cool, this looks great to me! Keyboard navigation works as expected. I really like how it uses the existing MenuItem components - another great example of how composable those components are. For some reason I assumed this would need to be implemented using Popover, but using Menu makes sense.

I'll defer to @laurenmrice on the decision of the secondary/tertiary/ghost variants, but to me the ghost one in particular seems problematic. It's visually very similar to the Inline Dropdown.

Since this is using the MenuItem components, I assume the items inherently support being individually disabled?

[janhassel]

@tay1orjones Yes, passing disabled to a MenuItem will work as expected. Same goes for kind="danger" for example.

[janhassel]

Just pushed an update adding a disabled MenuItem and a danger MenuItem to the demo as well as a MenuItemDivider.

[janhassel]

@tay1orjones @francinelucca I just pushed another update adding another experimental component MenuButton to accommodate use cases where there is no primary action like outlined in this comment: #3371 (comment)

For now it's two separate components even though the logic is very similar, but I could see this being more flexible with the button kinds it would support, hence the separation.

In this regard I outsourced the main logic to an internal hook useAttachedMenu which would then be used by OverflowMenuV2, ComboButton and MenuButton (and in the future perhaps a refactor of HeaderMenu from the UI Shell?).

Obviously nothing of this is set in stone and I'll keep this PR in draft status until we discussed further. I just wanna provide proof of concepts as it's easier to talk about working code than thoughts. 😉

I'll also connect with @Laura-Marshall as I know she worked quite a bit on buttons recently and wanna make sure she's in the loop.

[francinelucca]

mostly LGTM, just one minor comment about accepting a className

[francinelucca]

Behavior-wise I was kind of expecting the ComboButton/MenuButton's focus to move onto the first MenuItem when opening the menu via keyboard.
This is the OverflowMenu's current behavior but I realize that's currently not porting over to OverflowMenuV2, wondering if that's an oversight 🤔.

Any thoughts here @mbgower ? https://deploy-preview-13224--carbon-components-react.netlify.app/?path=/story/experimental-unstable-combobutton--playground

[mbgower]

Behavior-wise I was kind of expecting the ComboButton/MenuButton's focus to move onto the first MenuItem when opening the menu via keyboard.

I think one of two things needs to happen when the menu is opened: either the focus needs to stay on the open/close button, or it needs to go to the first item. As it is, it goes 'nowhere'.

I agree that putting the focus on the first item when the menu is opened is the best. That is a standard thing to do with a menu. It makes sense from an interaction perspective here; forcing the user to press down after opening just to get focus there seems pointless. (BTW, pressing Enter also puts focus on the first item, I discovered.)

[janhassel]

@mbgower when the menu opens, the focus is currently set on the entire menu widget. Any key event that is not escape will move the focus on the first item with the only exception of Arrow up which will focus the last item instead of the first.

I thought this is how the menu widget was specified at one point but either I'm remembering wrongly or it was updated (maybe you would know?). In any case: that can easily be adjusted. While reading through the current APG design pattern it also mentioned the focus should cycle through (after last item, the first item should be focused again and vice versa). This was also reported a while back in #8076

If you agree I'll update the behaviour of the Menu component to focus the first item when opened and enable cycling.

@francinelucca Thanks for catching that! I've added it to the list of remaining work items in the first comment. Does this mean the general structure and idea is fine with you two (cc @tay1orjones) and I should go ahead an add these items to the PR?

[francinelucca]

Created issue for Menu focus: #13265

[francinelucca]

Almost there! Super minor details

[laurenmrice]

@janhassel This looks great Jan, thanks for working on this. ⭐️ Below are some comments I have and addressing some of your questions:

---

Supporting different button types for combo button

The mockup only shows primary. I included all kinds that the IconButton supports for this proof of concept and think the secondary works well. Both ghost and tertiary though don't work all that good from a visual perspective:

• I only provided specs for the Primary button because my understanding of a combo button is that there is always one primary action (the explicit button on the page), and then inside of the menu is where the additional or secondary actions live. I think the other button types like Tertiary and Ghost would be more appropriate options to have for Menu button situations, which also seems in line with Laura Marshall's guidance and looks like you already have it in code in this PR. It also seems to be potentially confusing to me to use a secondary button in the combo button because technically, it should be the primary action.

Ghost icon buttons don't share a ghost text button's color and there is no visually detachment between the icon and the button which is bad for discoverability in this case

• I agree with this. For discoverability reasons, this would be difficult and confusing for people to navigate. It also would currently look a bit disjointed.

Tertiary buttons have an outline and therefore create a double outline where the gap is. A workaround could be to have a gap of -1px to overlay the right outline of the primary action and the left outline of the trigger. Not sure if it's intended from design though.

• Yes and I think that this would be a better option to support Menu button situations where there would be no gap between the two buttons.

• Additionally, my thoughts for not supporting a Danger button version:

  • We do not support an icon-only danger button.
  • I am hesitant to encourage people to use a combo button for danger-specific situations. I wonder how common of a use case is to have many important danger actions to choose from instead of just one or possibly two.

---

Visual differences

there are some slight visual differences between your last shared design spec and the implementation as it uses the design spec of the Menu component which, for example, doesn't have the indented dividing lines in between each option. I think it's best to keep these styles consistent so that a MenuItem looks and feels the same everywhere it is used, but do let me know if you disagree!

• Combo button: We wanted to keep the indented divider lines in the menu for combo button to keep the styling more in line with what we do for dropdown situations.
  

• Menu button: We wanted to have indented dividers in the menu here as well. But if then providing flush dividers (and removing indented dividers) if people need to section out the list of actions if there are many that could be visually grouped. Menu buttons typically can have more options to choose from than combo buttons which is why we wanted to include this.

---

React preview link for Combo button

• I see a Default and a Playground story in the ComboButton folder. The Default story shows the menu with a danger option, and the Playground story does not. I would rather have both stories be consistent and not show the danger option in the menu, and instead have the danger option as a prop you can turn on/off in the Playground story. Not sure if that is possible or not, but ideally I would want the danger option to be something additional that someone would add to the combo button for a specific use case rather it always being there.
• There is a bug in Firefox when opening the menu of the combo button. When I first open the menu, the menu width is a little wider than the combined button widths. In this example, we should keep the menu the same width as the buttons because that is what we recommend by default. I do know that we also will let people extend the width the menu depending on the use case at hand, but we can provide this in our design guidance. (This does not seem to be happening in Chrome or Safari and looks fine)
• When closing the menu and opening it again in all browsers, the menu width shrinks in size.

• The disabled prop in the dark themes shows a slight border around the primary combo button and primary menu button in all browsers. There should be no border.

[janhassel]

@laurenmrice Thanks for the review!

---

I just pushed an update that should fix the issues you were experiencing in Firefox.

---

The subtle border on disabled buttons seems to be an issue with the button component itself so I'd say this should be a separate issue. You can re-produce this in the storybook: https://react.carbondesignsystem.com/?path=/story/components-button--default&args=disabled:true&globals=theme:g100

---

irt the dividers: personally, I'm not a fan of creating and maintaining two different styles for the same component depending on their context in this way. Zooming out, the Menu component (with it's sub components – MenuItem, ...) could be used anywhere in Carbon where a menu design pattern is applied: context menus, overflow menus, combo buttons, menu buttons (and potentially header menu in the future?). We can leverage the same components and therefore the same styles everywhere to create a true "Menu" feel for Carbon. Also I don't think it's too bad to differentiate it from the dropdown slightly as dropdowns shouldn't be misused as menus.

Maybe as it's not too clear from the storybook demo, the dividers can be freely placed by the consumer, like so:

MenuButton
  MenuItem
  MenuItem
  MenuItemDivider
  MenuItem
  MenuItemDivider
  MenuItem

Keeping the same components and styles will allow us to save resources in both dev and figma maintenance (the children of each of the top-level components would all just be MenuItem and its variants).

Let me know if you disagree!

[laurenmrice]

The subtle border on disabled buttons seems to be an issue with the button component itself

Okay, good to know! We can fix that on our end, and I can open an issue for it. 👍

---

Dividers discussion
Recapping here: We agreed in Slack to remove indented dividers in the combo button menu. We also decided to create separate stories for both combo button and menu button to help separate out the default component versus the additional modifiers like a danger action and rule dividers.

[laurenmrice]

Looks good to me, Jan. Thank you for contributing this! ✨✅

[francinelucca]

@janhassel can we address this one #13224 (comment) 🙏🏻. Should be good to go after

[janhassel]

@francinelucca Oh sorry, I must've missed your response in that thread. Just updated to remove them entirely from the controls table 🙂

[francinelucca]

All props should be visible in the all stories, except in the Playground the uncontrollable props (children, translateWithId, className...) should be hidden. Right now they're hidden on all

[janhassel]

@francinelucca Sorry, I misunderstood what you meant in the first place. Let me know if it's all fixed now!