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 <noreply@anthropic.com> * Add JSDoc docstrings to satisfy CodeRabbit docstring coverage check Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Test hosted contextual tour overlay positioning --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
parent
41adf5f15b
commit
7afe7bb7ec
|
|
@ -0,0 +1,69 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { getContextualTourOverlayPanelPosition } from './contextual-tour-overlay-position'
|
||||
|
||||
function rect(
|
||||
partial: Pick<DOMRect, 'left' | 'right' | 'top' | 'bottom' | 'width' | 'height'>
|
||||
): DOMRect {
|
||||
return partial as DOMRect
|
||||
}
|
||||
|
||||
function elementWithRect(
|
||||
bounds: Pick<DOMRect, 'left' | 'right' | 'top' | 'bottom' | 'width' | 'height'>
|
||||
): 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')
|
||||
})
|
||||
})
|
||||
|
|
@ -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`
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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<DOMRect, 'left' | 'right' | 'top' | 'bottom' | 'width' | 'height'>
|
||||
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<DOMRect, 'left' | 'right' | 'top' | 'bottom' | 'width' | 'height'>
|
||||
|
|
@ -121,16 +126,22 @@ function getUnclampedPanelPosition(args: {
|
|||
}
|
||||
}
|
||||
|
||||
export function getContextualTourPanelCssPosition(args: {
|
||||
position: ContextualTourPanelPosition
|
||||
panelHostRect?: Pick<DOMRect, 'left' | 'top'> | null
|
||||
}): Pick<ContextualTourPanelPosition, 'left' | 'top' | 'arrowOffset'> {
|
||||
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<DOMRect, 'left' | 'right' | 'top' | 'bottom' | 'width' | 'height'>,
|
||||
hostRect: Pick<DOMRect, 'left' | 'top'>
|
||||
): Pick<DOMRect, 'left' | 'right' | 'top' | 'bottom' | 'width' | 'height'> {
|
||||
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)
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue