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

[SelectInput] Fix layout issue with displayEmpty #16743

Merged
merged 2 commits into from
Jul 27, 2019

Conversation

ypresto
Copy link
Contributor

@ypresto ypresto commented Jul 25, 2019

When certain condition with displayEmpty is enabled, SelectInput skips zero-width space hack, leads to layout issue like below:

screenshot

Conditions:

  • renderValue function returns blank string (like '', ' ', '\n' or '\t').
  • multiple === true && value === []

Reproduction:

diff --git a/docs/src/pages/components/selects/MultipleSelect.js b/docs/src/pages/components/selects/MultipleSelect.js
index 4333b82f8..931c1da6c 100644
--- a/docs/src/pages/components/selects/MultipleSelect.js
+++ b/docs/src/pages/components/selects/MultipleSelect.js
@@ -151,13 +151,6 @@ function handleChangeMultiple(event) {
           value={personName}
           onChange={handleChange}
           input={<Input id="select-multiple-placeholder" />}
-          renderValue={selected => {
-            if (selected.length === 0) {
-              return <em>Placeholder</em>;
-            }
-
-            return selected.join(', ');
-          }}
           MenuProps={MenuProps}
         >
           <MenuItem disabled value="">
diff --git a/docs/src/pages/components/selects/SimpleSelect.js b/docs/src/pages/components/selects/SimpleSelect.js
index 8d0e971c3..49c6f877e 100644
--- a/docs/src/pages/components/selects/SimpleSelect.js
+++ b/docs/src/pages/components/selects/SimpleSelect.js
@@ -81,6 +81,7 @@ function handleChange(event) {
           value={values.age}
           onChange={handleChange}
           displayEmpty
+          renderValue={() => '   '}
           name="age"
           className={classes.selectEmpty}
         >

@mui-pr-bot
Copy link

mui-pr-bot commented Jul 25, 2019

Details of bundle changes.

Comparing: cfedbcb...44a1bc3

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.02% 🔺 +0.03% 🔺 329,029 329,083 90,922 90,945
@material-ui/core/Paper 0.00% 0.00% 68,722 68,722 20,472 20,472
@material-ui/core/Paper.esm 0.00% 0.00% 61,984 61,984 19,222 19,222
@material-ui/core/Popper 0.00% 0.00% 28,960 28,960 10,420 10,420
@material-ui/core/Textarea 0.00% 0.00% 5,541 5,541 2,373 2,373
@material-ui/core/TrapFocus 0.00% 0.00% 3,808 3,808 1,602 1,602
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,162 16,162 5,815 5,815
@material-ui/core/useMediaQuery 0.00% 0.00% 3,101 3,101 1,312 1,312
@material-ui/lab 0.00% 0.00% 141,953 141,953 43,911 43,911
@material-ui/styles 0.00% 0.00% 52,124 52,124 15,463 15,463
@material-ui/system 0.00% 0.00% 15,579 15,579 4,431 4,431
Button 0.00% 0.00% 79,740 79,740 24,366 24,366
Modal 0.00% 0.00% 14,632 14,632 5,126 5,126
Portal 0.00% 0.00% 3,484 3,484 1,564 1,564
Rating 0.00% 0.00% 70,491 70,491 22,094 22,094
Slider 0.00% 0.00% 75,323 75,323 23,338 23,338
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 52,056 52,056 13,832 13,832
docs.main 0.00% 0.00% 606,828 606,828 194,402 194,402
packages/material-ui/build/umd/material-ui.production.min.js +0.02% 🔺 +0.01% 🔺 301,611 301,665 86,715 86,726

Generated by 🚫 dangerJS against 44a1bc3

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: select This is the name of the generic UI component, not the React module! labels Jul 25, 2019
@oliviertassinari oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: good for merge labels Jul 25, 2019
@oliviertassinari oliviertassinari self-assigned this Jul 26, 2019
@oliviertassinari oliviertassinari 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 Jul 26, 2019
@ypresto
Copy link
Contributor Author

ypresto commented Jul 26, 2019

Thanks for fixing! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: select This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants