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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Autocomplete] Padding issue with variant="filled" and hiddenLabel #21409

Open
2 tasks done
michael-land opened this issue Jun 12, 2020 · 5 comments
Open
2 tasks done
Labels
bug 馃悰 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process waiting for 馃憤 Waiting for upvotes

Comments

@michael-land
Copy link
Contributor

Related to #21408

image

image

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

Steps to Reproduce 馃暪

Steps:

  1. https://codesandbox.io/s/lucid-franklin-qf5lc?file=/src/Demo.js
@michael-land michael-land added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 12, 2020
@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Jun 12, 2020
@oliviertassinari
Copy link
Member

The padding is optimized for the need of the component.

@oliviertassinari oliviertassinari added support: question Community support but can be turned into an improvement and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 12, 2020
@oliviertassinari oliviertassinari added bug 馃悰 Something doesn't work and removed support: question Community support but can be turned into an improvement labels Aug 17, 2020
@oliviertassinari
Copy link
Member

@xiaoyu-tamu What do you think about this fix?

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 655dbef5e4..a840da5a08 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -110,7 +110,7 @@ export const styles = (theme) => ({
         paddingRight: 52 + 4 + 9,
       },
       '& $input': {
-        padding: '9px 4px',
+        padding: '9.5px 4px',
       },
       '& $endAdornment': {
         right: 9,
@@ -122,6 +122,10 @@ export const styles = (theme) => ({
         padding: '4.5px 4px',
       },
     },
+    '&[class*="MuiFilledInput-root"][class*="MuiInputBase-hiddenLabel"]': {
+      paddingTop: 9,
+      paddingBottom: 9,
+    },
   },
   /* Styles applied to the input element. */
   input: {
diff --git a/packages/material-ui/src/InputBase/InputBase.d.ts b/packages/material-ui/src/InputBase/InputBase.d.ts
index 43a3584592..38b5ac586a 100644
--- a/packages/material-ui/src/InputBase/InputBase.d.ts
+++ b/packages/material-ui/src/InputBase/InputBase.d.ts
@@ -158,9 +158,10 @@ export type InputBaseClassKey =
   | 'error'
   | 'marginDense'
   | 'multiline'
-  | 'fullWidth'
   | 'colorSecondary'
   | 'input'
+  | 'fullWidth'
+  | 'hiddenLabel'
   | 'inputMarginDense'
   | 'inputMultiline'
   | 'inputTypeSearch'
diff --git a/packages/material-ui/src/InputBase/InputBase.js b/packages/material-ui/src/InputBase/InputBase.js
index af52cbf82c..25eb829f52 100644
--- a/packages/material-ui/src/InputBase/InputBase.js
+++ b/packages/material-ui/src/InputBase/InputBase.js
@@ -78,6 +78,8 @@ export const styles = (theme) => {
     fullWidth: {
       width: '100%',
     },
+    /* Styles applied to the root element if `hiddenLabel={true}`. */
+    hiddenLabel: {},
     /* Styles applied to the `input` element. */
     input: {
       font: 'inherit',
@@ -422,6 +424,7 @@ const InputBase = React.forwardRef(function InputBase(props, ref) {
           [classes.multiline]: multiline,
           [classes.adornedStart]: startAdornment,
           [classes.adornedEnd]: endAdornment,
+          [classes.hiddenLabel]: fcs.hiddenLabel,
         },
         className,
       )}

Do you want to work on it? We would need to add a new visual regression test

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 17, 2020
@oliviertassinari oliviertassinari changed the title Autocomplete padding inconsistencies when variant is filled and hiddenLabel. [Autocomplete] Padding issue with variant="filled" and hiddenLabel Aug 17, 2020
@fakeharahman
Copy link
Contributor

Hi, I'm new to open source, can I try to solve this issue?

@Segolene-Alquier
Copy link

Segolene-Alquier commented Oct 3, 2020

Hey, I see the last comment dates from the 18th of August. Can I make a PR for this one if it hasn't been fixed yet? 馃

@oliviertassinari oliviertassinari removed the good first issue Great for first contributions. Enable to learn the contribution process. label Oct 7, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 7, 2020

@Segolene-Alquier thanks for the interest in this issue. We had a similar problem and solution proposed in #22544 however, the pull request that was meant to solve the issue was declined. I have removed the "good first issue" label because it requires further discussion.

From what I understand, the solution that relies on a globale CSS selector was declined by @eps1lon because developers can customize them to whatever they want (rarely done). If a developer does such, he would break my previous proposed solution.

I see two possible directions going forward:

  1. We anticipate that in v5, we have hard-coded class names with no options to remove the default ones (can only add). I'm not entirely sure yet that we will do such cc @mnajdova.
  2. We refactor the input components logic to have two modes. One mode where the <input> takes the whole space available, the default. And a second mode for the Autocomplete and TextareaAutosize. It's more work, riskier but has potential.

@oliviertassinari oliviertassinari added waiting for 馃憤 Waiting for upvotes and removed value: low labels Nov 5, 2021
@mj12albert mj12albert added the design: material This is about Material Design, please involve a visual or UX designer in the process label Nov 22, 2023
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! design: material This is about Material Design, please involve a visual or UX designer in the process waiting for 馃憤 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

5 participants