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} /> -
+