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

[Menu][material] Fixes slots and slotProps overriding defaults completely #37902

Merged
merged 9 commits into from
Jul 27, 2023
Merged

[Menu][material] Fixes slots and slotProps overriding defaults completely #37902

merged 9 commits into from
Jul 27, 2023

Conversation

gitstart
Copy link
Contributor

Description

Fixes slots & slotProps overriding defaults slots & slotProps on Menu components

Closes #37612


This code was written and reviewed by GitStart Community. Growing great engineers, one PR at a time.

Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@mui-bot
Copy link

mui-bot commented Jul 10, 2023

Netlify deploy preview

https://deploy-preview-37902--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 8b8069b

@danilo-leal danilo-leal changed the title MUI-37612 - [Menu][material] using slots and slotProps overrides defaults completely [Menu][material] Fixes slots and slotProps overriding defaults completely Jul 10, 2023
@danilo-leal danilo-leal added component: menu This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Jul 10, 2023
gitstart and others added 3 commits July 13, 2023 12:57
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@gitstart gitstart marked this pull request as ready for review July 13, 2023 14:02
@gitstart
Copy link
Contributor Author

@DiegoAndai this PR is ready for review

@DiegoAndai DiegoAndai self-requested a review July 13, 2023 14:40
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey @gitstart thanks for working on this!

I left a couple comments that should be fixed 😊

Besides that, we should also add tests to cover the cases this PR fixes

Let me know if you need any help

packages/mui-material/src/Menu/Menu.js Outdated Show resolved Hide resolved
packages/mui-material/src/Menu/Menu.js Outdated Show resolved Hide resolved
packages/mui-material/src/Menu/Menu.js Outdated Show resolved Hide resolved
gitstart and others added 2 commits July 19, 2023 16:43
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@gitstart
Copy link
Contributor Author

Your feedback has been implemented @DiegoAndai, thanks for the review.

@gitstart gitstart requested a review from DiegoAndai July 19, 2023 16:49
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 20, 2023
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey @gitstart! thanks for implementing the feedback, here are a few more comments so we can continue forward with this PR

packages/mui-material/src/Menu/Menu.js Show resolved Hide resolved
packages/mui-material/src/Menu/Menu.test.js Outdated Show resolved Hide resolved
packages/mui-material/src/Menu/Menu.js Outdated Show resolved Hide resolved
docs/translations/api-docs/menu/menu.json Outdated Show resolved Hide resolved
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 24, 2023
gitstart and others added 2 commits July 24, 2023 22:47
Co-authored-by: seunexplicit <48022904+seunexplicit@users.noreply.github.com>
@gitstart
Copy link
Contributor Author

Your comments have been addressed @DiegoAndai, thanks for the feedback.

Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Great work @gitstart! 🎉

@DiegoAndai DiegoAndai merged commit 83a876d into mui:master Jul 27, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Menu][material] using slots and slotProps overrides defaults completely
4 participants