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

[no-Jira] Reapply the autocomplete focus patch to @mui/base #998

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

canac
Copy link
Contributor

@canac canac commented Aug 13, 2024

It was broken by the MUI upgrade in c4d5fa4.

Description

When you hover over autocomplete option and then move the mouse outside of the dropdown, the option was still being highlight. Then if you click outside the dropdown it selects the option, which is confusing to users. I had fixed this behavior before in a patch to @mui/base, but it was inadvertantly broken again by the MUI upgrade in c4d5fa4.

Before

Before.mov

After

After.mov

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

@canac canac added the Preview Environment Add this label to create an Amplify Preview label Aug 13, 2024
@canac canac requested review from caleballdrin and dr-bizz August 13, 2024 16:36
Copy link
Contributor

Copy link
Contributor

Bundle sizes [mpdx-react]

Compared against bbc74f4

No significant changes found

Copy link
Contributor

@caleballdrin caleballdrin left a comment

Choose a reason for hiding this comment

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

It seems to work well on the preview environment!

Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

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

Looks great, I had a question, but nothing more.

Comment on lines +187 to +188
"@mui/base@^5.0.0-beta.36": "patch:@mui/base@npm%3A5.0.0-beta.40#./.yarn/patches/@mui-base-npm-5.0.0-beta.40-248417914d.patch",
"@mui/base@5.0.0-beta.40": "patch:@mui/base@npm%3A5.0.0-beta.40#./.yarn/patches/@mui-base-npm-5.0.0-beta.40-248417914d.patch"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why you need 2 @mui/base resolutions? I see that one is .36 and the other is .40. I assume our different MUI packages use different versions of @mui/base.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yarn creates these resolutions automatically when you commit the patch. I believe you're right. @mui/x-date-pickers depends on ^5.0.0-beta.36 and @mui/material depends on 5.0.0-beta.40, so I believe that this makes sure that all versions of @mui/base are patched. It also created a resolution for @mui/lab, which depends on 5.0.0-alpha.98, that I must have removed, so I'm going to add that one back too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also created a resolution for @mui/lab, which depends on 5.0.0-alpha.98, that I must have removed, so I'm going to add that one back too.

I forgot that that resolution causes build/test issues, and that's why I removed it.

@canac canac force-pushed the reapply-autocomplete-focus-patch branch from 6eee35a to 3294b96 Compare August 14, 2024 15:09
@canac canac force-pushed the reapply-autocomplete-focus-patch branch from 3294b96 to bc539b4 Compare August 14, 2024 15:31
@canac canac merged commit 103d35e into main Aug 14, 2024
18 checks passed
@canac canac deleted the reapply-autocomplete-focus-patch branch August 14, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants