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

Add interactive Popover hover state #100

Closed
wants to merge 4 commits into from

Conversation

ghsteff
Copy link
Contributor

@ghsteff ghsteff commented Feb 9, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Popovers disappear when you hover over them/try to interact with their content while using a hover trigger. There's no options to enable this kinda behavior

What is the new behavior?

Popover's stay visible while they're hovered when using hover trigger

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Screen.Recording.2024-02-12.at.9.16.29.AM.mov

This is to add the ability to interact with the content of a tooltip
Copy link
Member

@amcdnl amcdnl left a comment

Choose a reason for hiding this comment

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

So Tooltips are not supposed to be interactive - if you need a interactive 'tooltip' thats what the popover is for. I'd rather have you style that similar than extend this.

@ghsteff
Copy link
Contributor Author

ghsteff commented Feb 12, 2024

So Tooltips are not supposed to be interactive - if you need a interactive 'tooltip' thats what the popover is for. I'd rather have you style that similar than extend this.

Ah ok, so style a popover as in activate it on hover? I'll look into doing that instead

@ghsteff
Copy link
Contributor Author

ghsteff commented Feb 12, 2024

Ok now this functionality only works on Popover's when the trigger is hover

@ghsteff ghsteff changed the title Add interactive tooltips Add interactive Popover hover state Feb 12, 2024
if (
isPopover &&
(trigger === 'hover' ||
(Array.isArray(trigger) && trigger.includes('hover')))
Copy link
Contributor

Choose a reason for hiding this comment

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

minor optimization - you could make this a variable like isPopoverInteraction and also reuse it for onMouseLeave

@amcdnl amcdnl closed this Feb 12, 2024
@amcdnl amcdnl deleted the feat/make-tooltips-interactable branch February 12, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants