regression: tooltips not reappearing on subsequent hovers - #41428
regression: tooltips not reappearing on subsequent hovers#41428dionisio-bot[bot] merged 4 commits into
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughTooltipProvider centralizes anchor-title stashing and restoration, defers restoration after click dismissal until mouseleave, disconnects mutation observers during cleanup, and adds coverage for hover, reopening, dismissal, and dynamic tooltip content updates. ChangesTooltip title lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
could you mention the root cause here? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-8.7.0 #41428 +/- ##
==============================================
Coverage 68.59% 68.59%
==============================================
Files 4134 4134
Lines 160802 160802
Branches 29298 29298
==============================================
Hits 110303 110303
Misses 45392 45392
Partials 5107 5107 🚀 New features to boost your workflow:
|
|
Root cause This was a side effect of the recent React 19 migration (#40796). To show our own styled tooltip, the code temporarily removes the element's built-in Two steps run when you move the mouse away, and they lived in different places:
So the order depended on whether React's cleanup ran before or after that timer. |
abhinavkrin
added
the
stat: QA assured
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
abhinavkrin
added
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui-client/src/providers/TooltipProvider.tsx`:
- Around line 13-23: Update stashAnchorTitle and restoreAnchorTitle to stash and
restore the title attribute only when the anchor originally has one; do not add
a title="" attribute for data-tooltip-only elements, and ensure cleanup leaves
those elements without title while preserving existing title behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67f02f08-52a4-47d6-9d48-33a1b66544d9
📒 Files selected for processing (2)
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
⚠️ CI failures not shown inline (4)
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
🧠 Learnings (3)
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
packages/ui-client/src/providers/TooltipProvider.spec.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui-client/src/providers/TooltipProvider.spec.tsx (1)
185-185: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd click-dismiss test coverage for
data-tooltipanchors.To prevent regressions, consider verifying that tooltips using only
data-tooltipcan be successfully reopened after a click-dismiss. This ensures that the internal anchor tracking properly resets when the cursor leaves.🧪 Proposed test case
expect(screen.getByRole('tooltip', { hidden: true })).toHaveTextContent('World'); expect(anchor).not.toHaveAttribute('title'); }); + + it('should show the tooltip again after a click-dismiss and mouseleave cycle', () => { + const { anchor } = setupDataTooltip(); + + fireEvent.mouseOver(anchor); + waitForTooltipDebounce(); + fireEvent.click(anchor); + fireEvent.mouseLeave(anchor); + act(() => { + jest.runOnlyPendingTimers(); + }); + + fireEvent.mouseOver(anchor); + waitForTooltipDebounce(); + + expect(screen.getByRole('tooltip', { hidden: true })).toHaveTextContent('Hello'); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui-client/src/providers/TooltipProvider.spec.tsx` at line 185, Add a test in the TooltipProvider suite covering a tooltip anchor that uses only data-tooltip: open it, dismiss it by clicking, move the cursor away, and verify it can be reopened. Reuse the existing tooltip rendering and interaction helpers, and assert the tooltip visibility after reopening to validate anchor tracking resets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui-client/src/providers/TooltipProvider.tsx`:
- Around line 54-66: Update the cleanup block in the dismiss handler to attach
the one-time mouseleave listener whenever an anchor is present, without
requiring anchor.hasAttribute('data-title'). Keep restoreAnchorTitle(anchor) and
the dismissedAnchor.current/lastAnchor.current cleanup intact, since
restoreAnchorTitle safely handles anchors that only use data-tooltip.
---
Nitpick comments:
In `@packages/ui-client/src/providers/TooltipProvider.spec.tsx`:
- Line 185: Add a test in the TooltipProvider suite covering a tooltip anchor
that uses only data-tooltip: open it, dismiss it by clicking, move the cursor
away, and verify it can be reopened. Reuse the existing tooltip rendering and
interaction helpers, and assert the tooltip visibility after reopening to
validate anchor tracking resets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 14eb6bfe-f308-4c36-a309-87d73fd45c17
📒 Files selected for processing (2)
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
⚠️ CI failures not shown inline (4)
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
GitHub Check: Dionisio QA: Some checks did not pass
Conclusion: failure
**Conclusion:** failure
### Steps
- ✅ **No merge conflicts**
- ✅ **QA assured**
- ✅ **Mergeable**
- ✅ **Has milestone or project**
- ✅ **Valid PR title**
- ✅ **Correct target version**
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
🧠 Learnings (3)
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
packages/ui-client/src/providers/TooltipProvider.spec.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/ui-client/src/providers/TooltipProvider.spec.tsxpackages/ui-client/src/providers/TooltipProvider.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
abhinavkrin
removed
the
stat: QA assured
dionisio-bot
Bot
removed
the
stat: ready to merge
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
The effect cleanup now restores the anchor title synchronously on unmount, so the setTimeout-based restores in open/close are dead code — their only observable effect was re-triggering the MutationObserver while the previous tooltip was still mounted. Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
5f31b3d to
475e817
Compare
ricardogarim
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
|
/jira CORE-2410 |
4 participants
Proposed changes (including videos or screenshots)
Tooltips (custom emojis, user avatars in messages, header buttons, etc.) only showed the first time an element was hovered. On any subsequent hover of the same element, the tooltip failed to appear.
TooltipProvidersuppresses the browser's native tooltip by moving an element'stitleinto adata-titleattribute while its own tooltip is shown, and restores it on close. The restore was deferred viasetTimeout(0), while aMutationObserverguarding the title was disconnected in the tooltip's effect cleanup. This relied on the effect cleanup running before the deferred restore — an ordering that held under React 18 but flipped with the React 19 upgrade (root scheduling now defers to a microtask). As a result, the still-connected observer re-blanked the title after it was restored, leaving the element with a permanently emptytitleand no tooltip on re-hover.The fix restores the title synchronously inside the effect cleanup, immediately after the observer is disconnected, removing the reliance on scheduler timing. The stash/restore logic was also extracted into small helpers, and a regression test was added.
Issue(s)
Closes: CORE-2410
Steps to test or reproduce
Further comments
CORE-2410
Summary by CodeRabbit
title, with reliable stashing/restoring across hover close-and-reopen and click-dismiss flows.title(ordata-tooltip) changes while the tooltip is open.Task: [CORE-2463]