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

[Autocomplete] Adding startAdornment results in extra top padding on IE11 only #19213

Closed
2 tasks done
aisamu opened this issue Jan 13, 2020 · 9 comments · Fixed by #17680
Closed
2 tasks done

[Autocomplete] Adding startAdornment results in extra top padding on IE11 only #19213

aisamu opened this issue Jan 13, 2020 · 9 comments · Fixed by #17680
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@aisamu
Copy link
Contributor

aisamu commented Jan 13, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Using a startAdornment on AutoComplete's renderInput results in an extra top padding on IE11.

We've seen it with the outlined variant, but only when we set a placeholder.(i.e. works with label).

Manually setting the TextField's notched property to false appears to "fix" it.

Here's the issue in action (the table structure mimicks the components' placement on the screenshots):

Chrome Label Placeholder
Regular
Label + startAdornment Placeholder + startAdornment
IE11 Label Placeholder Padded
Label + startAdornment Placeholder + startAdornment

Inspecting the generated DOM element shows that the <fieldset> tag has a top: -5px property which, when cleared, fixes the issue:

Inspected

.PrivateNotchedOutline-root-85 {
  top: -5px;    <-----
  left: 0;
  right: 0;
  ...
}

Expected Behavior 🤔

Should be identical to the other browsers (tested on 5 others, listed below)

Steps to Reproduce 🕹

The modified Material UI autocomplete demo above should show the issue, but unfortunately codesandbox.io is currently broken on IE11 due to transpiling issues.

The gist of the demo is to create an AutoComplete with the placeholder and startAdornment params set on renderInput's TextField and visualize it on IE11:

renderInput={params => (
  <TextField
    {...params}
    placeholder="Placeholder text"
    variant="outlined"
    InputProps={{
      ...params.InputProps,
      startAdornment: <span>span</span>
    }}
    fullWidth
  />
)}

Context 🔦

We're happy MaterialUI users with IE11 users 😄

Your Environment 🌎

Tech Version
@material-ui/core 4.7.2
@material-ui/lab 4.0.0-alpha.38
React 16.12.0
Browser IE11 (❌ ), Chrome 79 (✅) , Firefox 72(✅), Edge 18(✅ , Edge 79(✅ ), Safari 13 (✅ )

Thanks for this great project!

@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Jan 14, 2020
@oliviertassinari
Copy link
Member

@aisamu Did you find a solution that we could consider deploying? What's so special about the autocomplete that breaks it?

@aisamu
Copy link
Contributor Author

aisamu commented Jan 14, 2020

I'm not sure.
Manually setting the TextField's notched property to false removes the extra padding from IE11, bringing it in line with the other browsers.
The reason I'm not confident to call that a fix is that the other browsers work without that flag set.

@oliviertassinari
Copy link
Member

@aisamu We very likely have a solution in #17680 (comment). Could you confirm?

@aisamu
Copy link
Contributor Author

aisamu commented Jan 17, 2020

@oliviertassinari Yup, but I'm unsure how to test that locally.

yarn add https://github.com/eps1lon/material-ui.git#7d61d14hangs at "resolving packages..." , so I cloned the repo and added links for the relevant packages, per yarn's documentation

yarn add link:../material-ui/packages/material-ui
yarn add link:../material-ui/packages/material-ui-icons
yarn add link:../material-ui/packages/material-ui-lab

That apparently works, and package.json seems to be correctly populated:

    "@material-ui/core": "link:../material-ui/packages/material-ui",
    "@material-ui/icons": "link:../material-ui/packages/material-ui-icons",
    "@material-ui/lab": "link:../material-ui/packages/material-ui-lab",

Unfortunately, the build fails to resolve things properly, emitting errors like this:

looking for modules in /projects/my-project/node_modules
  using description file: /projects/my-project/package.json (relative path: ./node_modules)
    using description file: /projects/my-project/node_modules/@material-ui/core/package.json (relative path: ./Typography)
      no extension
        resolved symlink to /projects/material-ui/packages/material-ui/Typography
          using description file: /projects/material-ui/packages/material-ui/package.json (relative path: ./Typography)
            no extension
              /projects/material-ui/packages/material-ui/Typography doesn't exist
            .wasm
              /projects/material-ui/packages/material-ui/Typography.wasm doesn't exist
            .mjs
              /projects/material-ui/packages/material-ui/Typography.mjs doesn't exist
            .js
              /projects/material-ui/packages/material-ui/Typography.js doesn't exist
...

There is a Typography.js, but on /projects/material-ui/packages/material-ui/ src/Typography/ Typography.js"

I'm not sure if that's related to the way material-ui packages things separately on the mono-repo or if it's due to our own webpack setup.

Using fully resolved links on package.json yielded the same results
(as in "@material-ui/core": "link:/projects/material-ui/packages/material-ui")

@oliviertassinari
Copy link
Member

@aisamu You could open your node_modules and apply the patch. I have never seen yarn link really work here.

@aisamu
Copy link
Contributor Author

aisamu commented Jan 20, 2020

@aisamu We very likely have a solution[...]. Could you confirm?

The height is now correct (IE11 now matches Chrome/FF/Safari), but it looks like there's a small visual regression when placeholders are used instead of labels (on all browsers), in the form of a discontinuity on the outline:

Chrome Screen Shot 2020-01-19 at 23 35 09
IE11 Screen Shot 2020-01-19 at 23 35 29

@oliviertassinari
Copy link
Member

Don't worry about the gap, it's another issue and being taken care of.

@aisamu
Copy link
Contributor Author

aisamu commented Jan 20, 2020

Don't worry about the gap

London Underground would like to have a word with you

it's another issue and being taken care of.

Again, thanks!

@oliviertassinari
Copy link
Member

"Please mind the gap between the train and the station" 🙉

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: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants