Skip to content

Fix: Popover with hover and click triggers closes on mouseleave #41511

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

Merged
merged 5 commits into from
Jun 3, 2025

Conversation

amitrathiesh
Copy link
Contributor

@amitrathiesh amitrathiesh commented Jun 1, 2025

When a popover is configured with trigger: 'hover click', if you open it by a click, it would incorrectly close when the mouse pointer leaves the trigger element. This was because the mouseleave event (part of the hover trigger) would hide the popover without adequately respecting the click trigger's intent to keep it open.

This commit modifies the click event listener within Tooltip.js (which Popover extends) to explicitly manage the _activeTrigger[TRIGGER_CLICK] state:

  • When a click opens the popover or makes a hover-opened popover sticky, _activeTrigger[TRIGGER_CLICK] is set to true.
  • When a click closes an already click-activated popover, _activeTrigger[TRIGGER_CLICK] is set to false.

The _leave() method, called by mouseleave, already checks _isWithActiveTrigger(). With _activeTrigger[TRIGGER_CLICK] now accurately reflecting the click state, _leave() will not hide a click-activated popover when the mouse leaves the trigger element. The popover will now correctly remain open until a subsequent click closes it.

Description

Motivation & Context

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

Closes #41379

When a popover is configured with `trigger: 'hover click'`,
if you open it by a click, it would incorrectly close when the
mouse pointer leaves the trigger element. This was because the
`mouseleave` event (part of the hover trigger) would hide the
popover without adequately respecting the click trigger's intent
to keep it open.

This commit modifies the click event listener within `Tooltip.js`
(which Popover extends) to explicitly manage the `_activeTrigger[TRIGGER_CLICK]`
state:
- When a click opens the popover or makes a hover-opened popover
  sticky, `_activeTrigger[TRIGGER_CLICK]` is set to `true`.
- When a click closes an already click-activated popover,
  `_activeTrigger[TRIGGER_CLICK]` is set to `false`.

The `_leave()` method, called by `mouseleave`, already checks
`_isWithActiveTrigger()`. With `_activeTrigger[TRIGGER_CLICK]`
now accurately reflecting the click state, `_leave()` will not
hide a click-activated popover when the mouse leaves the trigger
element. The popover will now correctly remain open until a
subsequent click closes it.
@github-project-automation github-project-automation bot moved this to To do in v5.3.7 Jun 2, 2025
@julien-deramond julien-deramond moved this from To do to Review in progress in v5.3.7 Jun 2, 2025
@julien-deramond julien-deramond self-requested a review June 2, 2025 18:01
@mdo
Copy link
Member

mdo commented Jun 2, 2025

@amitrathiesh Can you remove the test file here? I think I've confirmed this fixes the issue, but we don't want that new root file to be added. Thanks!

Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @amitrathiesh 🙏

Reviewing code changes in tooltip.js and popover.js is always a bit tricky. This may not be the ideal approach, but it’s the one I chose to help move things forward—it's debatable, but necessary given the limited test coverage at the moment.

My focus here isn’t on validating whether the behavior is the "correct" or intended one, but rather on ensuring there are no regressions.

To do this, I compared the Popover and Tooltip components from this branch against those in version v5.3.3.

Popover

Use case v5.3.3 This branch
Bootstrap documentation 🟢 🟢
CodePen from #40802 🟢 🟢
trigger: 'click' 🟢 🟢
trigger: 'hover' 🟢 🟢
trigger: 'focus' 🟢 🟢
trigger: 'click hover' 🟢 🟢
trigger: 'click focus' 🟢 🟢
trigger: 'hover focus' 🟢 🟢
trigger: 'click hover focus' 🟢 🟢

Tooltip

Use case v5.3.3 This branch
Bootstrap documentation 🟢 🟢
trigger: 'click' 🟢 🟢
trigger: 'hover' 🟢 🟢
trigger: 'focus' 🟢 🟢
trigger: 'click hover' 🟢 🟢
trigger: 'click focus' 🟢 🟢
trigger: 'hover focus' 🟢 🟢
trigger: 'click hover focus' 🟢 🟢

If anyone has additional tests in mind, please don’t hesitate to share them. 🙏

@julien-deramond
Copy link
Member

After the review, I've:

  • Removed test-popover.html via 15399f4
  • Fixed the linting issues via ee34fa6
  • Added a unit test to try to cover this specific use case via ec5cf1d. Voluntarily done it for the Popover as it's the use case from the issue, but that's debatable as we're modifying the Tooltip component here 🤷
    • ⚠️ Please double-check that it fails when run on the main branch. Done it on my side, but a second look could be helpful 🙏

@julien-deramond julien-deramond self-requested a review June 2, 2025 19:42
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

I can't guarantee it won't impact other things, but in my opinion, it's important that we move forward. Let's give it a try.

⚠️ Commits should be squashed when the PR is merged

@julien-deramond julien-deramond requested a review from mdo June 2, 2025 19:44
@mdo mdo merged commit 13aa16a into twbs:main Jun 3, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from Review in progress to Done in v5.3.7 Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Popovers always close when trigger is set to 'hover click'
3 participants