-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(#3800): MobileSearchAutocomplete: focused style persists on blur #3801
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -227,7 +227,7 @@ const SearchAutocompleteButton = React.forwardRef(function SearchAutocompleteBut | |||
|
|||
return ( | |||
<div | |||
{...mergeProps(hoverProps, focusProps, buttonProps)} | |||
{...mergeProps(buttonProps, focusProps, hoverProps)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bit odd that the order here actually mattered, will dig to see why. Ideally it shouldn't rely on the props merge order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
funny enough, it's due to this line here https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/useFocus.ts#L78, it should be undefined
the reason the order mattered with it though is due to how mergeProps works with event handler merging, if you pass null as an event handler, it will remove ALL event handlers of that name
https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/utils/src/mergeProps.ts#L45 typeof null is 'object' so we won't go down this path
instead, we'll fall through here https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/utils/src/mergeProps.ts#L66 and overwrite everything
I'm going to push the change to this branch
Closes #3800, Closes #3861
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: