diff --git a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 4a1e346cf..c7e34918f 100644 --- a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -787,6 +787,7 @@ function resolveOptions(opts: ScrollableElementCreationOptions): ScrollableEleme verticalScrollbarSize: (typeof opts.verticalScrollbarSize !== 'undefined' ? opts.verticalScrollbarSize : globalDefaultScrollbarSize), verticalHasArrows: (typeof opts.verticalHasArrows !== 'undefined' ? opts.verticalHasArrows : false), verticalSliderSize: (typeof opts.verticalSliderSize !== 'undefined' ? opts.verticalSliderSize : 0), + verticalScrollbarTopInset: (typeof opts.verticalScrollbarTopInset !== 'undefined' ? opts.verticalScrollbarTopInset : 0), scrollByPage: (typeof opts.scrollByPage !== 'undefined' ? opts.scrollByPage : false) }; diff --git a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts index 16f1fe3a7..08f300594 100644 --- a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts +++ b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts @@ -123,6 +123,11 @@ export interface ScrollableElementCreationOptions { * Defaults to false. */ verticalHasArrows?: boolean; + /** + * Space (in px) left above the vertical scrollbar, e.g. for a pinned header. + * Defaults to 0. + */ + verticalScrollbarTopInset?: number; /** * Scroll gutter clicks move by page vs. jump to position. * Defaults to false. @@ -166,5 +171,6 @@ export interface ScrollableElementResolvedOptions { verticalScrollbarSize: number; verticalSliderSize: number; verticalHasArrows: boolean; + verticalScrollbarTopInset: number; scrollByPage: boolean; } diff --git a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollbarState.ts b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollbarState.ts index 9a4f4e2cb..86c62ba34 100644 --- a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollbarState.ts +++ b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/scrollbarState.ts @@ -28,6 +28,11 @@ export class ScrollbarState { */ private readonly _arrowSize: number; + /** + * Space before the track, which starts this far into the scrollable area. + */ + private readonly _leadingInset: number; + // --- variables /** * For the vertical scrollbar: the viewport height. @@ -62,7 +67,8 @@ export class ScrollbarState { private _computedSliderRatio: number; private _computedSliderPosition: number; - constructor(arrowSize: number, scrollbarSize: number, oppositeScrollbarSize: number, visibleSize: number, scrollSize: number, scrollPosition: number) { + constructor(arrowSize: number, scrollbarSize: number, oppositeScrollbarSize: number, visibleSize: number, scrollSize: number, scrollPosition: number, leadingInset: number = 0) { + this._leadingInset = Math.round(leadingInset); this._scrollbarSize = Math.round(scrollbarSize); this._oppositeScrollbarSize = Math.round(oppositeScrollbarSize); this._arrowSize = Math.round(arrowSize); @@ -81,7 +87,7 @@ export class ScrollbarState { } public clone(): ScrollbarState { - return new ScrollbarState(this._arrowSize, this._scrollbarSize, this._oppositeScrollbarSize, this._visibleSize, this._scrollSize, this._scrollPosition); + return new ScrollbarState(this._arrowSize, this._scrollbarSize, this._oppositeScrollbarSize, this._visibleSize, this._scrollSize, this._scrollPosition, this._leadingInset); } public setVisibleSize(visibleSize: number): boolean { @@ -122,8 +128,8 @@ export class ScrollbarState { this._oppositeScrollbarSize = Math.round(oppositeScrollbarSize); } - private static _computeValues(oppositeScrollbarSize: number, arrowSize: number, visibleSize: number, scrollSize: number, scrollPosition: number) { - const computedAvailableSize = Math.max(0, visibleSize - oppositeScrollbarSize); + private static _computeValues(oppositeScrollbarSize: number, arrowSize: number, visibleSize: number, scrollSize: number, scrollPosition: number, leadingInset: number) { + const computedAvailableSize = Math.max(0, visibleSize - oppositeScrollbarSize - leadingInset); const computedRepresentableSize = Math.max(0, computedAvailableSize - 2 * arrowSize); const computedIsNeeded = (scrollSize > 0 && scrollSize > visibleSize); @@ -156,7 +162,7 @@ export class ScrollbarState { } private _refreshComputedValues(): void { - const r = ScrollbarState._computeValues(this._oppositeScrollbarSize, this._arrowSize, this._visibleSize, this._scrollSize, this._scrollPosition); + const r = ScrollbarState._computeValues(this._oppositeScrollbarSize, this._arrowSize, this._visibleSize, this._scrollSize, this._scrollPosition, this._leadingInset); this._computedAvailableSize = r.computedAvailableSize; this._computedIsNeeded = r.computedIsNeeded; this._computedSliderSize = r.computedSliderSize; @@ -164,6 +170,10 @@ export class ScrollbarState { this._computedSliderPosition = r.computedSliderPosition; } + public getLeadingInset(): number { + return this._leadingInset; + } + public getArrowSize(): number { return this._arrowSize; } diff --git a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts index 5617e1692..0105bafc1 100644 --- a/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts +++ b/apps/review-desktop/code-oss/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts @@ -28,7 +28,8 @@ export class VerticalScrollbar extends AbstractScrollbar { 0, scrollDimensions.height, scrollDimensions.scrollHeight, - scrollPosition.scrollTop + scrollPosition.scrollTop, + options.verticalScrollbarTopInset ), visibility: options.vertical, extraScrollbarClassName: 'vertical', @@ -77,7 +78,7 @@ export class VerticalScrollbar extends AbstractScrollbar { this.domNode.setWidth(smallSize); this.domNode.setHeight(largeSize); this.domNode.setRight(0); - this.domNode.setTop(0); + this.domNode.setTop(this._scrollbarState.getLeadingInset()); } public onDidScroll(e: ScrollEvent): boolean { diff --git a/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts b/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts index d6fb7167c..d488fbf36 100644 --- a/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts +++ b/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.ts @@ -27,6 +27,7 @@ import { ICodeEditor } from '../../editorBrowser.js'; import { ObservableElementSizeObserver } from '../diffEditor/utils.js'; import { DiffEditorItemTemplate, TemplateData } from './diffEditorItemTemplate.js'; import { IDocumentDiffItem } from './model.js'; +import { MULTI_DIFF_RESOURCE_HEADER_HEIGHT } from './multiDiffEditorResourceHeader.js'; import { DocumentDiffItemViewModel, MultiDiffEditorViewModel } from './multiDiffEditorViewModel.js'; import { RevealOptions } from './multiDiffEditorWidget.js'; import { ObjectPool } from './objectPool.js'; @@ -102,11 +103,14 @@ export class MultiDiffEditorWidgetImpl extends Disposable { smoothScrollDuration: 100, })); this._scrollableElement = this._register(new SmoothScrollableElement(this._scrollableElements.root, { - vertical: ScrollbarVisibility.Auto, + vertical: this._workbenchUIElementFactory.alwaysShowScrollbars ? ScrollbarVisibility.Visible : ScrollbarVisibility.Auto, horizontal: this._workbenchUIElementFactory.horizontalScrollbar === 'hidden' ? ScrollbarVisibility.Hidden - : ScrollbarVisibility.Auto, + : this._workbenchUIElementFactory.alwaysShowScrollbars ? ScrollbarVisibility.Visible : ScrollbarVisibility.Auto, useShadows: false, + verticalScrollbarTopInset: this._workbenchUIElementFactory.scrollbarBelowResourceHeader && !this._workbenchUIElementFactory.hideResourceHeader + ? MULTI_DIFF_RESOURCE_HEADER_HEIGHT + : 0, }, this._scrollable)); this._elements = h('div.monaco-component.multiDiffEditor', {}, [ h('div', {}, [this._scrollableElement.getDomNode()]), diff --git a/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/workbenchUIElementFactory.ts b/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/workbenchUIElementFactory.ts index 15555e5af..382232239 100644 --- a/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/workbenchUIElementFactory.ts +++ b/apps/review-desktop/code-oss/src/vs/editor/browser/widget/multiDiffEditor/workbenchUIElementFactory.ts @@ -22,6 +22,10 @@ export interface IWorkbenchUIElementFactory { /** Controls the outer multi-diff scroller for compact embedded hosts. */ readonly horizontalScrollbar?: 'auto' | 'hidden'; + /** Keep needed scrollbars visible even when the pointer is elsewhere. */ + readonly alwaysShowScrollbars?: boolean; + /** Start the vertical scrollbar below a resource header pinned at the top. */ + readonly scrollbarBelowResourceHeader?: boolean; /** * External host for the inner editors' overflowing widgets (hover, diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts index 9e4604d59..2c9646429 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts @@ -263,6 +263,9 @@ export class ReviewFilesDiffView extends Disposable { false, undefined, ); + factory.alwaysShowScrollbars = Boolean(document); + // A document embed shows one file, so its header stays pinned at the top. + factory.scrollbarBelowResourceHeader = Boolean(document); this.headerFactory = factory; this.widget = this._register( diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts index 262c90cb3..f627011cc 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts @@ -44,6 +44,9 @@ export class ReviewMultiDiffUIElementFactory implements IWorkbenchUIElementFactory { + alwaysShowScrollbars = false; + scrollbarBelowResourceHeader = false; + get headerClickToCollapse(): boolean { return !this.hideResourceHeader; } diff --git a/packages/review/app/src/App.tsx b/packages/review/app/src/App.tsx index df7a7d181..46eba06d7 100644 --- a/packages/review/app/src/App.tsx +++ b/packages/review/app/src/App.tsx @@ -32,6 +32,7 @@ import { } from "./debug-settings"; import { DiffLayoutControl } from "./diff-layout-control"; import { ReviewDiffView } from "./DiffView"; +import { useDocumentEmbedScroll } from "./document-embed-scroll"; import { useReviewSession } from "./host/review-session"; import { DiscordIcon, MarkerUnderline, SettingsSlidersIcon } from "./icons"; import { ReviewPanelHost } from "./review-components"; @@ -370,6 +371,7 @@ function ReviewLayoutContent({ containerRef: appRef, }); + useDocumentEmbedScroll(scrollRegionRef); const viewStateSync = useReviewViewStateSync({ scrollRegionRef, panelStore }); const hasChangeRange = range.baseCommit !== range.headCommit; diff --git a/packages/review/app/src/document-embed-scroll.browser.test.ts b/packages/review/app/src/document-embed-scroll.browser.test.ts new file mode 100644 index 000000000..0ee9799e7 --- /dev/null +++ b/packages/review/app/src/document-embed-scroll.browser.test.ts @@ -0,0 +1,123 @@ +import { afterEach, expect, it, vi } from "vitest"; + +import { routeDocumentEmbedScroll } from "./document-embed-scroll"; + +let dispose: (() => void) | undefined; + +afterEach(() => dispose?.()); + +function mount(embedClass: string, inDocument = true) { + const region = document.createElement("section"); + region.style.cssText = + "width:300px;height:200px;overflow:auto;line-height:20px"; + region.innerHTML = `
+
+
Content
+
+
`; + document.body.append(region); + dispose = routeDocumentEmbedScroll(region); + const embed = region.querySelector(`.${embedClass}`)!; + const content = embed.firstElementChild!; + + return { region, embed, content }; +} + +it.each(["sequence-diagram", "flow-diagram", "database-lens", "code-peek"])( + "scrolls the document in both directions over %s without scrolling the embed", + (embedClass) => { + const { region, embed, content } = mount(embedClass); + const embeddedWheel = vi.fn<(event: WheelEvent) => void>(); + embed.addEventListener("wheel", embeddedWheel); + region.scrollTop = 300; + embed.scrollTop = 50; + embed.scrollLeft = 60; + + content.dispatchEvent( + new WheelEvent("wheel", { + bubbles: true, + cancelable: true, + deltaY: 120, + deltaX: 30, + }), + ); + expect(region.scrollTop).toBe(420); + expect(region.scrollLeft).toBe(30); + content.dispatchEvent( + new WheelEvent("wheel", { bubbles: true, cancelable: true, deltaY: -80 }), + ); + expect(region.scrollTop).toBe(340); + expect(embed.scrollTop).toBe(50); + expect(embed.scrollLeft).toBe(60); + expect(embeddedWheel).not.toHaveBeenCalled(); + }, +); + +it("normalizes line and page wheel deltas", () => { + const { region, content } = mount("code-peek"); + content.dispatchEvent( + new WheelEvent("wheel", { + bubbles: true, + cancelable: true, + deltaY: 3, + deltaMode: WheelEvent.DOM_DELTA_LINE, + }), + ); + expect(region.scrollTop).toBe(60); + content.dispatchEvent( + new WheelEvent("wheel", { + bubbles: true, + cancelable: true, + deltaY: 1, + deltaMode: WheelEvent.DOM_DELTA_PAGE, + }), + ); + expect(region.scrollTop).toBe(60 + region.clientHeight); +}); + +it("leaves fullscreen tours and pinch-to-zoom alone", () => { + const { region, content } = mount("sequence-diagram", false); + + const wheel = new WheelEvent("wheel", { + bubbles: true, + cancelable: true, + deltaY: 100, + }); + + content.dispatchEvent(wheel); + expect(wheel.defaultPrevented).toBe(false); + expect(region.scrollTop).toBe(0); + region.firstElementChild!.className = "review-document"; + + const pinch = new WheelEvent("wheel", { + bubbles: true, + cancelable: true, + deltaY: 100, + ctrlKey: true, + }); + + content.dispatchEvent(pinch); + expect(pinch.defaultPrevented).toBe(false); + expect(region.scrollTop).toBe(0); +}); + +it("continues scrolling the document when the embed fits and removes the handler on cleanup", () => { + const { region, embed, content } = mount("sequence-diagram"); + (content as HTMLElement).style.cssText = "height:20px;width:20px"; + expect(embed.scrollHeight).toBe(embed.clientHeight); + content.dispatchEvent( + new WheelEvent("wheel", { bubbles: true, cancelable: true, deltaY: 80 }), + ); + expect(region.scrollTop).toBe(80); + dispose?.(); + + const wheel = new WheelEvent("wheel", { + bubbles: true, + cancelable: true, + deltaY: 100, + }); + + content.dispatchEvent(wheel); + expect(wheel.defaultPrevented).toBe(false); + expect(region.scrollTop).toBe(80); +}); diff --git a/packages/review/app/src/document-embed-scroll.ts b/packages/review/app/src/document-embed-scroll.ts new file mode 100644 index 000000000..099e91e25 --- /dev/null +++ b/packages/review/app/src/document-embed-scroll.ts @@ -0,0 +1,51 @@ +import { type RefObject, useEffect } from "react"; + +/** Embedded content is scrolled with its scrollbars; wheel gestures belong + * to the document, even when an embedded native editor handles wheel input. */ +export function useDocumentEmbedScroll( + regionRef: RefObject, +) { + useEffect(() => { + const region = regionRef.current; + + if (!region) return; + + return routeDocumentEmbedScroll(region); + }, [regionRef]); +} + +export function routeDocumentEmbedScroll(region: HTMLElement) { + const onWheel = (event: WheelEvent) => { + if (event.ctrlKey || !(event.target instanceof Element)) return; + + const embed = event.target.closest( + ".sequence-diagram, .flow-diagram, .database-lens, .code-peek", + ); + + if (!embed?.closest(".review-document") || !region.contains(embed)) return; + + // Capture before React Flow/Monaco or native overflow scrolling can consume + // the gesture. Preserve trackpad deltas and normalize mouse line/page units. + event.preventDefault(); + event.stopPropagation(); + const lineHeight = parseFloat(getComputedStyle(region).lineHeight) || 20; + const unit = event.deltaMode === WheelEvent.DOM_DELTA_LINE ? lineHeight : 1; + region.scrollBy({ + left: + event.deltaX * + (event.deltaMode === WheelEvent.DOM_DELTA_PAGE + ? region.clientWidth + : unit), + top: + event.deltaY * + (event.deltaMode === WheelEvent.DOM_DELTA_PAGE + ? region.clientHeight + : unit), + behavior: "instant", + }); + }; + + region.addEventListener("wheel", onWheel, { capture: true, passive: false }); + + return () => region.removeEventListener("wheel", onWheel, { capture: true }); +} diff --git a/packages/review/app/src/styles.css b/packages/review/app/src/styles.css index 4539c2fb0..65bb2d2ed 100644 --- a/packages/review/app/src/styles.css +++ b/packages/review/app/src/styles.css @@ -3738,6 +3738,51 @@ button { page becomes unscrollable wherever the cursor sits over the diagram. */ } +/* Explicit, draggable scrollbars for document diagrams. `auto` adds each + scrollbar only when that axis overflows; styling keeps it visible on macOS. */ +.review-document :is(.sequence-diagram-body, .database-lens-diagram) { + scrollbar-width: auto; + scrollbar-color: auto; +} + +.review-document + :is(.sequence-diagram-body, .database-lens-diagram)::-webkit-scrollbar { + width: var(--review-scrollbar-size); + height: var(--review-scrollbar-size); +} + +.review-document + :is(.sequence-diagram-body, .database-lens-diagram)::-webkit-scrollbar-track, +.review-document + :is( + .sequence-diagram-body, + .database-lens-diagram + )::-webkit-scrollbar-corner { + background: transparent; +} + +.review-document + :is(.sequence-diagram-body, .database-lens-diagram)::-webkit-scrollbar-thumb { + background: var(--review-scrollbar-thumb); + border-radius: 5px; +} + +.review-document + :is( + .sequence-diagram-body, + .database-lens-diagram + )::-webkit-scrollbar-thumb:hover { + background: var(--review-scrollbar-thumb-hover); +} + +.review-document + :is( + .sequence-diagram-body, + .database-lens-diagram + )::-webkit-scrollbar-thumb:active { + background: var(--review-scrollbar-thumb-active); +} + .sequence-diagram > .diagram-header em { background: var(--tray); color: var(--ink-muted);