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

[input] Fix layout shift with display: flex #43839

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Sep 21, 2024

Fix #39539. A regression introduced in Chrome #26449 (comment) after #28070.

As an end-user, I was annoyed by #39539 (comment), the template doesn't feel great with a bug like this. It makes us look like amateurs.

You can test that is works on https://deploy-preview-43839--material-ui.netlify.app/experiments/base/autofill/.

Screen.Recording.2024-09-22.at.01.44.40.mov

and https://deploy-preview-43839--material-ui.netlify.app/material-ui/getting-started/templates/dashboard/.


TODO

  • Regarding [InputBase] Fix autofill issue #28070 I'm not sure what happened but emotion doesn't removed empty keyframe with the version that we have today:

  • @emotion/styled@11.13.0

  • @emotion/react@11.13.3

SCR-20240922-cljq

The reproduction in #26449 uses:

To dive deeper, don't want to merge without clarity on this.

  • Is this still the best way to solve this problem?

@mui-bot
Copy link

mui-bot commented Sep 21, 2024

Netlify deploy preview

https://deploy-preview-43839--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 9257196

@oliviertassinari oliviertassinari changed the title Branch test [input] Fix layout shift with display: flex Sep 21, 2024
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: text field This is the name of the generic UI component, not the React module! regression A bug, but worse labels Sep 21, 2024
@oliviertassinari oliviertassinari marked this pull request as ready for review September 21, 2024 23:45
@zannager zannager requested a review from siriwatknp September 23, 2024 13:44
Comment on lines +260 to +261
'@keyframes mui-auto-fill': {},
'@keyframes mui-auto-fill-cancel': {},
Copy link
Member

Choose a reason for hiding this comment

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

Why not removing the InputGlobalStyles entirely?

Copy link
Member Author

Choose a reason for hiding this comment

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

Per the history of those lines, it was required for the autofill detection logic to work. But, ok, I didn't test if it's still true.

Copy link
Member

Choose a reason for hiding this comment

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

If this trick involving the dummy animation to expose a JS hook still works, it may be needed to fix #44135 (comment)

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: text field This is the name of the generic UI component, not the React module! regression A bug, but worse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][InputBase] Auto-fill keyframes may cause unwanted animations
4 participants