Repository navigation
Conversation
|
Warning Review limit reachedOnly developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing. Next included review available in 1 minute. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Comment |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a localized tooltip-positioning fix with safe zero-height behavior outside overlay-enabled desktop windows. It changes no APIs, data, security-sensitive code, deployment configuration, or static-analysis settings. Notes:
You can add or adjust custom eligibility rules. Learn more. |
|
Note: GPT-6 on behalf of shivam (@shivamhwp). This conflicts with current main in |
Dismissing prior approval to re-evaluate 1f5afaa
A tooltip that flipped up out of the diff panel toolbar landed inside the titlebar strip, where the OS composites the minimize/maximize/close buttons above every DOM layer. No z-index can win that, so reserve the strip as collision padding and let the tooltip flip down instead.
1f5afaa to
4065116
Compare
|
Note Written by Hi! We are cleaning up open PRs, and this one appears to have been created with an older model ( |
|
Note Written by Reopening, this was closed by mistake. Sorry for the noise! |
Problem
In the desktop app, hovering the line-wrapping toggle in the diff panel toolbar showed its tooltip inside the titlebar strip, where the OS composites the minimize, maximize, and close buttons above every DOM layer. The tooltip was hidden behind the native controls. Raising
z-indexcannot fix it, because those controls are not part of the page (#8882).Change
The shared
TooltipPopuppositioner reserves the window-controls-overlay height as top collision padding, so a tooltip that would land in that strip flips below its trigger instead. The height comes fromnavigator.windowControlsOverlay.getTitlebarAreaRect()through a newuseWindowControlsOverlayHeighthook that tracksgeometrychange. It is 0 in a regular browser, so web placement is unchanged. Fixing it in the shared tooltip covers every tooltip near the top of the window, not just the one in the report.Scope and approval
Small, focused fix for an obvious bug: a tooltip rendered under native window controls and could not be read. #8882 is not yet triaged; a maintainer confirmed the approach still addresses the reported placement and asked only for the rebase: #8893 (comment)
Verification
side="top"at y 9.5, inside the strip; after it reportedside="bottom"at y 71.5.windowControlsOverlay.tsby keepinggetElectronPlatformClassNamesprivate (from refactor(web): keep app utilities private and remove dead helpers #10227) with the new overlay-height hook above it.tsc --noEmitreported no errors;vp lintandvp fmt --checkon both changed files are clean.Fixes #8882
Built with Claude Opus 5 in T3 Code. Rebase and body update by Claude Opus 5.5 via Claude Code.