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(button): mouse out #9425

Closed
wants to merge 11 commits into from

Conversation

sstrubberg
Copy link
Member

@sstrubberg sstrubberg commented Aug 5, 2021

Closes #8961
Refs #7040
Refs #4560
Refs #8022

*** DO NOT MERGE** until the MouseOut logic is removed from the Button story.

Refactored Button as it pertains to how it handles tooltips.

Changed

  • In order to isolate the issue, we ended up stripping Button down of most of it's logic to get a sense of what conditionals were happening when the functions handleFocus, handleBlur, handleMouseEnter, and handleMouseLeave ran. In doing so we fairly quickly realized we didn't need the alllowTooltipVisibilty, isHovered, and isFocused states; and replaced them with simply isTooltipVisible. This took care of most of the work, but then we found that there were instances where multiple tooltips would occupy the screen simultaneously in some conditions. We ended up gutting the old closeTooltips function and replaced it with the global store for tracking and closing tooltips.

Testing / Reviewing

Open storybook and examine Button, specifically iconOnly button and the MouseOut story (the story that needs to be removed before merging the PR). Ensure the tooltips on the button fire on/off correctly with mouse and keyboard control.

@netlify
Copy link

netlify bot commented Aug 5, 2021

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 244edf7

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/611d5b6c43745d0008d167db

😎 Browse the preview: https://deploy-preview-9425--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Aug 5, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 244edf7

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/611d5b6ccca1ec0007bcd6e2

😎 Browse the preview: https://deploy-preview-9425--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Aug 5, 2021

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 244edf7

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/611d5b6c7d519d0007bc7693

😎 Browse the preview: https://deploy-preview-9425--carbon-components-react.netlify.app

@sstrubberg sstrubberg marked this pull request as ready for review August 6, 2021 11:37
@sstrubberg sstrubberg requested a review from a team as a code owner August 6, 2021 11:37
@tw15egan
Copy link
Collaborator

tw15egan commented Aug 6, 2021

It seems like tooltips on focus are no longer appearing

2021-08-06 09 11 35

I'd take a look at #8022 and ensure all use-cases are still being handled, and we'll also need to port these same changes over to TooltipIcon

@dakahn
Copy link
Contributor

dakahn commented Aug 18, 2021

Added the WIP label since it seems like this one is still cookin 👍🏾

@sstrubberg sstrubberg marked this pull request as draft August 19, 2021 16:38
@sstrubberg sstrubberg closed this Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tooltip doesn't disappear on mouseout when icon-only Button was previously disabled
4 participants