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

Fix animated multi-value select width bug #5376

Merged
merged 2 commits into from Oct 18, 2022

Conversation

lukebennett88
Copy link
Collaborator

@lukebennett88 lukebennett88 commented Oct 13, 2022

Description

This PR fixes #5251 (where the width of selected item in a multi-value select could sometimes be far too wide if it was animated).

As an added bonus this refactors a class component into a function component.

Since useEffect fires after the first paint, the width that we get from getBoundingClientRect should always be correct.

Before:

React.Select.Animated.Width.Bug.Before.mp4

After:

React.Select.Animated.Width.Bug.After.mp4

In my testing I couldn't reliably reproduce the bug — before I made the changes, most of the time I could get bug to happen, but not always (you can see in the video about that it doesn't happen on my first attempt). After making the changes I haven't been able to reproduce it. I'm reasonably confident that this is fixed, but some extra eyes on this would be appreciated.

@changeset-bot
Copy link

changeset-bot bot commented Oct 13, 2022

🦋 Changeset detected

Latest commit: 436e250

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-select Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 13, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 436e250:

Sandbox Source
react-codesandboxer-example Configuration

@lukebennett88 lukebennett88 force-pushed the luke/fix-animated-multiselect-width branch 2 times, most recently from bc15f11 to 61a6965 Compare October 13, 2022 06:09
@lukebennett88 lukebennett88 added the pr/bug-fix PRs that are specifically addressing a bug label Oct 13, 2022
@lukebennett88 lukebennett88 force-pushed the luke/fix-animated-multiselect-width branch 2 times, most recently from b89fbc8 to 528284f Compare October 13, 2022 06:42
@Rall3n
Copy link
Collaborator

Rall3n commented Oct 14, 2022

Easiest solution would have been to enforce flex layout if isMulti is set, as the problem probably stems from switching between grid and flex when the first option is being selected after typing at least one letter.

@lukebennett88 lukebennett88 force-pushed the luke/fix-animated-multiselect-width branch from 528284f to 00238f1 Compare October 17, 2022 02:51
@lukebennett88
Copy link
Collaborator Author

Thanks @Rall3n, I've updated this PR to be much simpler now that I know what the root cause is 😅

@lukebennett88 lukebennett88 merged commit f369bba into master Oct 18, 2022
@lukebennett88 lukebennett88 deleted the luke/fix-animated-multiselect-width branch October 18, 2022 02:25
@github-actions github-actions bot mentioned this pull request Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/bug-fix PRs that are specifically addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI issue when using the makeAnimated()
3 participants