Skip to content

Refactored comments to hoist TooltipProvider to the list level#28401

Draft
9larsons wants to merge 2 commits into
mainfrom
ala-comments-tooltip-provider-rebase
Draft

Refactored comments to hoist TooltipProvider to the list level#28401
9larsons wants to merge 2 commits into
mainfrom
ala-comments-tooltip-provider-rebase

Conversation

@9larsons

@9larsons 9larsons commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

What

Rebases the optimization from #25771 onto the current Comments structure. That PR predates the Comments overhaul that split comments-list.tsx into sub-components, so its original diff no longer applies cleanly — but the underlying concern survived the refactor and actually got worse.

Each comment row currently renders its own Radix TooltipProvider instances:

  • CommentHeader — timestamp tooltip + commenting-disabled indicator
  • CommentMetrics — one per Metric (Replies, Likes, Dislikes, Reports)

That's ~4–6 provider contexts per row. With the virtualized list showing ~20 rows, that's 80–120 provider instances, none of which coordinate tooltip timing (skipDelayDuration only applies within a single provider, so hover delays don't share state across them).

Changes

  • Wrap the comments list once in a single TooltipProvider (comments-list.tsx).
  • Remove the per-row providers from comment-header.tsx and comment-metrics.tsx.
  • The thread sidebar reuses CommentHeader/CommentMetrics and renders within the same React subtree (context flows through the Radix Sheet portal via the React tree), so it stays covered by the same provider.

Net: ~80–120 provider contexts → 1, plus consistent hover-delay behavior across a row's tooltips. No UI/behavior change.

Tests

  • Updated comment-header.test.tsx to render within a TooltipProvider, mirroring how the app now supplies it at the list level.
  • All comments unit tests pass; lint clean.

Rebases / supersedes #25771.

Each comment row rendered its own Radix TooltipProvider instances inside
CommentHeader (timestamp + commenting-disabled indicator) and inside every
CommentMetrics Metric (replies, likes, dislikes, reports), giving ~4-6
provider contexts per row. With the virtualized list showing ~20 rows
that's 80-120 provider instances, none of which coordinate tooltip timing
(skipDelayDuration only applies within a single provider).

Wrap the comments list once in a single TooltipProvider and drop the
per-row providers. The thread sidebar reuses CommentHeader/CommentMetrics
and renders within the same React subtree (context flows through the Sheet
portal), so it is covered by the same provider.

Rebases the stale approach from #25771 onto the current component
structure; the original diff predates the comments overhaul that split
comments-list.tsx into sub-components.

ref #25771
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1c012fc0-aeb7-4bc0-b909-7024cb3c54c0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ala-comments-tooltip-provider-rebase

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@9larsons 9larsons changed the title refactor(comments): hoist TooltipProvider to the list level Refactored comments to hoist TooltipProvider to the list level Jun 7, 2026
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.

1 participant