fix: 悬停始终弹出描述tooltip，趋势页新增tooltip#170
Merged
Merged
Conversation
…s page Remove truncation detection threshold in RepositoryCard — the scrollHeight > clientHeight check was unreliable across different fonts/browsers, causing tooltips to not show when text appeared visually truncated. Now tooltips always show on hover. Add hover tooltip to SubscriptionRepoCard (trends page) for both description and AI summary sections, which previously had no way to view truncated text. Closes #165 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a reusable FloatingTooltip (portal, placement/flip, resize/scroll updates) and rewires RepositoryCard and SubscriptionRepoCard to show full-text tooltips via explicit boolean state and trigger refs on hover/focus/touch; RepositoryCard simplifies timeout cleanup to unmount-only clearing. ChangesTooltip Hover Enhancement
Sequence Diagram(s)sequenceDiagram
participant User
participant RepositoryCard
participant SubscriptionRepoCard
participant FloatingTooltip
participant Window
User->>RepositoryCard: mouseenter / focus / touchstart (description)
RepositoryCard->>FloatingTooltip: render visible tooltip anchored to descTriggerRef
FloatingTooltip->>Window: measure trigger bounds and viewport
FloatingTooltip->>FloatingTooltip: compute placement / flip if needed
FloatingTooltip->>User: show positioned tooltip
User->>SubscriptionRepoCard: mouseenter / focus / touchstart (description or ai_summary)
SubscriptionRepoCard->>FloatingTooltip: render visible tooltip anchored to corresponding triggerRef
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/components/RepositoryCard.tsx`:
- Around line 866-879: The tooltip is currently only toggled by mouse events;
update the container (the element using setShowTooltip and showTooltip in
RepositoryCard, and the paragraph rendering highlightSearchTerm) to support
keyboard and touch by adding onFocus and onBlur handlers that
setShowTooltip(true/false), a tabIndex={0} so it is focusable, and touch/click
handlers (onClick and/or onTouchStart) that toggle or open the tooltip for touch
users; also add appropriate ARIA attributes (e.g., aria-expanded={showTooltip}
and aria-describedby or role="button"/role="group" as appropriate) so screen
readers can detect the tooltip state.
In `@src/components/SubscriptionRepoCard.tsx`:
- Around line 408-424: The tooltip currently controlled only by
onMouseEnter/onMouseLeave (descTooltip and setDescTooltip) is inaccessible to
keyboard and touch users; update the container(s) that show repo.description
(the div wrapping the <p> and the similar block at lines 429-445) to be
focusable (add tabIndex={0}), handle onFocus/onBlur to setDescTooltip and add an
onClick or onKeyDown handler (handling Enter/Space) to toggle the tooltip for
touch/keyboard activation, and ensure appropriate ARIA attributes (e.g.,
role="button" and aria-expanded={descTooltip}) are set so screen readers and
keyboard users can open the full-text panels as well.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0618bf99-2c22-4707-8f76-1d84fe4a55f8
📒 Files selected for processing (2)
src/components/RepositoryCard.tsxsrc/components/SubscriptionRepoCard.tsx
Add onFocus/onBlur/onTouchStart handlers and tabIndex={0} to all tooltip
containers so keyboard and touch users can also access full descriptions.
Addresses CodeRabbitAI review findings on PR #170.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace absolute-positioned tooltips with FloatingTooltip component that uses createPortal to render outside overflow containers. The tooltip uses fixed positioning calculated from getBoundingClientRect(), so it's never clipped by parent overflow-y-auto scroll containers. This fixes the issue where trend page tooltips were being cut off by the DiscoveryView scroll container boundary. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/FloatingTooltip.tsx (2)
53-58: 💤 Low valueOptional: throttle
scroll/resizerepositioning.handleResizerunsgetBoundingClientRect()+ style writes synchronously on every capture-phase scroll/resize event, which can be a hot path in theDiscoveryViewscroll container. Consider coalescing withrequestAnimationFrame.🤖 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 `@src/components/FloatingTooltip.tsx` around lines 53 - 58, The scroll/resize handler (handleResize -> updatePosition) runs expensive layout reads/writes on every event; wrap/coalesce calls via requestAnimationFrame to avoid synchronous work on every capture-phase scroll/resize. Modify where window.addEventListener('resize', handleResize) and window.addEventListener('scroll', handleResize, true) are registered so handleResize schedules a single rAF-driven runner (cancel previous rAF if pending) that calls updatePosition once per animation frame, and ensure the cleanup removes both the event listeners and cancels any pending rAF handle; reference the handleResize function, updatePosition call, and the addEventListener/removeEventListener calls when making this change.
19-48: ⚡ Quick winReposition runs in
useEffect, causing a first-frame flash.useEffectfires after paint, so on first show the fixed tooltip is briefly painted at its default location (top-left of the viewport) beforeupdatePositionmoves it — visible because ofanimate-fade-in. UseuseLayoutEffectso positioning happens before the browser paints.♻️ Proposed change
-import React, { useEffect, useRef, useCallback } from 'react'; +import React, { useLayoutEffect, useRef, useCallback } from 'react';- useEffect(() => { + useLayoutEffect(() => { if (visible) { updatePosition();🤖 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 `@src/components/FloatingTooltip.tsx` around lines 19 - 48, The repositioning is running in useEffect which causes a first-frame flash; change the effect that calls updatePosition to useLayoutEffect so positioning runs before paint. Locate the effect that invokes updatePosition (references updatePosition, triggerRef, tooltipRef, visible) and replace useEffect with useLayoutEffect; ensure updatePosition's dependency array includes tooltipRef.current (or tooltipRef) and any window listeners (resize/scroll) are also registered/cleaned up inside that useLayoutEffect so the tooltip (and its animate-fade-in) is positioned before the first paint.
🤖 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 `@src/components/FloatingTooltip.tsx`:
- Around line 50-61: The tooltip immediately unmounts when the trigger's
onMouseLeave fires, preventing hovering/scrolling the tooltip; change
FloatingTooltip to implement a hover-intent hide delay: keep the existing
visible/updatePosition logic but add a hideTimeoutRef (or similar) that starts a
short timeout on the trigger's onMouseLeave instead of immediately setting
visible=false, expose an onMouseEnter handler on the tooltip DOM that clears
that timeout (and also clear the timeout on tooltip onMouseLeave to restart it),
ensure cleanup of the timeout in useEffect cleanup and when visible changes, and
update consumers (SubscriptionRepoCard/RepositoryCard) to stop toggling
visibility synchronously and rely on this delayed hide behavior.
---
Nitpick comments:
In `@src/components/FloatingTooltip.tsx`:
- Around line 53-58: The scroll/resize handler (handleResize -> updatePosition)
runs expensive layout reads/writes on every event; wrap/coalesce calls via
requestAnimationFrame to avoid synchronous work on every capture-phase
scroll/resize. Modify where window.addEventListener('resize', handleResize) and
window.addEventListener('scroll', handleResize, true) are registered so
handleResize schedules a single rAF-driven runner (cancel previous rAF if
pending) that calls updatePosition once per animation frame, and ensure the
cleanup removes both the event listeners and cancels any pending rAF handle;
reference the handleResize function, updatePosition call, and the
addEventListener/removeEventListener calls when making this change.
- Around line 19-48: The repositioning is running in useEffect which causes a
first-frame flash; change the effect that calls updatePosition to
useLayoutEffect so positioning runs before paint. Locate the effect that invokes
updatePosition (references updatePosition, triggerRef, tooltipRef, visible) and
replace useEffect with useLayoutEffect; ensure updatePosition's dependency array
includes tooltipRef.current (or tooltipRef) and any window listeners
(resize/scroll) are also registered/cleaned up inside that useLayoutEffect so
the tooltip (and its animate-fade-in) is positioned before the first paint.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: de63050c-27eb-429c-b511-7773f407fc93
📒 Files selected for processing (3)
src/components/FloatingTooltip.tsxsrc/components/RepositoryCard.tsxsrc/components/SubscriptionRepoCard.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/RepositoryCard.tsx
Comment on lines
+50
to
+61
| useEffect(() => { | ||
| if (visible) { | ||
| updatePosition(); | ||
| const handleResize = () => updatePosition(); | ||
| window.addEventListener('resize', handleResize); | ||
| window.addEventListener('scroll', handleResize, true); | ||
| return () => { | ||
| window.removeEventListener('resize', handleResize); | ||
| window.removeEventListener('scroll', handleResize, true); | ||
| }; | ||
| } | ||
| }, [visible, updatePosition]); |
Contributor
There was a problem hiding this comment.
Tooltip closes before the user can reach it, breaking scrollable long content. The tooltip is rendered 8px above the trigger and visibility is driven entirely by the trigger wrapper's onMouseLeave (see SubscriptionRepoCard lines 415/439 and RepositoryCard). When the cursor leaves the trigger to move onto the tooltip, onMouseLeave fires immediately and unmounts it — so a user can never hover the panel to scroll, which the PR test plan ("长文本 tooltip 内可滚动") requires. Adding onMouseEnter here alone won't fix it because the trigger has already toggled visible to false.
Recommend a hover-intent pattern: delay the hide, expose onMouseEnter on the tooltip to cancel the pending hide, and have consumers clear the timer on re-entry.
🛠️ Suggested pattern
interface FloatingTooltipProps {
content: React.ReactNode;
visible: boolean;
triggerRef: React.RefObject<HTMLElement | null>;
+ onMouseEnter?: () => void;
onMouseLeave: () => void;
} <div
ref={tooltipRef}
+ onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}Consumers should hide on a short timeout (cleared by the tooltip's onMouseEnter) instead of synchronously on the wrapper's onMouseLeave.
🤖 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 `@src/components/FloatingTooltip.tsx` around lines 50 - 61, The tooltip
immediately unmounts when the trigger's onMouseLeave fires, preventing
hovering/scrolling the tooltip; change FloatingTooltip to implement a
hover-intent hide delay: keep the existing visible/updatePosition logic but add
a hideTimeoutRef (or similar) that starts a short timeout on the trigger's
onMouseLeave instead of immediately setting visible=false, expose an
onMouseEnter handler on the tooltip DOM that clears that timeout (and also clear
the timeout on tooltip onMouseLeave to restart it), ensure cleanup of the
timeout in useEffect cleanup and when visible changes, and update consumers
(SubscriptionRepoCard/RepositoryCard) to stop toggling visibility synchronously
and rely on this delayed hide behavior.
Three fixes addressing CodeRabbitAI audit findings: 1. Hover-intent pattern: tooltip hide is delayed 150ms, cancelled on re-entry (both trigger and tooltip onMouseEnter). Users can now move cursor from trigger to tooltip and scroll long content. 2. useLayoutEffect instead of useEffect for positioning: prevents first-frame flash where tooltip briefly appears at default position. 3. requestAnimationFrame throttling for scroll/resize: coalesces rapid reposition events into one per animation frame. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lostiv
pushed a commit
to lostiv/GithubStarsManager
that referenced
this pull request
May 30, 2026
Add onFocus/onBlur/onTouchStart handlers and tabIndex={0} to all tooltip
containers so keyboard and touch users can also access full descriptions.
Addresses CodeRabbitAI review findings on PR AmintaCCCP#170.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
(cherry picked from commit a1bacb8)
This was referenced Jun 1, 2026
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scrollHeight > clientHeight），悬停时始终弹出 tooltip，解决因字体渲染差异导致检测不准的问题Closes #165
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes