From d155bb67674208ef998b68c833b4c41fd96c7eca Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 5 Feb 2026 09:14:58 -0800 Subject: [PATCH] Move overviewRuler API under scrollbar Fixes #5678 --- addons/addon-fit/src/FitAddon.ts | 2 +- .../client/components/window/optionsWindow.ts | 1 - demo/client/components/window/testWindow.ts | 4 +- src/browser/CoreBrowserTerminal.ts | 11 ++--- src/browser/Viewport.ts | 3 +- .../decorations/OverviewRulerRenderer.ts | 10 ++--- src/common/services/OptionsService.ts | 1 - src/common/services/Services.ts | 3 +- test/playwright/Terminal.test.ts | 4 +- typings/xterm.d.ts | 40 +++++++++---------- 10 files changed, 36 insertions(+), 43 deletions(-) diff --git a/addons/addon-fit/src/FitAddon.ts b/addons/addon-fit/src/FitAddon.ts index c51cd63d..a1ef53e7 100644 --- a/addons/addon-fit/src/FitAddon.ts +++ b/addons/addon-fit/src/FitAddon.ts @@ -72,7 +72,7 @@ export class FitAddon implements ITerminalAddon, IFitApi { const showScrollbar = this._terminal.options.scrollbar?.showScrollbar ?? true; const scrollbarWidth = (this._terminal.options.scrollback === 0 || !showScrollbar ? 0 - : (this._terminal.options.overviewRuler?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH)); + : (this._terminal.options.scrollbar?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH)); const parentElementStyle = _getComputedStyle(this._terminal.element.parentElement); const parentElementHeight = parseInt(parentElementStyle.getPropertyValue('height')); diff --git a/demo/client/components/window/optionsWindow.ts b/demo/client/components/window/optionsWindow.ts index dfea4aed..04adbec1 100644 --- a/demo/client/components/window/optionsWindow.ts +++ b/demo/client/components/window/optionsWindow.ts @@ -115,7 +115,6 @@ export class OptionsWindow extends BaseWindow implements IControlWindow { 'documentOverride', 'linkHandler', 'logger', - 'overviewRuler', 'quirks', 'theme', 'vtExtensions', diff --git a/demo/client/components/window/testWindow.ts b/demo/client/components/window/testWindow.ts index 6f2c6c07..acab7b3a 100644 --- a/demo/client/components/window/testWindow.ts +++ b/demo/client/components/window/testWindow.ts @@ -738,7 +738,7 @@ function loadTestLongLines(term: Terminal, addons: AddonCollection): void { } function addDecoration(term: Terminal, dim: number = 1): void { - term.options['overviewRuler'] = { width: 14 }; + term.options.scrollbar = { ...(term.options.scrollbar ?? {}), width: 14, overviewRuler: term.options.scrollbar?.overviewRuler ?? {} }; const marker = term.registerMarker(1); const decoration = term.registerDecoration({ marker, @@ -755,7 +755,7 @@ function addDecoration(term: Terminal, dim: number = 1): void { } function addOverviewRuler(term: Terminal): void { - term.options['overviewRuler'] = { width: 14 }; + term.options.scrollbar = { ...(term.options.scrollbar ?? {}), width: 14, overviewRuler: term.options.scrollbar?.overviewRuler ?? {} }; term.registerDecoration({ marker: term.registerMarker(1), overviewRulerOptions: { color: '#ef2929' } }); term.registerDecoration({ marker: term.registerMarker(3), overviewRulerOptions: { color: '#8ae234' } }); term.registerDecoration({ marker: term.registerMarker(5), overviewRulerOptions: { color: '#729fcf' } }); diff --git a/src/browser/CoreBrowserTerminal.ts b/src/browser/CoreBrowserTerminal.ts index 902f5d35..8ad8409f 100644 --- a/src/browser/CoreBrowserTerminal.ts +++ b/src/browser/CoreBrowserTerminal.ts @@ -617,17 +617,12 @@ export class CoreBrowserTerminal extends CoreTerminal implements ITerminal { this._register(this.optionsService.onSpecificOptionChange('screenReaderMode', e => this._handleScreenReaderModeOptionChange(e))); const showScrollbar = this.options.scrollbar?.showScrollbar ?? true; - if (showScrollbar && this.options.overviewRuler.width) { + const overviewRulerWidth = this.options.scrollbar?.width; + if (showScrollbar && overviewRulerWidth) { this._overviewRulerRenderer = this._register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement)); } - this.optionsService.onSpecificOptionChange('overviewRuler', value => { - const shouldShow = (this.options.scrollbar?.showScrollbar ?? true) && !!value?.width; - if (!this._overviewRulerRenderer && shouldShow && this._viewportElement && this.screenElement) { - this._overviewRulerRenderer = this._register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement)); - } - }); this.optionsService.onSpecificOptionChange('scrollbar', value => { - const shouldShow = (value?.showScrollbar ?? true) && !!this.options.overviewRuler.width; + const shouldShow = (value?.showScrollbar ?? true) && !!value?.width; if (!this._overviewRulerRenderer && shouldShow && this._viewportElement && this.screenElement) { this._overviewRulerRenderer = this._register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement)); } diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index 49e744cf..09864f7a 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -62,7 +62,6 @@ export class Viewport extends Disposable { this._register(this._optionsService.onMultipleOptionChange([ 'scrollSensitivity', 'fastScrollSensitivity', - 'overviewRuler', 'scrollbar' ], () => this._scrollableElement.updateOptions(this._getChangeOptions()))); // Don't handle mouse wheel if wheel events are supported by the current mouse prototcol @@ -135,7 +134,7 @@ export class Viewport extends Disposable { const showScrollbar = this._optionsService.rawOptions.scrollbar?.showScrollbar ?? true; const showArrows = this._optionsService.rawOptions.scrollbar?.showArrows ?? false; const verticalScrollbarSize = showScrollbar - ? (this._optionsService.rawOptions.overviewRuler?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH) + ? (this._optionsService.rawOptions.scrollbar?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH) : 0; return { mouseWheelScrollSensitivity: this._optionsService.rawOptions.scrollSensitivity, diff --git a/src/browser/decorations/OverviewRulerRenderer.ts b/src/browser/decorations/OverviewRulerRenderer.ts index 576cdf6f..c5492f55 100644 --- a/src/browser/decorations/OverviewRulerRenderer.ts +++ b/src/browser/decorations/OverviewRulerRenderer.ts @@ -38,11 +38,12 @@ export class OverviewRulerRenderer extends Disposable { private readonly _ctx: CanvasRenderingContext2D; private readonly _colorZoneStore: IColorZoneStore = new ColorZoneStore(); private get _width(): number { - const showScrollbar = this._optionsService.rawOptions.scrollbar?.showScrollbar ?? true; + const scrollbar = this._optionsService.rawOptions.scrollbar; + const showScrollbar = scrollbar?.showScrollbar ?? true; if (!showScrollbar) { return 0; } - return this._optionsService.rawOptions.overviewRuler?.width ?? 0; + return scrollbar?.width ?? 0; } private _animationFrame: number | undefined; @@ -99,7 +100,6 @@ export class OverviewRulerRenderer extends Disposable { })); this._register(this._coreBrowserService.onDprChange(() => this._queueRefresh(true))); - this._register(this._optionsService.onSpecificOptionChange('overviewRuler', () => this._queueRefresh(true))); this._register(this._optionsService.onSpecificOptionChange('scrollbar', () => this._queueRefresh(true))); this._register(this._themeService.onChangeColors(() => this._queueRefresh())); this._queueRefresh(true); @@ -181,10 +181,10 @@ export class OverviewRulerRenderer extends Disposable { private _renderRulerOutline(): void { this._ctx.fillStyle = this._themeService.colors.overviewRulerBorder.css; this._ctx.fillRect(0, 0, Constants.OVERVIEW_RULER_BORDER_WIDTH, this._canvas.height); - if (this._optionsService.rawOptions.overviewRuler.showTopBorder) { + if (this._optionsService.rawOptions.scrollbar?.overviewRuler?.showTopBorder) { this._ctx.fillRect(Constants.OVERVIEW_RULER_BORDER_WIDTH, 0, this._canvas.width - Constants.OVERVIEW_RULER_BORDER_WIDTH, Constants.OVERVIEW_RULER_BORDER_WIDTH); } - if (this._optionsService.rawOptions.overviewRuler.showBottomBorder) { + if (this._optionsService.rawOptions.scrollbar?.overviewRuler?.showBottomBorder) { this._ctx.fillRect(Constants.OVERVIEW_RULER_BORDER_WIDTH, this._canvas.height - Constants.OVERVIEW_RULER_BORDER_WIDTH, this._canvas.width - Constants.OVERVIEW_RULER_BORDER_WIDTH, this._canvas.height); } } diff --git a/src/common/services/OptionsService.ts b/src/common/services/OptionsService.ts index 397825e9..ec647aaa 100644 --- a/src/common/services/OptionsService.ts +++ b/src/common/services/OptionsService.ts @@ -55,7 +55,6 @@ export const DEFAULT_OPTIONS: Readonly> = { altClickMovesCursor: true, convertEol: false, termName: 'xterm', - overviewRuler: {}, quirks: {}, vtExtensions: {} }; diff --git a/src/common/services/Services.ts b/src/common/services/Services.ts index 85819960..b40f16fd 100644 --- a/src/common/services/Services.ts +++ b/src/common/services/Services.ts @@ -265,7 +265,6 @@ export interface ITerminalOptions { windowsPty?: IWindowsPty; windowOptions?: IWindowOptions; wordSeparator?: string; - overviewRuler?: IOverviewRulerOptions; quirks?: ITerminalQuirks; scrollbar?: IScrollbarOptions; scrollOnEraseInDisplay?: boolean; @@ -313,6 +312,8 @@ export interface ITerminalQuirks { export interface IScrollbarOptions { showScrollbar?: boolean; showArrows?: boolean; + width?: number; + overviewRuler?: IOverviewRulerOptions; } export interface IVtExtensions { diff --git a/test/playwright/Terminal.test.ts b/test/playwright/Terminal.test.ts index 9985fe67..2d1133cc 100644 --- a/test/playwright/Terminal.test.ts +++ b/test/playwright/Terminal.test.ts @@ -816,7 +816,7 @@ test.describe('API Integration Tests', () => { }); test.describe('overviewRulerDecorations', () => { test('should not add an overview ruler when width is not set', async () => { - await openTerminal(ctx); + await openTerminal(ctx, { scrollbar: { overviewRuler: {} } }); await ctx.page.evaluate(`window.marker1 = window.term.registerMarker(1)`); await ctx.page.evaluate(`window.marker2 = window.term.registerMarker(2)`); await ctx.page.evaluate(`window.term.registerDecoration({ marker: window.marker1, overviewRulerOptions: { color: 'red', position: 'full' } })`); @@ -824,7 +824,7 @@ test.describe('API Integration Tests', () => { await pollFor(ctx.page, `document.querySelectorAll('.xterm-decoration-overview-ruler').length`, 0); }); test('should add an overview ruler when width is set', async () => { - await openTerminal(ctx, { overviewRuler: { width: 15 } }); + await openTerminal(ctx, { scrollbar: { width: 15, overviewRuler: {} } }); await ctx.page.evaluate(`window.marker1 = window.term.registerMarker(1)`); await ctx.page.evaluate(`window.marker2 = window.term.registerMarker(2)`); await ctx.page.evaluate(`window.term.registerDecoration({ marker: window.marker1, overviewRulerOptions: { color: 'red', position: 'full' } })`); diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 28f02b68..a850c7be 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -206,12 +206,6 @@ declare module '@xterm/xterm' { */ minimumContrastRatio?: number; - /** - * Controls the visibility and style of the overview ruler which visualizes - * decorations underneath the scroll bar. - */ - overviewRuler?: IOverviewRulerOptions; - /** * Control various quirks features that are either non-standard or standard * in but generally rejected in modern terminals. @@ -399,8 +393,8 @@ declare module '@xterm/xterm' { scrollbarSliderActiveBackground?: string; /** * The border color of the overview ruler. This visually separates the - * terminal from the scroll bar when {@link IOverviewRulerOptions.width} is - * set. When this is not set it defaults to black (`#000000`). + * terminal from the scroll bar when {@link IScrollbarOptions.width} is set. + * When this is not set it defaults to black (`#000000`). */ overviewRulerBorder?: string; /** ANSI black (eg. `\x1b[30m`) */ @@ -688,8 +682,8 @@ declare module '@xterm/xterm' { /** * When defined, renders the decoration in the overview ruler to the right - * of the terminal. {@link IOverviewRulerOptions.width} must be set in order - * to see the overview ruler. + * of the terminal. {@link IScrollbarOptions.width} must be set in order to + * see the overview ruler. * @param color The color of the decoration. * @param position The position of the decoration. */ @@ -712,16 +706,10 @@ declare module '@xterm/xterm' { tooMuchOutput: string; } + /** + * Options for configuring the overview ruler rendered beside the scrollbar. + */ export interface IOverviewRulerOptions { - /** - * When defined, renders decorations in the overview ruler to the right of - * the terminal. This must be set in order to see the overview ruler. - * This is ignored when {@link IScrollbarOptions.showScrollbar} is false. - * @param color The color of the decoration. - * @param position The position of the decoration. - */ - width?: number; - /** * Whether to show the top border of the overview ruler, which uses the * {@link ITheme.overviewRulerBorder} color. @@ -741,7 +729,7 @@ declare module '@xterm/xterm' { export interface IScrollbarOptions { /** * Whether to show the scrollbar. When false, this supersedes - * {@link IOverviewRulerOptions.width}. Defaults to true. + * {@link IScrollbarOptions.width}. Defaults to true. */ showScrollbar?: boolean; /** @@ -749,6 +737,18 @@ declare module '@xterm/xterm' { * to false. */ showArrows?: boolean; + + /** + * The width of the scrollbar and overview ruler in CSS pixels. When set, + * this enables the overview ruler. + */ + width?: number; + + /** + * Controls the visibility and style of the overview ruler which visualizes + * decorations underneath the scroll bar. + */ + overviewRuler?: IOverviewRulerOptions; } /**