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
[Tooltip] Fix tooltip visibility logic #1035
Conversation
} | ||
window.clearTimeout(openTimerRef.current); |
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.
I think I was a bit too trigger happy when refactoring everything into this onChange handler after the original PR feedback because this was only clearing if the open
value changed from false
to true
(or vice versa) which is incorrect:
CleanShot.2021-12-13.at.14.43.37.mp4
When the mouse enters, the open timer starts but open
is still false, then the mouse leaves which calls setOpen(false)
and because that is the current value, the open timer wasn't cleared.
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.
Good catch!
}); | ||
})} | ||
onFocus={composeEventHandlers(props.onFocus, () => { | ||
if (!isMouseDownRef.current) context.onOpen(); |
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.
Explicitly only open if it isn't a mousedown focus.
8bafeef
to
3e8d9e8
Compare
} | ||
window.clearTimeout(openTimerRef.current); |
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.
Good catch!
70b594c
to
a1726e8
Compare
A couple of fixes here so I've left comments inline to clarify what's what.