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

[RFC][material-ui][Alert] Revise action, icon, and iconMappings props #40680

Open
DiegoAndai opened this issue Jan 18, 2024 · 2 comments
Open
Assignees
Labels
component: alert This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material RFC Request For Comments

Comments

@DiegoAndai
Copy link
Member

DiegoAndai commented Jan 18, 2024

What's the problem?

The action, icon, and iconMappings props could use a revision:

  • They don't have respective slots
  • Seems arbitrary that icon is to the left and action to the right
  • iconMapping might be better handled via slots

What are the requirements?

Revise the API, comparing with the rest of components, and deciding whether it's still the best option or if it needs an update.

What are our options?

  • Leave as is
  • Implement something similar to Joy UI

Proposed solution

Implement something similar to Joy UI: deprecate and eventually remove action and icon in favor of startDecorator and endDecorator, each with a corresponding slot like Joy UI. The slots receive the ownerState so they can be used to replace iconMappings.

Resources and benchmarks

Search keywords:

@DiegoAndai DiegoAndai added package: material-ui Specific to @mui/material component: alert This is the name of the generic UI component, not the React module! RFC Request For Comments labels Jan 18, 2024
@DiegoAndai DiegoAndai added this to the Material UI: v7 draft milestone Jan 18, 2024
@DiegoAndai DiegoAndai self-assigned this Jan 18, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 18, 2024

A few notes:

  1. They don't have respective slots

Do they need a slot? Every time we can use a React element over a React component is a significantly better DX (more flexible), but it's not always possible.

  1. Seems arbitrary that icon is to the left and action to the right

Indeed, though it depends on what we optimize for. For example, icon / action is clearer than start / end if it's 80% of the use case.

  1. iconMapping might be better handled via slots

If I recall correctly, this prop is meant for theme customization (use with defaultProps), or through a wrapper to save boilerplate code. It was inspired by the variantMapping prop of the Typography (though there it's a component, not an element as object value).

@DiegoAndai
Copy link
Member Author

@oliviertassinari I think we could optimize for the most common use case but be flexible:

We could keep icon/action as is and introduce leftDecorator and rightDecorator slots for anything beyond that use case. Using leftDecorator would override icon, for example.

Regarding iconMapping. I think using leftDecorator would be more flexible, and users could still provide it as defaultProps. Overall, I think promoting the slots pattern for all specialized use cases where possible would result in users getting familiar with it and incorporating it in their mental models, instead of having different mental models for each component. I would say:

  • If 80% of the component's use cases include certain functionality, we provide a straightforward API for achieving it. In this example, that would be icon and action.
  • For the 20% remaining, promote slots unless impossible or impractical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: alert This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material RFC Request For Comments
Projects
None yet
Development

No branches or pull requests

2 participants