From f2991a5636d8791c900185ea8f2f6c6f206a25cc Mon Sep 17 00:00:00 2001 From: meganrogge Date: Tue, 8 Feb 2022 21:10:34 -0600 Subject: [PATCH] cleanup --- css/xterm.css | 2 +- demo/client.ts | 4 +- src/browser/Terminal.ts | 13 +++--- src/browser/public/Terminal.ts | 3 ++ src/browser/services/DecorationService.ts | 54 ++++++++++------------- src/browser/services/Services.ts | 9 ++++ test/api/Terminal.api.ts | 13 ++++-- typings/xterm.d.ts | 6 +++ 8 files changed, 62 insertions(+), 42 deletions(-) diff --git a/css/xterm.css b/css/xterm.css index ac8cd45b..f8bfcd23 100644 --- a/css/xterm.css +++ b/css/xterm.css @@ -174,7 +174,7 @@ text-decoration: line-through; } -#terminal-container > div > div.xterm-screen > div.xterm-decoration { +.xterm-screen .xterm-decorations .xterm-decoration { z-index: 6; position: absolute; } diff --git a/demo/client.ts b/demo/client.ts index 193b78b0..977bcfd8 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -149,7 +149,7 @@ if (document.location.pathname === '/test') { document.getElementById('serialize').addEventListener('click', serializeButtonHandler); document.getElementById('custom-glyph').addEventListener('click', writeCustomGlyphHandler); document.getElementById('load-test').addEventListener('click', loadTest); - document.getElementById('decoration').addEventListener('click', decoration); + document.getElementById('decoration').addEventListener('click', addDecoration); } function createTerminal(): void { @@ -527,7 +527,7 @@ function loadTest() { }); } -function decoration() { +function addDecoration() { const marker = term.addMarker(1); const decoration = term.registerDecoration({ marker }); decoration.element.style.backgroundColor = 'red'; diff --git a/src/browser/Terminal.ts b/src/browser/Terminal.ts index 14a107f9..bd79b7c5 100644 --- a/src/browser/Terminal.ts +++ b/src/browser/Terminal.ts @@ -45,7 +45,7 @@ import { EventEmitter, IEvent, forwardEvent } from 'common/EventEmitter'; import { DEFAULT_ATTR_DATA } from 'common/buffer/BufferLine'; import { ColorManager } from 'browser/ColorManager'; import { RenderService } from 'browser/services/RenderService'; -import { ICharSizeService, IRenderService, IMouseService, ISelectionService, ISoundService, ICoreBrowserService, ICharacterJoinerService } from 'browser/services/Services'; +import { ICharSizeService, IRenderService, IMouseService, ISelectionService, ISoundService, ICoreBrowserService, ICharacterJoinerService, IDecorationService } from 'browser/services/Services'; import { CharSizeService } from 'browser/services/CharSizeService'; import { IBuffer } from 'common/buffer/Types'; import { MouseService } from 'browser/services/MouseService'; @@ -55,7 +55,7 @@ import { CoreTerminal } from 'common/CoreTerminal'; import { color, rgba } from 'browser/Color'; import { CharacterJoinerService } from 'browser/services/CharacterJoinerService'; import { toRgbString } from 'common/input/XParseColor'; -import { DecorationService, IDecorationService } from 'browser/services/DecorationService'; +import { DecorationService } from 'browser/services/DecorationService'; // Let it work inside Node.js for automated testing purposes. const document: Document = (typeof window !== 'undefined') ? window.document : null as any; @@ -81,7 +81,6 @@ export class Terminal extends CoreTerminal implements ITerminal { private _charSizeService: ICharSizeService | undefined; private _mouseService: IMouseService | undefined; private _renderService: IRenderService | undefined; - private _decorationService: IDecorationService | undefined; private _characterJoinerService: ICharacterJoinerService | undefined; private _selectionService: ISelectionService | undefined; private _soundService: ISoundService | undefined; @@ -110,6 +109,7 @@ export class Terminal extends CoreTerminal implements ITerminal { public linkifier: ILinkifier; public linkifier2: ILinkifier2; public viewport: IViewport | undefined; + public decorationService: IDecorationService; private _compositionHelper: ICompositionHelper | undefined; private _mouseZoneManager: IMouseZoneManager | undefined; private _accessibilityManager: AccessibilityManager | undefined; @@ -174,6 +174,8 @@ export class Terminal extends CoreTerminal implements ITerminal { // Setup listeners this.register(this._bufferService.onResize(e => this._afterResize(e.cols, e.rows))); + + this.decorationService = this.register(this._instantiationService.createInstance(DecorationService)); } /** @@ -515,8 +517,6 @@ export class Terminal extends CoreTerminal implements ITerminal { this.register(this._renderService.onRenderedBufferChange(e => this._onRender.fire(e))); this.onResize(e => this._renderService!.resize(e.cols, e.rows)); - this._decorationService = this.register(this._instantiationService.createInstance(DecorationService, this.screenElement)); - this._compositionView = document.createElement('div'); this._compositionView.classList.add('composition-view'); this._compositionHelper = this._instantiationService.createInstance(CompositionHelper, this.textarea, this._compositionView); @@ -578,6 +578,7 @@ export class Terminal extends CoreTerminal implements ITerminal { this.linkifier.attachToDom(this.element, this._mouseZoneManager); this.linkifier2.attachToDom(this.screenElement, this._mouseService, this._renderService); + this.decorationService.attachToDom(this.screenElement); // This event listener must be registered aftre MouseZoneManager is created this.register(addDisposableDomListener(this.element, 'mousedown', (e: MouseEvent) => this._selectionService!.onMouseDown(e))); @@ -1007,7 +1008,7 @@ export class Terminal extends CoreTerminal implements ITerminal { if (this.buffer !== this.buffers.normal) { return undefined; } - return this._decorationService!.registerDecoration(decorationOptions); + return this.decorationService!.registerDecoration(decorationOptions); } /** diff --git a/src/browser/public/Terminal.ts b/src/browser/public/Terminal.ts index 1c4c653b..ff2f88cf 100644 --- a/src/browser/public/Terminal.ts +++ b/src/browser/public/Terminal.ts @@ -173,6 +173,9 @@ export class Terminal implements ITerminalApi { } public registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined { this._checkProposedApi(); + if (decorationOptions.x) { + this._verifyIntegers(decorationOptions.x); + } return this._core.registerDecoration(decorationOptions); } public addMarker(cursorYOffset: number): IMarker | undefined { diff --git a/src/browser/services/DecorationService.ts b/src/browser/services/DecorationService.ts index cfe10045..8a6b96da 100644 --- a/src/browser/services/DecorationService.ts +++ b/src/browser/services/DecorationService.ts @@ -3,26 +3,19 @@ * @license MIT */ -import { IRenderService } from 'browser/services/Services'; +import { IDecorationService, IRenderService } from 'browser/services/Services'; import { EventEmitter, IEvent } from 'common/EventEmitter'; import { Disposable } from 'common/Lifecycle'; -import { createDecorator } from 'common/services/ServiceRegistry'; import { IBufferService } from 'common/services/Services'; -import { IDisposable } from 'common/Types'; import { IDecorationOptions, IDecoration, IMarker } from 'xterm'; -export interface IDecorationService extends IDisposable { - registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined; - refresh(): void; - dispose(): void; -} - export class DecorationService extends Disposable implements IDecorationService { private _decorations: Decoration[] = []; + private _screenElement: HTMLElement | undefined; + constructor( - private readonly _screenElement: HTMLElement, @IBufferService private readonly _bufferService: IBufferService, @IRenderService private readonly _renderService: IRenderService ) { @@ -30,8 +23,12 @@ export class DecorationService extends Disposable implements IDecorationService this.register(this._renderService.onRenderedBufferChange(() => this.refresh())); } + public attachToDom(screenElement: HTMLElement): void { + this._screenElement = screenElement; + } + public registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined { - if (decorationOptions.marker.isDisposed) { + if (decorationOptions.marker.isDisposed || !this._screenElement) { return undefined; } const decoration = new Decoration(decorationOptions, this._screenElement, this._renderService, this._bufferService); @@ -58,15 +55,14 @@ export class DecorationService extends Disposable implements IDecorationService } } -export const IDecorationService = createDecorator('DecorationService'); class Decoration extends Disposable implements IDecoration { - private static _nextId = 1; + public readonly id: number = Decoration._nextId++; + private static _nextId: number = 1; + private _marker: IMarker; private _element: HTMLElement | undefined; - private _id: number = Decoration._nextId++; public isDisposed: boolean = false; - public get id(): number { return this._id; } public get element(): HTMLElement { return this._element!; } public get marker(): IMarker { return this._marker; } @@ -86,18 +82,19 @@ class Decoration extends Disposable implements IDecoration { this._marker = _decorationOptions.marker; this._createElement(); this._render(); - } - - public dispose(): void { - if (this.isDisposed) { - return; - } - this._screenElement.removeChild(this.element); - this.isDisposed = true; - this._marker.dispose(); - // Emit before super.dispose such that dispose listeners get a change to react - this._onDispose.fire(); - super.dispose(); + this.register({ + dispose: () => { + if (this.isDisposed) { + return; + } + this._screenElement.removeChild(this.element); + this.isDisposed = true; + this._marker.dispose(); + // Emit before super.dispose such that dispose listeners get a change to react + this._onDispose.fire(); + super.dispose(); + } + }); } private _createElement(): void { @@ -119,9 +116,6 @@ class Decoration extends Disposable implements IDecoration { } private _resolveDimensions(): void { - if (!this._renderService.dimensions.scaledCellWidth || !this._renderService.dimensions.scaledCellHeight) { - throw new Error(`Cannot resolve dimensions for decoration when scaled cell dimensions are undefined ${this._renderService.dimensions}.`); - } this._decorationOptions.width = this._decorationOptions.width ? this._decorationOptions.width * this._renderService.dimensions.scaledCellWidth : this._renderService.dimensions.scaledCellWidth; this._decorationOptions.height = this._decorationOptions.height ? this._decorationOptions.height * this._renderService.dimensions.scaledCellHeight : this._renderService.dimensions.scaledCellHeight; } diff --git a/src/browser/services/Services.ts b/src/browser/services/Services.ts index 4928fa28..5bab6886 100644 --- a/src/browser/services/Services.ts +++ b/src/browser/services/Services.ts @@ -9,6 +9,7 @@ import { IColorSet } from 'browser/Types'; import { ISelectionRedrawRequestEvent as ISelectionRequestRedrawEvent, ISelectionRequestScrollLinesEvent } from 'browser/selection/Types'; import { createDecorator } from 'common/services/ServiceRegistry'; import { IDisposable } from 'common/Types'; +import { IDecorationOptions, IDecoration } from 'xterm'; export const ICharSizeService = createDecorator('CharSizeService'); export interface ICharSizeService { @@ -113,3 +114,11 @@ export interface ICharacterJoinerService { deregister(joinerId: number): boolean; getJoinedCharacters(row: number): [number, number][]; } + + +export const IDecorationService = createDecorator('DecorationService'); +export interface IDecorationService extends IDisposable { + registerDecoration(decorationOptions: IDecorationOptions): IDecoration | undefined; + refresh(): void; + attachToDom(screenElement: HTMLElement): void; +} diff --git a/test/api/Terminal.api.ts b/test/api/Terminal.api.ts index c10094ca..22964998 100644 --- a/test/api/Terminal.api.ts +++ b/test/api/Terminal.api.ts @@ -731,7 +731,7 @@ describe('API Integration Tests', function(): void { await pollFor(page, `window.term._core._renderService.dimensions.actualCellWidth > 0`, true); }); - describe.only('registerDecoration', () => { + describe('registerDecoration', () => { it('should register a decoration', async () => { await openTerminal(page); await page.evaluate(`window.marker = window.term.addMarker(1)`); @@ -745,10 +745,17 @@ describe('API Integration Tests', function(): void { assert.equal(await page.evaluate(`window.decoration = window.term.registerDecoration({ marker: window.marker });`), undefined); assert.equal(await page.evaluate(`document.querySelector('.xterm-screen .xterm-decoration')`), undefined); }); - it.skip('should throw when a negative x offset is provided', async () => { + it('should throw when a negative x offset is provided', async () => { await openTerminal(page); await page.evaluate(`window.marker = window.term.addMarker(1)`); - assert.throws(async () => await page.evaluate(`window.decoration = window.term.registerDecoration({ marker: window.marker, x: -2 });`)); + await page.evaluate(` + try { + window.decoration = window.term.registerDecoration({ marker: window.marker, x: -2 }); + } catch (e) { + window.throwMessage = e.message; + } + `); + await pollFor(page, 'window.throwMessage', 'Decoration options x value cannot be negative, but was -2.'); assert.equal(await page.evaluate(`document.querySelector('.xterm-screen .xterm-decoration')`), undefined); }); }); diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index 5ba7dcf5..0a307f31 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -434,6 +434,12 @@ declare module 'xterm' { readonly element: HTMLElement | undefined; } + /** + * Options provided when registering a decoration + * containing a @param marker, @param anchor, + * @param x offset from the anchor, @param width in cells + * and @param height in cells. + */ export interface IDecorationOptions { /** * The line in the terminal where