Skip to content

[2.x] fix(tooltip): resolve native tooltip flash, async teardown leaks, and dynamic text - #4674

Merged
imorland merged 4 commits into
flarum:2.xfrom
huoxin233:huoxin/2.x-fix-tooltip
May 30, 2026
Merged

[2.x] fix(tooltip): resolve native tooltip flash, async teardown leaks, and dynamic text#4674
imorland merged 4 commits into
flarum:2.xfrom
huoxin233:huoxin/2.x-fix-tooltip

Conversation

@huoxin233

Copy link
Copy Markdown
Contributor

Fixes #4596

Changes proposed in this pull request:

Tooltip component (common/components/Tooltip.tsx):

  • Text-only changes now update the existing Bootstrap tooltip in-place (via data-original-title and .tooltip-inner) instead of destroying and recreating the entire instance, which caused an async race condition that left the element with no active tooltip.
  • Tooltip is now initialized via the Bootstrap title config option with the DOM title attribute explicitly removed, preventing the
    browser's native tooltip from ever appearing as a fallback.
  • After an in-place text update, tooltip.show() is called to force Bootstrap to recalculate positioning, preventing vertical
    misalignment when surrounding layout shifts (e.g. the announcement list expanding/collapsing).
  • The .fade CSS class is stripped before every destroy() call to force synchronous cleanup, preventing stranded tooltip DOM nodes and race conditions during rapid updates.
  • checkDomNodeChanged() now properly destroys the tooltip on the old DOM node before reassigning, fixing a pre-existing leak where swapped nodes left orphaned tooltip instances.

AnnouncementsWidget (admin/components/AnnouncementsWidget.tsx):

  • Added e.currentTarget.blur() to the visibility toggle button's click handler so the tooltip dismisses on mouse-out after clicking.

Reviewers should focus on:

For the blur() call in AnnouncementsWidget, I am not fully sure if this is the right approach. The issue is that after clicking the show/hide button, the browser keeps the button focused, which causes the tooltip to stay visible even after the cursor leaves (because tooltips trigger on both hover and focus). The blur() call fixes this, but other buttons in Flarum also has the same issue (e.g. the Tools button on top, shown in the video below).

So I am not sure if this is a intended design or this should be fix (if we are fixing that at a broader scope, then probably I should not include the fix for AnnouncementsWidget in this PR). I will leave this to the team to decide.

Screenshot

Before applying blur():

PixPin_2026-05-29_00-47-14.mp4

After applying blur():

PixPin_2026-05-29_00-48-40.mp4

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)

huoxin233 and others added 2 commits May 29, 2026 00:58
… dynamic text

This overhauls the Tooltip component to handle dynamic updates properly:
- Initializes tooltip via options `title` instead of `data-original-title`
  attribute, removing the native browser tooltip fallback flash.
- Updates `.tooltip-inner` in-place when text dynamically changes to
  avoid race conditions with asynchronous teardown.
- Calls `tooltip.show()` after text updates to force recalculation of
  absolute coordinates, preventing vertical misalignment.
- Strips `.fade` class immediately during `onremove` and DOM changes to
  enforce synchronous cleanup and prevent stranded tooltips.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After toggling announcement visibility, the button retains browser focus, which keeps the tooltip visible even after the cursor moves away (since the tooltip triggers on both hover and focus). Call blur() on the button after the click to release focus and allow the tooltip to dismiss naturally on mouse-out.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@huoxin233
huoxin233 requested a review from a team as a code owner May 28, 2026 16:59
@huoxin233 huoxin233 changed the title fix(tooltip): resolve native tooltip flash, async teardown leaks, and dynamic text [2.x] fix(tooltip): resolve native tooltip flash, async teardown leaks, and dynamic text May 28, 2026
@dsevillamartin

Copy link
Copy Markdown
Member

For the blur() call in AnnouncementsWidget,

Wondering if it perhaps makes sense to add this to all Tooltip components, or perhaps Tooltip components with buttons inside. Would need to make sure that clicking on the tooltip itself doesn't call blur() but otherwise I think it makes sense to blur after clicking. Since this isn't just a problem with buttons that disappear but even normal ones with a tooltip (eg. clicking down on "show announcements" button but not letting go before moving away doesn't actually "click" but tooltip remains).

@huoxin233

Copy link
Copy Markdown
Contributor Author

Wondering if it perhaps makes sense to add this to all Tooltip components,

I did a test by adding EventListener to listen for mousedown on the child element and schedule a blur on the subsequent mouseup, this will handle the the press and drag away case, but wouldnt this be a discrepancy between the buttons? As shown in the video below, button with tooltip will call blur in the press and drag away case, but normal buttons will still stay focus

PixPin_2026-05-29_23-20-50.mp4
PixPin_2026-05-30_03-45-09.mp4

@dsevillamartin

Copy link
Copy Markdown
Member

Looking further into it, I don't think applying .blur() is the correct strategy since that does more than we want. It could be replaced with a $(this).tooltip('hide'); -- I believe that should do the trick.

However, I just tested the latest 2.x (dfb5ef7) and the behavior is different than what I remember:

Screen.Recording.2026-05-30.105803.mp4

So perhaps we don't need to worry about blurring/hiding as much. I honestly think this might be fine - the other fixes included in this PR are still an improvement. And if we do want to keep it, I think calling hide makes more sense to avoid styling from the blur.

This is what it looks like with this PR's changes (excluding the .blur() in AnnouncementsWidget).

Screen.Recording.2026-05-30.110149.mp4

@huoxin233

Copy link
Copy Markdown
Contributor Author

Looking further into it, I don't think applying .blur() is the correct strategy since that does more than we want。

Sure, I will be reverting the .blur() in AnnouncementsWidget

@imorland imorland added this to the 2.0.0-rc.2 milestone May 30, 2026
@imorland
imorland merged commit bd665e3 into flarum:2.x May 30, 2026
19 checks passed
@huoxin233
huoxin233 deleted the huoxin/2.x-fix-tooltip branch June 21, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[1.x & 2.x] Toggle announcement button tooltip overridden by title

3 participants