[Input][Joy] Fix autofill styles

[siriwatknp]

Close #34071

After:

https://deploy-preview-35056--material-ui.netlify.app/joy-ui/react-text-field/

[mui-bot]

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

Details of bundle changes

@mui/joy: parsed: +0.18% , gzip: +0.09%

Generated by 🚫 dangerJS against 667cb1e

[oliviertassinari]

Nice for improving the forms UX 👍

[oliviertassinari]

We have a negative margin that only applies when autofill is triggered. I would image that it might make it harder for the developers to customize the component.

I think that we could move the change of margin outside of :-webkit-autofill so it always apply (to catch customization bug sooner), and maybe to see if we can remove the need to a negative margin.

[siriwatknp]

I tried but it is more complicated because the Autocomplete is affected. I think it makes more sense to put them under :webkit-autofill if you think of it as a module.

It also looks weird for simple use case when you inspect the element and found that the negative margin is there at the beginning?

[oliviertassinari]

It also looks weird for simple use case when you inspect the element and found that the negative margin is there at the beginning?

I think that it's the value of doing it. I have struggled to understand how this could work in the first place because it was hard to reproduce, I needed to trigger the autofill style. So IMHO, it would be better to move the margin change to be always there, for predictability. For example, so that as a developer, when you do a customization, you don't realize 4 days later, through customer support, that the input looks broken on the login page.

[siriwatknp]

I have struggled to understand how this could work in the first place because it was hard to reproduce, I needed to trigger the autofill style

If the problem is about triggering the autofill styles, we should fix it or let the browser take care of it.

Using a negative margin directly should not be the solution to this.

So IMHO, it would be better to move the margin change to be always there, for predictability

It does not guarantee that what you customize will work with autofill. For example, this won't look good in autofill styles:

<Input sx={{ px: 1, py: 1 }} />

I rather keep the initial CSS plain as much as possible and add extra styles for autofill. If you don't toggle the autofill styles, there is no way to be sure that the autofill styles work.

[oliviertassinari]

this won't look good in autofill styles:

To continue on this example, my concern is about:

1. the delay feedback. In a first PR, a developer is likely to take the simple approach: https://codesandbox.io/s/elegant-lake-xftkxy?file=/demo.tsx, which would look good:

but, a few days later, he would notice that the autofill style is broken in his form. He doesn't get this to work "for free":

2. the extra complexity to customize. Without '&:-webkit-autofill': {, this would be enough:

<Input sx={{ '& input': { px: 1, py: 1 } }} placeholder="Placeholder" />

but with the current logic, we need to do this customization to get the right style in all the cases. It's harder to figure out:

<Input
  sx={{
    px: 1,
    py: 1,
    "& input:-webkit-autofill": {
      mx: -1,
      my: -1,
      px: 1,
      py: 1
    }
  }}
  placeholder="Placeholder"
/>

[oliviertassinari]

Either way (with or without a negative margin), it will delay the feedback.

I don't understand this point. To be clear, my proposal is to remove '&:-webkit-autofill': { so that developers don't need to be in a form with the autofill triggered to be able to customize the component.

---

I don't share the same vison on this but no objections to move forward with the current changes.
I would expect that users' feedback will utimately surface the same pain point that I raised, and if they don't, great.

At the core I think that the difference of our perspectives is about how much the autofill use case of an input inside a form is an edge case to the component or core. I think it's core, that no customization should be done without this constraint.

This is why we will need to educate developers to use CSS variables

I agree, but I think that it shouldn't be our default go to solution. For example, we would likely need a CSS vars for padding horizontal and one for vertical. It's also not something that developers can easily find with the dev tools. It would be simpler if raw CSS was working.

If your concern is with the use of negative margins (I personally don't mind), we could maybe change the styles to not rely on them.

---

Off-topic. I wonder if marginLeft wouldn't be better than marginInlineStart. It's clearer to me what the former does. I have to google the latter to understand what's going on. I would expect it's the same for all the backend developers that uses us. With the RTL plugin that swap the properties automatically, I would expect no difference in real life.

[mnajdova]

👍 for marginInlineStart, we are already using paddingInlineStart. Regarding the styles being added in the :webkit-autofill, I am fine with it, having fewer styles by default, and I agree with what @siriwatknp said, you anyway need to verify that the styles for autofill work if you are doing some customizations.

[oliviertassinari]

we are already using paddingInlineStart

@mnajdova I would apply the same reasoning. This CSS property has an extra mental step for a developer to understand what it does compared to paddingLeft. The problem, is that paddingInlineStart seems to solve almost no problems in exchange for the customization overhead it creates for a developer that want to change the padding left.

you anyway need to verify that the styles for autofill work

The problem is with how much CSS developers have to write to customization the output. In the previous comments I made the point that with how it's on HEAD, developers have to write

<Input
  sx={{
    px: 1,
    py: 1.5,
    "& input:-webkit-autofill": {
      mx: -1,
      my: -1,
      px: 1,
      py: 1.5,
    }
  }}
  placeholder="Placeholder"
/>

to get the correct output. Vs how it could look like for developers if Joy UI's input was using negative margin:

<Input
  sx={{ '& input': { px: 1, py: 1.5 } }}
  placeholder="Placeholder"
/>

[mnajdova]

Fair enough. My thought was that we would advertise:

<Input sx={{ '--Input-paddingInline': '8px' }} />

Which is the way towards which we want to go.

[oliviertassinari]

I would be curious to see developers' feedback on this. On my end, I can't imagine that developers would want

<Input
  sx={{ '--Input-paddingInline': '8px', '--Input-paddingBlock': '12px' }}
  placeholder="Placeholder"
/>

when they can have:

<Input
  sx={{ '& input': { px: 1, py: 1.5 } }}
  placeholder="Placeholder"
/>

but maybe I'm wrong, so to test out 👍

[mnajdova]

I noticed this white gap near the bottom border:

[siriwatknp]

I noticed this white gap near the bottom border:

Is it because you zoom in? I experience it sometimes.

[mnajdova]

Is it because you zoom in? I experience it sometimes.

I can see it on the side window on Codesandbox, can't see it on the docs 🤷‍♀️ Not really useful info I guess...