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

[Feature Request]: v12 overflowMenu support for renderIcon prop on MenuItem. and checkbox #18148

Open
1 task done
devadula-nandan opened this issue Nov 25, 2024 · 4 comments · May be fixed by #18153
Open
1 task done

Comments

@devadula-nandan
Copy link

devadula-nandan commented Nov 25, 2024

The problem

Migration from v11 to v12 OverflowMenu (experimental). doesn't provide an option to render icons in MenuItem AFAIK.

We have an issue in C4IP where

In v11, we utilized the OverflowMenu component with OverflowMenuItem as its child, which supported an itemText prop that accepted a React node. This allowed us to pass both an icon and text together.

To render the checkbox, we implemented a workaround, though it was not ideal, as shown below. This approach needs to be updated to align with Carbon's implementation. However, I could not find an option to achieve the same result with the MenuItem component. Do you have any recommendations on how to approach this?

Image

In v12 of OverflowMenu, the MenuItem component is used as a child element with properties such as label and shortcut, both of which are strings. Additionally, the renderIcon prop does not function as expected on MenuItem unless the parent component is set to mode='basic'. However, this configuration is only applicable to the Menu component and cannot be used with OverflowMenu.

The solution

ability to render checkboxes and icons for v12 OverflowMenu, like we did in v11.

Image 1 Image 2

Examples

v11 implementation (go to folder 1 to see 2 overflow menu use cases in action)
https://ibm-products.carbondesignsystem.com/?path=/story/ibm-products-patterns-add-and-select-multiaddselect--with-hierarchy&globals=viewport:basic

Application/PAL

c4ip

Business priority

Medium Priority = upcoming release but is not pressing

Available extra resources

No response

Code of Conduct

Copy link
Contributor

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

@devadula-nandan devadula-nandan changed the title [Feature Request]: v12 overflowMenu support for renderIcon prop on MenuItem and checkbox [Feature Request]: v12 overflowMenu support for renderIcon prop on MenuItem. and checkbox Nov 25, 2024
@elycheea elycheea moved this to 🕵️‍♀️ Triage in Design System Nov 25, 2024
@aagonzales
Copy link
Member

The static icon part of this issue is resolved by #18153. For the the interactive checkbox designs we actually recommend using a popover where the menu requires non-menu actions like shown in the filter. Here is an example in this data table story of the suggested solution for filter menus.

@aagonzales aagonzales moved this from Triage to Next ➡ in Roadmap Nov 25, 2024
@aagonzales aagonzales moved this from 🕵️‍♀️ Triage to 🪆 Needs Refined in Design System Nov 25, 2024
@thyhmdo
Copy link
Member

thyhmdo commented Nov 25, 2024

Anna's suggestion is great! I just want to add a point of clarification here. Currently, we have an OverflowMenu Render Custom Icon that uses the Filter icon (or any other icon). I think this creates some confusion.

@janhassel, we should aim for consistency in where specific Overflow Menus will live and consider providing more flexibility—for example, using a popover for the Overflow Menu button within the data table.

@janhassel
Copy link
Member

@thyhmdo Wouldn't an OverflowMenu with three MenuItemSelectable work in the shared use case instead of checkboxes inside of a popover?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🪆 Needs Refined
Status: Next ➡
Development

Successfully merging a pull request may close this issue.

4 participants