From 7afe7bb7ecf6d7933fbdf925574000226b8bd17e Mon Sep 17 00:00:00 2001 From: Bryant Ung Date: Tue, 9 Jun 2026 22:55:26 -0700 Subject: [PATCH] Fix workspace-creation tour panel clipped by the Create Worktree dialog (#5078) * Fix workspace-creation tour panel clipped by the composer dialog The tour panel portals into dialog/sheet content that clips overflow, but its position was clamped against the window viewport. With the Project field spanning nearly the dialog's full width, the panel landed past the dialog's right edge and overflow-hidden cut it down to a sliver. Clamp hosted panels within the host's bounds instead, so the panel flips below the target and stays fully visible. Co-Authored-By: Claude Fable 5 * Add JSDoc docstrings to satisfy CodeRabbit docstring coverage check Co-Authored-By: Claude Sonnet 4.6 * Test hosted contextual tour overlay positioning --------- Co-authored-by: Claude Fable 5 Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- .../contextual-tour-overlay-position.test.ts | 69 +++++++++++++++++++ .../contextual-tour-overlay-position.ts | 26 ++++--- .../contextual-tour-panel-position.test.ts | 44 +++++++----- .../contextual-tour-panel-position.ts | 27 +++++--- 4 files changed, 130 insertions(+), 36 deletions(-) create mode 100644 src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.test.ts diff --git a/src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.test.ts b/src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.test.ts new file mode 100644 index 000000000..d8b9f2b14 --- /dev/null +++ b/src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest' +import { getContextualTourOverlayPanelPosition } from './contextual-tour-overlay-position' + +function rect( + partial: Pick +): DOMRect { + return partial as DOMRect +} + +function elementWithRect( + bounds: Pick +): HTMLElement { + return { + getBoundingClientRect: () => rect(bounds) + } as HTMLElement +} + +describe('contextual tour overlay position', () => { + it('returns viewport coordinates for floating panels', () => { + const position = getContextualTourOverlayPanelPosition({ + targetRect: rect({ left: 100, right: 200, top: 200, bottom: 240, width: 100, height: 40 }), + panelElement: elementWithRect({ + left: 0, + right: 320, + top: 0, + bottom: 180, + width: 320, + height: 180 + }), + panelHost: null, + viewport: { width: 1024, height: 768 } + }) + + expect(position.panelPlacement).toBe('right') + expect(position.panelPosition.left).toBe(212) + expect(position.panelPosition.top).toBe(130) + expect(position.panelPosition['--contextual-tour-arrow-offset']).toBe('90px') + }) + + it('returns host-local coordinates for panels portaled into clipped dialog content', () => { + const position = getContextualTourOverlayPanelPosition({ + targetRect: rect({ left: 110, right: 1018, top: 240, bottom: 315, width: 908, height: 75 }), + panelElement: elementWithRect({ + left: 0, + right: 320, + top: 0, + bottom: 180, + width: 320, + height: 180 + }), + panelHost: elementWithRect({ + left: 55, + right: 1075, + top: 42, + bottom: 952, + width: 1020, + height: 910 + }), + viewport: { width: 1512, height: 982 } + }) + + expect(position.panelPlacement).toBe('bottom') + expect(position.panelPosition.left).toBe(349) + expect(position.panelPosition.top).toBe(285) + expect(position.panelPosition.left).toBeGreaterThanOrEqual(12) + expect(Number(position.panelPosition.left) + 320).toBeLessThanOrEqual(1020 - 12) + expect(position.panelPosition['--contextual-tour-arrow-offset']).toBe('160px') + }) +}) diff --git a/src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.ts b/src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.ts index 55a11d5e9..1ac20f3b5 100644 --- a/src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.ts +++ b/src/renderer/src/components/contextual-tours/contextual-tour-overlay-position.ts @@ -3,7 +3,7 @@ import type { ContextualTourStepPlacement } from '../../../../shared/contextual- import type { ContextualTourPanelPlacement } from './contextual-tour-panel-position' import { clampContextualTourPanelPosition, - getContextualTourPanelCssPosition + getContextualTourTargetRectInHost } from './contextual-tour-panel-position' const PANEL_FALLBACK_SIZE = { width: 304, height: 172 } @@ -13,6 +13,10 @@ export type ContextualTourOverlayPanelPosition = { panelPlacement: ContextualTourPanelPlacement } +/** + * Returns the CSS position and placement for a tour panel rendered inside an overlay host, + * clamping coordinates to host space so clipped containers don't obscure the panel. + */ export function getContextualTourOverlayPanelPosition(args: { targetRect: DOMRect panelElement: HTMLElement | null @@ -24,22 +28,24 @@ export function getContextualTourOverlayPanelPosition(args: { const panel = panelRect ? { width: panelRect.width, height: panelRect.height } : PANEL_FALLBACK_SIZE + // Why: hosted panels portal into dialog/sheet content whose overflow clips + // them, so position and clamp in host space — viewport clamping can park the + // panel in the clipped region outside the host and leave only a sliver visible. + const hostRect = args.panelHost?.getBoundingClientRect() const clamped = clampContextualTourPanelPosition({ - targetRect: args.targetRect, - viewport: args.viewport, + targetRect: hostRect + ? getContextualTourTargetRectInHost(args.targetRect, hostRect) + : args.targetRect, + viewport: hostRect ? { width: hostRect.width, height: hostRect.height } : args.viewport, panel, preferredPlacement: args.preferredPlacement }) - const cssPosition = getContextualTourPanelCssPosition({ - position: clamped, - panelHostRect: args.panelHost?.getBoundingClientRect() - }) return { panelPlacement: clamped.placement, panelPosition: { - left: cssPosition.left, - top: cssPosition.top, - '--contextual-tour-arrow-offset': `${cssPosition.arrowOffset}px` + left: clamped.left, + top: clamped.top, + '--contextual-tour-arrow-offset': `${clamped.arrowOffset}px` } } } diff --git a/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.test.ts b/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.test.ts index 91938d2d6..6ad3fa36c 100644 --- a/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.test.ts +++ b/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import { clampContextualTourPanelPosition, - getContextualTourPanelCssPosition + getContextualTourTargetRectInHost } from './contextual-tour-panel-position' describe('contextual tour panel position', () => { @@ -40,25 +40,33 @@ describe('contextual tour panel position', () => { expect(position.arrowOffset).toBeLessThan(120) }) - it('converts viewport panel coordinates into hosted dialog coordinates', () => { - const position = { - left: 838, - top: 168, - placement: 'right' as const, - arrowOffset: 64 - } - + it('translates a viewport target rect into hosted dialog coordinates', () => { expect( - getContextualTourPanelCssPosition({ - position, - panelHostRect: { left: 500, top: 80 } - }) - ).toEqual({ left: 338, top: 88, arrowOffset: 64 }) - expect(getContextualTourPanelCssPosition({ position })).toEqual({ - left: 838, - top: 168, - arrowOffset: 64 + getContextualTourTargetRectInHost( + { left: 555, right: 1018, top: 240, bottom: 315, width: 463, height: 75 }, + { left: 500, top: 80 } + ) + ).toEqual({ left: 55, right: 518, top: 160, bottom: 235, width: 463, height: 75 }) + }) + + it('keeps a hosted panel inside a dialog whose field spans nearly its full width', () => { + // Regression: the workspace-creation tour panel was clamped against the + // viewport, so it sat to the right of the Project field — outside the + // dialog content that clips overflow — and only a sliver was visible. + const hostRect = { left: 55, top: 42 } + const position = clampContextualTourPanelPosition({ + targetRect: getContextualTourTargetRectInHost( + { left: 110, right: 1018, top: 240, bottom: 315, width: 908, height: 75 }, + hostRect + ), + viewport: { width: 1020, height: 910 }, + panel: { width: 320, height: 180 } }) + + expect(position.placement).toBe('bottom') + expect(position.left).toBeGreaterThanOrEqual(12) + expect(position.left + 320).toBeLessThanOrEqual(1020 - 12) + expect(position.top + 180).toBeLessThanOrEqual(910 - 12) }) it('flips below the target when neither side has horizontal room', () => { diff --git a/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.ts b/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.ts index 3e9fa45ec..5b50e6630 100644 --- a/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.ts +++ b/src/renderer/src/components/contextual-tours/contextual-tour-panel-position.ts @@ -17,6 +17,10 @@ type PanelSize = { height: number } +/** + * Computes the position and placement for a tour panel relative to a target element, + * choosing the best side and clamping the panel within the viewport. + */ export function clampContextualTourPanelPosition(args: { targetRect: Pick viewport: ViewportSize @@ -90,6 +94,7 @@ export function clampContextualTourPanelPosition(args: { return { left: clampedLeft, top: clampedTop, placement, arrowOffset } } +/** Returns the raw (unclamped) top-left position for a panel at the given placement side. */ function getUnclampedPanelPosition(args: { placement: ContextualTourPanelPlacement targetRect: Pick @@ -121,16 +126,22 @@ function getUnclampedPanelPosition(args: { } } -export function getContextualTourPanelCssPosition(args: { - position: ContextualTourPanelPosition - panelHostRect?: Pick | null -}): Pick { - const { position, panelHostRect } = args - const left = panelHostRect ? position.left - panelHostRect.left : position.left - const top = panelHostRect ? position.top - panelHostRect.top : position.top - return { left, top, arrowOffset: position.arrowOffset } +/** Translates a target rect from viewport coordinates into the host element's local coordinate space. */ +export function getContextualTourTargetRectInHost( + targetRect: Pick, + hostRect: Pick +): Pick { + return { + left: targetRect.left - hostRect.left, + right: targetRect.right - hostRect.left, + top: targetRect.top - hostRect.top, + bottom: targetRect.bottom - hostRect.top, + width: targetRect.width, + height: targetRect.height + } } +/** Clamps a number between min and max, inclusive. */ function clampNumber(value: number, min: number, max: number): number { return Math.min(Math.max(value, min), max) }