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

[Select] Accept non-component children #33530

Merged
merged 14 commits into from Dec 26, 2022

Conversation

boutahlilsoufiane
Copy link
Contributor

@boutahlilsoufiane boutahlilsoufiane commented Jul 16, 2022

Fix #32253.

This fixes the problem of conditional rendering on the options with "" or passing no React element option to Select component!

@mui-bot
Copy link

mui-bot commented Jul 16, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-33530--material-ui.netlify.app/

Details of bundle changes

Generated by 🚫 dangerJS against c222b56

@boutahlilsoufiane boutahlilsoufiane changed the title [Select] Handle no react elements [Select] Handle children type primitive Jul 16, 2022
packages/mui-material/src/Select/SelectInput.js Outdated Show resolved Hide resolved
packages/mui-material/src/Select/SelectInput.js Outdated Show resolved Hide resolved
packages/mui-material/src/Select/Select.test.js Outdated Show resolved Hide resolved
packages/mui-material/src/Select/Select.test.js Outdated Show resolved Hide resolved
@boutahlilsoufiane
Copy link
Contributor Author

Hi @michaldudak, thanks for reviewing the PR in a short time, I appreciate that and I'm sorry for being late, the changes are done! thanks!

@michaldudak
Copy link
Member

Could you please rebase/merge the latest master? This should fix argos.

@michaldudak michaldudak changed the title [Select] Handle children type primitive [Select] Handle non-component children Aug 3, 2022
@michaldudak michaldudak changed the title [Select] Handle non-component children [Select] Accept non-component children Aug 3, 2022
@boutahlilsoufiane
Copy link
Contributor Author

Hi @michaldudak thanks for the review, the changes are addressed!

@@ -1,22 +1,22 @@
import * as React from 'react';
import Divider from '@mui/material/Divider';
Copy link
Member

Choose a reason for hiding this comment

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

Please revert the change of import order. It's not related to this PR. We usually import react first, then external packages, and then @mui-scoped package.

@michaldudak michaldudak added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI component: select This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material enhancement This is not a bug, nor a new feature labels Oct 12, 2022
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I pushed some cleanup commits to revert the unnecessary changes. The logic & test-case look great. Thanks @boutahlilsoufiane 👌

@boutahlilsoufiane
Copy link
Contributor Author

Hi @mnajdova, I appreciate your help.

@ZeeshanTamboli ZeeshanTamboli removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Dec 21, 2022
@mnajdova mnajdova merged commit a09cc9e into mui:master Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Select] Throwing a value undefined error in latest 5.6.1
5 participants