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] Prevent shrink animation in uncontrolled Autocomplete when default value is set #44873

Merged

Conversation

ZeeshanTamboli
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli commented Dec 28, 2024

Closes #44506

Also related to #36548 (see comment 1 and comment 2).

Problem

The shrink animation on input label is triggered when the HTML input value in the notched outlined input component changes from empty to filled.

In the Autocomplete component, the HTML input value is determined by the inputValue prop. Initially, inputValue is empty (source) when defaultValue prop is provided. The input value is later updated via state (source), causing a re-render and the shrink animation.

Fix

Compute the initial inputValue on the first render when defaultValue is provided. This ensures the input is correctly initialized, preventing the shrink animation on InputLabel.

Alternative that can be considered

Tell developer to use a controlled Autocomplete by managing inputValue and onInputValueChange. This avoids reliance on the default state mechanism. CodeSandbox: https://codesandbox.io/p/sandbox/stoic-fire-zz67mz-forked-8k9lmg

But since we already set the HTML input value using defaultValue, it makes sense to initialize inputValue accordingly during the first render to avoid the animation. And it already works for other components like TextField and Select.

Not sure how to add a test case for this.

Demos

@ZeeshanTamboli ZeeshanTamboli added the component: autocomplete This is the name of the generic UI component, not the React module! label Dec 28, 2024
@mui-bot
Copy link

mui-bot commented Dec 28, 2024

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 4c51335

@ZeeshanTamboli ZeeshanTamboli changed the title [Autocomplete] Fix shrink animation in Autocomplete when value is set [WIP][Autocomplete] Fix shrink animation in Autocomplete when value is set Dec 28, 2024
@ZeeshanTamboli ZeeshanTamboli changed the title [WIP][Autocomplete] Fix shrink animation in Autocomplete when value is set [Autocomplete] Fix shrink animation in Autocomplete when value is set Jan 2, 2025
@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work package: material-ui Specific to @mui/material labels Jan 2, 2025
@ZeeshanTamboli ZeeshanTamboli marked this pull request as ready for review January 2, 2025 13:38
@ZeeshanTamboli ZeeshanTamboli changed the title [Autocomplete] Fix shrink animation in Autocomplete when value is set [Autocomplete] Prevent shrink animation in Autocomplete when value is set Jan 2, 2025
@ZeeshanTamboli ZeeshanTamboli changed the title [Autocomplete] Prevent shrink animation in Autocomplete when value is set [Autocomplete] Prevent shrink animation in Autocomplete when value is set for uncontrolled Autocomplete Jan 4, 2025
@ZeeshanTamboli ZeeshanTamboli changed the title [Autocomplete] Prevent shrink animation in Autocomplete when value is set for uncontrolled Autocomplete [Autocomplete] Prevent shrink animation in Autocomplete when default value is set for uncontrolled Autocomplete Jan 4, 2025
@ZeeshanTamboli ZeeshanTamboli changed the title [Autocomplete] Prevent shrink animation in Autocomplete when default value is set for uncontrolled Autocomplete [Autocomplete] Prevent shrink animation in uncontrolled Autocomplete when default value is set Jan 4, 2025
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

@ZeeshanTamboli this solution makes sense 👍🏼

About the test, have you tried adding a regression test? Animations should be disabled on those, so with the bug the label would be on top of the input.

@ZeeshanTamboli
Copy link
Member Author

About the test, have you tried adding a regression test? Animations should be disabled on those, so with the bug the label would be on top of the input.

@DiegoAndai Yes, I tried adding a regression test in the master branch to reproduce the bug. Here's what I did:

In test/regressions/index.test.js, I added this test:

it('should not perform shrink animation when default value is provided', async () => {
  const testcase = await renderFixture(
    '/regression-Autocomplete/ShrinkAnimationDefaultValue',
  );
  await takeScreenshot({
    testcase,
    route: '/regression-Autocomplete/ShrinkAnimationDefaultValue2',
  });
});

The ShrinkAnimationDefaultValue component:

import * as React from 'react';
import Autocomplete from '@mui/material/Autocomplete';
import TextField from '@mui/material/TextField';

const options = [
  { title: 'The Shawshank Redemption', year: 1994 },
  { title: 'The Godfather', year: 1972 },
  { title: 'The Godfather: Part II', year: 1974 },
];

export default function Demo() {
  return (
    <Autocomplete
      options={options}
      getOptionLabel={(option) => option.title}
      defaultValue={options[1]}
      renderInput={(params) => <TextField {...params} label="limitTags" />}
      sx={{ width: '500px', m: 4 }}
    />
  );
}

However, the shrink animation still doesn't trigger for the screenshot. Here's a video of the issue on the master branch:

Screen-Recording.2.mp4

@ZeeshanTamboli
Copy link
Member Author

@DiegoAndai This is ready for further review. Just pinging you as a reminder.

@DiegoAndai
Copy link
Member

However, the shrink animation still doesn't trigger for the screenshot

@ZeeshanTamboli, I don't understand. Isn't that what we want to fix? The shrink animation not to trigger?

Or is it that it doesn't trigger without the fix?

@ZeeshanTamboli
Copy link
Member Author

ZeeshanTamboli commented Jan 17, 2025

Or is it that it doesn't trigger without the fix?

@DiegoAndai Yes, the animation doesn't trigger without the fix. I tried this by running the test on the master branch, which doesn't include the fix. Can you try?

@DiegoAndai
Copy link
Member

DiegoAndai commented Jan 17, 2025

Can you try?

@ZeeshanTamboli you're right, I was wrong about how the animation would work.

I found a way to test this by spying on onInputChange. Before the fix, this is triggered with the 'reset' reason from here. With the fix, it's not called as it returns here.

Let me know if this makes sense.

@ZeeshanTamboli
Copy link
Member Author

ZeeshanTamboli commented Jan 18, 2025

I found a way to test this by spying on onInputChange. Before the fix, this is triggered with the 'reset' reason from here. With the fix, it's not called as it returns here.

Let me know if this makes sense.

@DiegoAndai Good idea. It makes sense. 👍

@DiegoAndai DiegoAndai merged commit 8a4eaf6 into mui:master Jan 21, 2025
19 checks passed
@ZeeshanTamboli ZeeshanTamboli deleted the fix-44506-autocomplete-shrink-animation branch January 21, 2025 13:28
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! package: material-ui Specific to @mui/material
Projects
None yet
3 participants