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

[docs] Add note in docs about componentsProps.root taking precedence #33097

Conversation

ZeeshanTamboli
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli commented Jun 10, 2022

While working on SnackbarUnstyled component, I stumbled upon a use case about what if both props are specified which one will take precedence?
componentsProps.root or the additional native DOM props?

This is what I mean - see the following CodeSandbox: https://codesandbox.io/s/eloquent-hugle-xfri7g?file=/src/Demo.tsx:1245-1449
Here the id badge-1 of root slot gets applied, not badge-2. Hence mentioned it in the docs.

However, I think that it should be the other way around, the additional native props passed directly on the component should take precedence. But then we will have to change the implementation on all the components.

Docs preview: https://deploy-preview-33097--material-ui.netlify.app/base/getting-started/usage/

@ZeeshanTamboli ZeeshanTamboli changed the title Base components props root slot precedence docs [docs] Add note in docs about componentsProps.root taking precedence Jun 10, 2022
@ZeeshanTamboli ZeeshanTamboli added docs Improvements or additions to the documentation package: base-ui Specific to @mui/base labels Jun 10, 2022
@mui-bot
Copy link

mui-bot commented Jun 10, 2022

No bundle size changes

Generated by 🚫 dangerJS against e985016

@mnajdova
Copy link
Member

cc @michaldudak can you verify and approve?

Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

However, I think that it should be the other way around, the additional native props passed directly on the component should take precedence.

IMO what's set explicitly should override values set implicitly. If you set componentsProps.root.id = 'x', you clearly state that the id prop of the root slot should be 'x'. On the other hand, spreading additional unstyled component's props on the root is more of a convention. That's why I think the current way is more intuitive.

docs/data/base/getting-started/usage/usage.md Outdated Show resolved Hide resolved
@ZeeshanTamboli
Copy link
Member Author

ZeeshanTamboli commented Jun 24, 2022

I am merging this. The docs suggestion is applied. Could be helpful in #33272.

@ZeeshanTamboli ZeeshanTamboli merged commit a59baca into mui:master Jun 24, 2022
@ZeeshanTamboli ZeeshanTamboli deleted the base-componentsProps-root-slot-precedence-docs branch June 24, 2022 15:13
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
mui#33097)

* docs

* docs wording

* run CI

* Update docs/data/base/getting-started/usage/usage.md

Co-authored-by: Michał Dudak <michal.dudak@gmail.com>

Co-authored-by: Michał Dudak <michal.dudak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation package: base-ui Specific to @mui/base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants