Skip to content

Commit b287397

Browse files
authored
Merge pull request #5680 from Tyriar/5678
Move overviewRuler API under scrollbar
2 parents 3a147e8 + aebbe81 commit b287397

File tree

10 files changed

+61
-53
lines changed

10 files changed

+61
-53
lines changed

addons/addon-fit/src/FitAddon.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export class FitAddon implements ITerminalAddon, IFitApi {
7272
const showScrollbar = this._terminal.options.scrollbar?.showScrollbar ?? true;
7373
const scrollbarWidth = (this._terminal.options.scrollback === 0 || !showScrollbar
7474
? 0
75-
: (this._terminal.options.overviewRuler?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH));
75+
: (this._terminal.options.scrollbar?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH));
7676

7777
const parentElementStyle = _getComputedStyle(this._terminal.element.parentElement);
7878
const parentElementHeight = parseInt(parentElementStyle.getPropertyValue('height'));

demo/client/components/window/optionsWindow.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -115,19 +115,20 @@ export class OptionsWindow extends BaseWindow implements IControlWindow {
115115
'documentOverride',
116116
'linkHandler',
117117
'logger',
118-
'overviewRuler',
119118
'quirks',
120119
'theme',
121120
'vtExtensions',
122121
'windowOptions',
123122
'windowsPty',
124123
];
125-
const nestedBooleanOptions: { label: string, parent: string, prop: string }[] = [
126-
{ label: 'scrollbar.showScrollbar', parent: 'scrollbar', prop: 'showScrollbar' },
127-
{ label: 'scrollbar.showArrows', parent: 'scrollbar', prop: 'showArrows' },
128-
{ label: 'vtExtensions.kittyKeyboard', parent: 'vtExtensions', prop: 'kittyKeyboard' },
129-
{ label: 'vtExtensions.kittySgrBoldFaintControl', parent: 'vtExtensions', prop: 'kittySgrBoldFaintControl' },
130-
{ label: 'vtExtensions.win32InputMode', parent: 'vtExtensions', prop: 'win32InputMode' }
124+
const nestedBooleanOptions: { label: string, path: string[], prop: string }[] = [
125+
{ label: 'scrollbar.showScrollbar', path: ['scrollbar'], prop: 'showScrollbar' },
126+
{ label: 'scrollbar.showArrows', path: ['scrollbar'], prop: 'showArrows' },
127+
{ label: 'scrollbar.overviewRuler.showTopBorder', path: ['scrollbar', 'overviewRuler'], prop: 'showTopBorder' },
128+
{ label: 'scrollbar.overviewRuler.showBottomBorder', path: ['scrollbar', 'overviewRuler'], prop: 'showBottomBorder' },
129+
{ label: 'vtExtensions.kittyKeyboard', path: ['vtExtensions'], prop: 'kittyKeyboard' },
130+
{ label: 'vtExtensions.kittySgrBoldFaintControl', path: ['vtExtensions'], prop: 'kittySgrBoldFaintControl' },
131+
{ label: 'vtExtensions.win32InputMode', path: ['vtExtensions'], prop: 'win32InputMode' }
131132
];
132133
const stringOptions: { [key: string]: string[] | null } = {
133134
cursorStyle: ['block', 'underline', 'bar'],
@@ -163,8 +164,10 @@ export class OptionsWindow extends BaseWindow implements IControlWindow {
163164
booleanOptions.forEach(o => {
164165
html += `<div class="option"><label><input id="opt-${o}" type="checkbox" ${(this._terminal.options as Record<string, unknown>)[o] ? 'checked' : ''}/> ${o}</label></div>`;
165166
});
166-
nestedBooleanOptions.forEach(({ label, parent, prop }) => {
167-
const checked = (this._terminal.options as Record<string, Record<string, unknown> | undefined>)[parent]?.[prop] ?? false;
167+
nestedBooleanOptions.forEach(({ label, path, prop }) => {
168+
const options = this._terminal.options as Record<string, unknown>;
169+
const parent = path.reduce<Record<string, unknown> | undefined>((acc, key) => (acc as Record<string, unknown> | undefined)?.[key] as Record<string, unknown> | undefined, options);
170+
const checked = (parent as Record<string, unknown> | undefined)?.[prop] ?? false;
168171
html += `<div class="option"><label><input id="opt-${label.replace('.', '-')}" type="checkbox" ${checked ? 'checked' : ''}/> ${label}</label></div>`;
169172
});
170173
html += '</div><div class="option-group">';
@@ -198,11 +201,22 @@ export class OptionsWindow extends BaseWindow implements IControlWindow {
198201
}
199202
});
200203
});
201-
nestedBooleanOptions.forEach(({ label, parent, prop }) => {
204+
nestedBooleanOptions.forEach(({ label, path, prop }) => {
202205
const input = document.getElementById(`opt-${label.replace('.', '-')}`) as HTMLInputElement;
203206
addDomListener(input, 'change', () => {
204207
console.log('change', label, input.checked);
205-
(this._terminal.options as Record<string, unknown>)[parent] = { ...(this._terminal.options as Record<string, Record<string, unknown> | undefined>)[parent], [prop]: input.checked };
208+
const options = this._terminal.options as Record<string, unknown>;
209+
if (path.length === 1) {
210+
const parentKey = path[0];
211+
options[parentKey] = { ...(options[parentKey] as Record<string, unknown> | undefined), [prop]: input.checked };
212+
return;
213+
}
214+
if (path.length === 2) {
215+
const [parentKey, childKey] = path;
216+
const parent = (options[parentKey] as Record<string, unknown> | undefined) ?? {};
217+
const child = (parent[childKey] as Record<string, unknown> | undefined) ?? {};
218+
options[parentKey] = { ...parent, [childKey]: { ...child, [prop]: input.checked } };
219+
}
206220
});
207221
});
208222
numberOptions.forEach(o => {

demo/client/components/window/testWindow.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ function loadTestLongLines(term: Terminal, addons: AddonCollection): void {
738738
}
739739

740740
function addDecoration(term: Terminal, dim: number = 1): void {
741-
term.options['overviewRuler'] = { width: 14 };
741+
term.options.scrollbar = { ...(term.options.scrollbar ?? {}), width: 14, overviewRuler: term.options.scrollbar?.overviewRuler ?? {} };
742742
const marker = term.registerMarker(1);
743743
const decoration = term.registerDecoration({
744744
marker,
@@ -755,7 +755,7 @@ function addDecoration(term: Terminal, dim: number = 1): void {
755755
}
756756

757757
function addOverviewRuler(term: Terminal): void {
758-
term.options['overviewRuler'] = { width: 14 };
758+
term.options.scrollbar = { ...(term.options.scrollbar ?? {}), width: 14, overviewRuler: term.options.scrollbar?.overviewRuler ?? {} };
759759
term.registerDecoration({ marker: term.registerMarker(1), overviewRulerOptions: { color: '#ef2929' } });
760760
term.registerDecoration({ marker: term.registerMarker(3), overviewRulerOptions: { color: '#8ae234' } });
761761
term.registerDecoration({ marker: term.registerMarker(5), overviewRulerOptions: { color: '#729fcf' } });

src/browser/CoreBrowserTerminal.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -617,17 +617,12 @@ export class CoreBrowserTerminal extends CoreTerminal implements ITerminal {
617617
this._register(this.optionsService.onSpecificOptionChange('screenReaderMode', e => this._handleScreenReaderModeOptionChange(e)));
618618

619619
const showScrollbar = this.options.scrollbar?.showScrollbar ?? true;
620-
if (showScrollbar && this.options.overviewRuler.width) {
620+
const overviewRulerWidth = this.options.scrollbar?.width;
621+
if (showScrollbar && overviewRulerWidth) {
621622
this._overviewRulerRenderer = this._register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement));
622623
}
623-
this.optionsService.onSpecificOptionChange('overviewRuler', value => {
624-
const shouldShow = (this.options.scrollbar?.showScrollbar ?? true) && !!value?.width;
625-
if (!this._overviewRulerRenderer && shouldShow && this._viewportElement && this.screenElement) {
626-
this._overviewRulerRenderer = this._register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement));
627-
}
628-
});
629624
this.optionsService.onSpecificOptionChange('scrollbar', value => {
630-
const shouldShow = (value?.showScrollbar ?? true) && !!this.options.overviewRuler.width;
625+
const shouldShow = (value?.showScrollbar ?? true) && !!value?.width;
631626
if (!this._overviewRulerRenderer && shouldShow && this._viewportElement && this.screenElement) {
632627
this._overviewRulerRenderer = this._register(this._instantiationService.createInstance(OverviewRulerRenderer, this._viewportElement, this.screenElement));
633628
}

src/browser/Viewport.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ export class Viewport extends Disposable {
6262
this._register(this._optionsService.onMultipleOptionChange([
6363
'scrollSensitivity',
6464
'fastScrollSensitivity',
65-
'overviewRuler',
6665
'scrollbar'
6766
], () => this._scrollableElement.updateOptions(this._getChangeOptions())));
6867
// Don't handle mouse wheel if wheel events are supported by the current mouse prototcol
@@ -135,7 +134,7 @@ export class Viewport extends Disposable {
135134
const showScrollbar = this._optionsService.rawOptions.scrollbar?.showScrollbar ?? true;
136135
const showArrows = this._optionsService.rawOptions.scrollbar?.showArrows ?? false;
137136
const verticalScrollbarSize = showScrollbar
138-
? (this._optionsService.rawOptions.overviewRuler?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH)
137+
? (this._optionsService.rawOptions.scrollbar?.width ?? ViewportConstants.DEFAULT_SCROLL_BAR_WIDTH)
139138
: 0;
140139
return {
141140
mouseWheelScrollSensitivity: this._optionsService.rawOptions.scrollSensitivity,

src/browser/decorations/OverviewRulerRenderer.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@ export class OverviewRulerRenderer extends Disposable {
3838
private readonly _ctx: CanvasRenderingContext2D;
3939
private readonly _colorZoneStore: IColorZoneStore = new ColorZoneStore();
4040
private get _width(): number {
41-
const showScrollbar = this._optionsService.rawOptions.scrollbar?.showScrollbar ?? true;
41+
const scrollbar = this._optionsService.rawOptions.scrollbar;
42+
const showScrollbar = scrollbar?.showScrollbar ?? true;
4243
if (!showScrollbar) {
4344
return 0;
4445
}
45-
return this._optionsService.rawOptions.overviewRuler?.width ?? 0;
46+
return scrollbar?.width ?? 0;
4647
}
4748
private _animationFrame: number | undefined;
4849

@@ -99,7 +100,6 @@ export class OverviewRulerRenderer extends Disposable {
99100
}));
100101

101102
this._register(this._coreBrowserService.onDprChange(() => this._queueRefresh(true)));
102-
this._register(this._optionsService.onSpecificOptionChange('overviewRuler', () => this._queueRefresh(true)));
103103
this._register(this._optionsService.onSpecificOptionChange('scrollbar', () => this._queueRefresh(true)));
104104
this._register(this._themeService.onChangeColors(() => this._queueRefresh()));
105105
this._queueRefresh(true);
@@ -181,10 +181,10 @@ export class OverviewRulerRenderer extends Disposable {
181181
private _renderRulerOutline(): void {
182182
this._ctx.fillStyle = this._themeService.colors.overviewRulerBorder.css;
183183
this._ctx.fillRect(0, 0, Constants.OVERVIEW_RULER_BORDER_WIDTH, this._canvas.height);
184-
if (this._optionsService.rawOptions.overviewRuler.showTopBorder) {
184+
if (this._optionsService.rawOptions.scrollbar?.overviewRuler?.showTopBorder) {
185185
this._ctx.fillRect(Constants.OVERVIEW_RULER_BORDER_WIDTH, 0, this._canvas.width - Constants.OVERVIEW_RULER_BORDER_WIDTH, Constants.OVERVIEW_RULER_BORDER_WIDTH);
186186
}
187-
if (this._optionsService.rawOptions.overviewRuler.showBottomBorder) {
187+
if (this._optionsService.rawOptions.scrollbar?.overviewRuler?.showBottomBorder) {
188188
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);
189189
}
190190
}

src/common/services/OptionsService.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ export const DEFAULT_OPTIONS: Readonly<Required<ITerminalOptions>> = {
5555
altClickMovesCursor: true,
5656
convertEol: false,
5757
termName: 'xterm',
58-
overviewRuler: {},
5958
quirks: {},
6059
vtExtensions: {}
6160
};

src/common/services/Services.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,6 @@ export interface ITerminalOptions {
265265
windowsPty?: IWindowsPty;
266266
windowOptions?: IWindowOptions;
267267
wordSeparator?: string;
268-
overviewRuler?: IOverviewRulerOptions;
269268
quirks?: ITerminalQuirks;
270269
scrollbar?: IScrollbarOptions;
271270
scrollOnEraseInDisplay?: boolean;
@@ -313,6 +312,8 @@ export interface ITerminalQuirks {
313312
export interface IScrollbarOptions {
314313
showScrollbar?: boolean;
315314
showArrows?: boolean;
315+
width?: number;
316+
overviewRuler?: IOverviewRulerOptions;
316317
}
317318

318319
export interface IVtExtensions {

test/playwright/Terminal.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -816,15 +816,15 @@ test.describe('API Integration Tests', () => {
816816
});
817817
test.describe('overviewRulerDecorations', () => {
818818
test('should not add an overview ruler when width is not set', async () => {
819-
await openTerminal(ctx);
819+
await openTerminal(ctx, { scrollbar: { overviewRuler: {} } });
820820
await ctx.page.evaluate(`window.marker1 = window.term.registerMarker(1)`);
821821
await ctx.page.evaluate(`window.marker2 = window.term.registerMarker(2)`);
822822
await ctx.page.evaluate(`window.term.registerDecoration({ marker: window.marker1, overviewRulerOptions: { color: 'red', position: 'full' } })`);
823823
await ctx.page.evaluate(`window.term.registerDecoration({ marker: window.marker2, overviewRulerOptions: { color: 'blue', position: 'full' } })`);
824824
await pollFor(ctx.page, `document.querySelectorAll('.xterm-decoration-overview-ruler').length`, 0);
825825
});
826826
test('should add an overview ruler when width is set', async () => {
827-
await openTerminal(ctx, { overviewRuler: { width: 15 } });
827+
await openTerminal(ctx, { scrollbar: { width: 15, overviewRuler: {} } });
828828
await ctx.page.evaluate(`window.marker1 = window.term.registerMarker(1)`);
829829
await ctx.page.evaluate(`window.marker2 = window.term.registerMarker(2)`);
830830
await ctx.page.evaluate(`window.term.registerDecoration({ marker: window.marker1, overviewRulerOptions: { color: 'red', position: 'full' } })`);

typings/xterm.d.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,6 @@ declare module '@xterm/xterm' {
206206
*/
207207
minimumContrastRatio?: number;
208208

209-
/**
210-
* Controls the visibility and style of the overview ruler which visualizes
211-
* decorations underneath the scroll bar.
212-
*/
213-
overviewRuler?: IOverviewRulerOptions;
214-
215209
/**
216210
* Control various quirks features that are either non-standard or standard
217211
* in but generally rejected in modern terminals.
@@ -399,8 +393,8 @@ declare module '@xterm/xterm' {
399393
scrollbarSliderActiveBackground?: string;
400394
/**
401395
* The border color of the overview ruler. This visually separates the
402-
* terminal from the scroll bar when {@link IOverviewRulerOptions.width} is
403-
* set. When this is not set it defaults to black (`#000000`).
396+
* terminal from the scroll bar when {@link IScrollbarOptions.width} is set.
397+
* When this is not set it defaults to black (`#000000`).
404398
*/
405399
overviewRulerBorder?: string;
406400
/** ANSI black (eg. `\x1b[30m`) */
@@ -688,8 +682,8 @@ declare module '@xterm/xterm' {
688682

689683
/**
690684
* When defined, renders the decoration in the overview ruler to the right
691-
* of the terminal. {@link IOverviewRulerOptions.width} must be set in order
692-
* to see the overview ruler.
685+
* of the terminal. {@link IScrollbarOptions.width} must be set in order to
686+
* see the overview ruler.
693687
* @param color The color of the decoration.
694688
* @param position The position of the decoration.
695689
*/
@@ -712,16 +706,10 @@ declare module '@xterm/xterm' {
712706
tooMuchOutput: string;
713707
}
714708

709+
/**
710+
* Options for configuring the overview ruler rendered beside the scrollbar.
711+
*/
715712
export interface IOverviewRulerOptions {
716-
/**
717-
* When defined, renders decorations in the overview ruler to the right of
718-
* the terminal. This must be set in order to see the overview ruler.
719-
* This is ignored when {@link IScrollbarOptions.showScrollbar} is false.
720-
* @param color The color of the decoration.
721-
* @param position The position of the decoration.
722-
*/
723-
width?: number;
724-
725713
/**
726714
* Whether to show the top border of the overview ruler, which uses the
727715
* {@link ITheme.overviewRulerBorder} color.
@@ -741,14 +729,26 @@ declare module '@xterm/xterm' {
741729
export interface IScrollbarOptions {
742730
/**
743731
* Whether to show the scrollbar. When false, this supersedes
744-
* {@link IOverviewRulerOptions.width}. Defaults to true.
732+
* {@link IScrollbarOptions.width}. Defaults to true.
745733
*/
746734
showScrollbar?: boolean;
747735
/**
748736
* Whether to show arrows at the top and bottom of the scrollbar. Defaults
749737
* to false.
750738
*/
751739
showArrows?: boolean;
740+
741+
/**
742+
* The width of the scrollbar and overview ruler in CSS pixels. When set,
743+
* this enables the overview ruler.
744+
*/
745+
width?: number;
746+
747+
/**
748+
* Controls the visibility and style of the overview ruler which visualizes
749+
* decorations underneath the scroll bar.
750+
*/
751+
overviewRuler?: IOverviewRulerOptions;
752752
}
753753

754754
/**

0 commit comments

Comments
 (0)