From 035928e0a0df08a137e6325fcbb868cf881dcf6f Mon Sep 17 00:00:00 2001 From: batuboy31 Date: Wed, 4 Nov 2026 20:22:59 +0300 Subject: [PATCH] Keep panel close buttons clear of the window controls on Windows On Windows and Linux the window controls sit over the top-right corner of whichever part reaches it, but the canvas never learned their width: a guided tour's close button, fullscreen or in the right column, sat under them, and so did the close buttons of side peeks and Ask in that column. The workbench now writes --review-chrome-right-width on the part at the window's top-right corner, an editor group or the Ask column, only when it changes. Panel headers read it into their right padding: the right column's header and a fullscreen tour's panel set the inset, so nothing moves on macOS or where no controls cover the canvas. Fixes #1490 Co-Authored-By: Claude Opus 5.5 --- .../src/vs/review/browser/workbench.ts | 40 ++++++ .../review/app/src/blocks.browser.test.tsx | 115 ++++++++++++++++++ packages/review/app/src/diagram-tour.tsx | 18 ++- packages/review/app/src/side-panel-styles.ts | 6 +- 4 files changed, 176 insertions(+), 3 deletions(-) diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/workbench.ts b/apps/review-desktop/code-oss/src/vs/review/browser/workbench.ts index f16813b83..4c2bfe8af 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/workbench.ts +++ b/apps/review-desktop/code-oss/src/vs/review/browser/workbench.ts @@ -317,6 +317,8 @@ export class ReviewWorkbench extends Disposable implements IAgentWorkbenchLayout protected _editorPartAutoVisibilitySuppressionCount = 0; protected _hasAppliedInitialEditorSplit = false; private readonly reviewChromeTabStrips: HTMLElement[] = []; + private reviewChromeRightPart: HTMLElement | undefined; + private reviewChromeRightWidth: string | undefined; private readonly reviewChromeDragListeners = this._register(new DisposableStore()); private readonly reviewChromeInsetScheduler = this._register(new RunOnceScheduler(() => this.updateReviewChromeInset(), 0)); @@ -996,6 +998,9 @@ export class ReviewWorkbench extends Disposable implements IAgentWorkbenchLayout if (titlebarPart instanceof MainReviewTitlebarPart) { this._register(titlebarPart.onDidChangeChromeInsets(() => this.reviewChromeInsetScheduler.schedule())); } + // The Ask column opening or closing moves the window's top-right corner + // to another part. + this._register(this.onDidChangePartVisibility(() => this.reviewChromeInsetScheduler.schedule())); this._register(editorMainPart.onDidAddGroup(() => this.reviewChromeInsetScheduler.schedule())); this._register(editorMainPart.onDidRemoveGroup(() => this.reviewChromeInsetScheduler.schedule())); this._register(editorMainPart.onDidMoveGroup(() => this.reviewChromeInsetScheduler.schedule())); @@ -1207,7 +1212,42 @@ export class ReviewWorkbench extends Disposable implements IAgentWorkbenchLayout tabStrip.style.setProperty('--review-explorer-width', `${explorerWidth}px`); } + /** + * With tabs hidden, whichever part reaches the window's top-right corner, + * a top-row editor group or the Ask column, runs under the window + * controls Windows and Linux draw there. Its canvas reads their width so + * a header's close button clears them. Written on that part, and only + * when it changes, for the reason above. + */ + private updateReviewChromeRightInset(): void { + const bounds = this.mainContainer.getBoundingClientRect(); + const candidates = [ + ...Array.from(this._editorPartContainer?.getElementsByClassName('editor-group-container') ?? []), + ...Array.from(this.mainContainer.getElementsByClassName('review-ask-part')), + ]; + const part = candidates.find((candidate): candidate is HTMLElement => { + if (!isHTMLElement(candidate)) { + return false; + } + const rect = candidate.getBoundingClientRect(); + return rect.width > 0 && Math.abs(rect.top - bounds.top) <= 1 && Math.abs(rect.right - bounds.right) <= 1; + }); + const titlebarPart = this.getPart(Parts.TITLEBAR_PART); + const width = part && titlebarPart instanceof MainReviewTitlebarPart ? `${titlebarPart.chromeInsets.right}px` : undefined; + if (part === this.reviewChromeRightPart && width === this.reviewChromeRightWidth) { + return; + } + this.reviewChromeRightPart?.style.removeProperty('--review-chrome-right-width'); + this.reviewChromeRightPart = part; + this.reviewChromeRightWidth = width; + if (part && width) { + part.style.setProperty('--review-chrome-right-width', width); + } + } + private updateReviewChromeInset(): void { + this.updateReviewChromeRightInset(); + this.reviewChromeDragListeners.clear(); this.mainContainer.classList.remove('review-tab-dragging'); for (const tabStrip of this.reviewChromeTabStrips) { diff --git a/packages/review/app/src/blocks.browser.test.tsx b/packages/review/app/src/blocks.browser.test.tsx index a3770f30b..9c37c3924 100644 --- a/packages/review/app/src/blocks.browser.test.tsx +++ b/packages/review/app/src/blocks.browser.test.tsx @@ -2159,6 +2159,121 @@ it("opens a flow node in a full-screen tour with all its code attachments", asyn expect(container.querySelector(".lens-flow-node")).not.toBeNull(); }); +it("keeps a tour's close button in Desktop's right column clear of window controls", async () => { + // A step picked on the diagram opens the tour in Desktop's right column, + // which runs under the window controls Windows and Linux draw at the + // window's top-right corner. The host publishes their width on the column + // as --review-chrome-right-width. + const controls = document.createElement("div"); + const column = document.createElement("div"); + + controls.style.cssText = + "position:fixed;top:0;right:0;width:138px;height:40px;z-index:2147483647"; + column.style.cssText = + "position:fixed;top:0;right:0;bottom:0;width:480px;display:flex;flex-direction:column;--review-chrome-right-width:138px"; + document.body.append(column, controls); + + try { + const { container } = await mountFixture( + "sequence", + { + diagramFullscreenOnClick: false, + host: { askPanelHost: () => column }, + }, + "position:fixed;top:0;bottom:0;left:0;right:480px", + ); + + const step = await settled(() => + container.querySelector( + "button[data-review-anchor-id='step-1']", + ), + ); + + await act(async () => step!.click()); + + const close = await settled(() => + document.querySelector( + 'button[aria-label="Close guided tour"]', + ), + ); + + expect(column.contains(close)).toBe(true); + // The reader clicks once the panel has slid in. + await Promise.allSettled( + column + .getAnimations({ subtree: true }) + .map((animation) => animation.finished), + ); + const box = close!.getBoundingClientRect(); + + const hit = document.elementFromPoint( + box.left + box.width / 2, + box.top + box.height / 2, + ); + + expect(close!.contains(hit)).toBe(true); + await act(async () => close!.click()); + expect( + document.querySelector('button[aria-label="Close guided tour"]'), + ).toBeNull(); + } finally { + controls.remove(); + column.remove(); + } +}); + +it("keeps a full-screen tour's close button clear of window controls on the right", async () => { + // Windows and Linux draw the window controls over the canvas's top-right + // corner, above everything the canvas renders. The host publishes their + // width as --review-chrome-right-width. + const controls = document.createElement("div"); + + controls.style.cssText = + "position:fixed;top:0;right:0;width:138px;height:35px;z-index:2147483647"; + document.body.append(controls); + + try { + const { container } = await mountFixture( + "sequence", + {}, + "position:fixed;inset:0;--review-chrome-right-width:138px", + ); + + const expand = await settled(() => + container.querySelector(".diagram-tour-button"), + ); + + await act(async () => expand!.click()); + + const close = await settled(() => + container.querySelector( + '[role="dialog"] button[aria-label="Close guided tour"]', + ), + ); + + expect(close).toBeTruthy(); + // The reader clicks once the panel has slid in. + await Promise.allSettled( + close! + .closest('[role="dialog"]')! + .getAnimations({ subtree: true }) + .map((animation) => animation.finished), + ); + const box = close!.getBoundingClientRect(); + + const hit = document.elementFromPoint( + box.left + box.width / 2, + box.top + box.height / 2, + ); + + expect(close!.contains(hit)).toBe(true); + await act(async () => close!.click()); + expect(container.querySelector('[role="dialog"]')).toBeNull(); + } finally { + controls.remove(); + } +}); + it.each([ { kind: "database_lens" as const, diff --git a/packages/review/app/src/diagram-tour.tsx b/packages/review/app/src/diagram-tour.tsx index adfca6b8d..9e17ef795 100644 --- a/packages/review/app/src/diagram-tour.tsx +++ b/packages/review/app/src/diagram-tour.tsx @@ -100,7 +100,12 @@ export function DiagramTourOverlay({ )} {...paneResize.separatorProps} /> -
+