From 6c3b084fe946e9f5ae227d5ed993ad0f3d9ce770 Mon Sep 17 00:00:00 2001 From: UmairShahzad <18100099@lums.edu.pk> Date: Thu, 15 Aug 2019 07:41:23 +0500 Subject: [PATCH 1/3] added scroll bubbling on wheel and touch move events --- src/Terminal.ts | 10 ++++++---- src/TestUtils.test.ts | 4 ++-- src/browser/Types.d.ts | 4 ++-- src/browser/Viewport.ts | 30 ++++++++++++++++++++++++------ 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/Terminal.ts b/src/Terminal.ts index 6d23e16c..10094200 100644 --- a/src/Terminal.ts +++ b/src/Terminal.ts @@ -1037,8 +1037,9 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp // the shell for example this.register(addDisposableDomListener(el, 'wheel', (ev: WheelEvent) => { if (this.mouseEvents) return; - this.viewport.onWheel(ev); - return this.cancel(ev); + if (!this.viewport.onWheel(ev)) { + return this.cancel(ev); + } })); this.register(addDisposableDomListener(el, 'touchstart', (ev: TouchEvent) => { @@ -1049,8 +1050,9 @@ export class Terminal extends Disposable implements ITerminal, IDisposable, IInp this.register(addDisposableDomListener(el, 'touchmove', (ev: TouchEvent) => { if (this.mouseEvents) return; - this.viewport.onTouchMove(ev); - return this.cancel(ev); + if (!this.viewport.onTouchMove(ev)) { + return this.cancel(ev); + } })); } diff --git a/src/TestUtils.test.ts b/src/TestUtils.test.ts index 0ee948d5..4a8e98d6 100644 --- a/src/TestUtils.test.ts +++ b/src/TestUtils.test.ts @@ -401,13 +401,13 @@ export class MockViewport implements IViewport { onThemeChange(colors: IColorSet): void { throw new Error('Method not implemented.'); } - onWheel(ev: WheelEvent): void { + onWheel(ev: WheelEvent): boolean { throw new Error('Method not implemented.'); } onTouchStart(ev: TouchEvent): void { throw new Error('Method not implemented.'); } - onTouchMove(ev: TouchEvent): void { + onTouchMove(ev: TouchEvent): boolean { throw new Error('Method not implemented.'); } syncScrollArea(): void { } diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index 3f3a7637..894beb36 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -37,9 +37,9 @@ export interface IViewport extends IDisposable { scrollBarWidth: number; syncScrollArea(): void; getLinesScrolled(ev: WheelEvent): number; - onWheel(ev: WheelEvent): void; + onWheel(ev: WheelEvent): boolean; onTouchStart(ev: TouchEvent): void; - onTouchMove(ev: TouchEvent): void; + onTouchMove(ev: TouchEvent): boolean; onThemeChange(colors: IColorSet): void; } diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index 9625588d..96ee9621 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -152,20 +152,34 @@ export class Viewport extends Disposable implements IViewport { this._scrollLines(diff, true); } + + private _bubbleScroll(amount: number): boolean { + const scrollPosFromTop = this._viewportElement.scrollTop + this._lastRecordedViewportHeight; + if ((amount < 0 && this._viewportElement.scrollTop !== 0) || + (amount > 0 && scrollPosFromTop < this._lastRecordedBufferHeight)) { + return false; + } + return true; + } + /** * Handles mouse wheel events by adjusting the viewport's scrollTop and delegating the actual * scrolling to `onScroll`, this event needs to be attached manually by the consumer of * `Viewport`. * @param ev The mouse wheel event. */ - public onWheel(ev: WheelEvent): void { + public onWheel(ev: WheelEvent): boolean { const amount = this._getPixelsScrolled(ev); if (amount === 0) { - return; + return false; } this._viewportElement.scrollTop += amount; // Prevent the page from scrolling when the terminal scrolls - ev.preventDefault(); + const shouldBubbleEvent = this._bubbleScroll(amount); + if (!shouldBubbleEvent && ev.cancelable) { + ev.preventDefault(); + } + return shouldBubbleEvent; } private _getPixelsScrolled(ev: WheelEvent): number { @@ -220,13 +234,17 @@ export class Viewport extends Disposable implements IViewport { * Handles the touchmove event, scrolling the viewport if the position shifted. * @param ev The touch event. */ - public onTouchMove(ev: TouchEvent): void { + public onTouchMove(ev: TouchEvent): boolean { const deltaY = this._lastTouchY - ev.touches[0].pageY; this._lastTouchY = ev.touches[0].pageY; if (deltaY === 0) { - return; + return false; } this._viewportElement.scrollTop += deltaY; - ev.preventDefault(); + const shouldBubbleEvent = this._bubbleScroll(deltaY); + if (!shouldBubbleEvent && ev.cancelable) { + ev.preventDefault(); + } + return shouldBubbleEvent; } } From 582934390d3794507cef8034a7df9f468f7ac0c4 Mon Sep 17 00:00:00 2001 From: UmairShahzad <18100099@lums.edu.pk> Date: Wed, 21 Aug 2019 00:57:41 +0500 Subject: [PATCH 2/3] restructured bubble scroll --- src/browser/Viewport.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index 96ee9621..0d441ebe 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -152,12 +152,19 @@ export class Viewport extends Disposable implements IViewport { this._scrollLines(diff, true); } - - private _bubbleScroll(amount: number): boolean { + /** + * Handles bubbling of scroll event in case the viewport has reached top or bottom + * @param ev The scroll event. + * @param amount The amount scrolled + */ + private _bubbleScroll(ev: Event, amount: number): boolean { const scrollPosFromTop = this._viewportElement.scrollTop + this._lastRecordedViewportHeight; if ((amount < 0 && this._viewportElement.scrollTop !== 0) || (amount > 0 && scrollPosFromTop < this._lastRecordedBufferHeight)) { - return false; + if (ev.cancelable) { + ev.preventDefault(); + } + return false; } return true; } @@ -174,12 +181,7 @@ export class Viewport extends Disposable implements IViewport { return false; } this._viewportElement.scrollTop += amount; - // Prevent the page from scrolling when the terminal scrolls - const shouldBubbleEvent = this._bubbleScroll(amount); - if (!shouldBubbleEvent && ev.cancelable) { - ev.preventDefault(); - } - return shouldBubbleEvent; + return this._bubbleScroll(ev, amount); } private _getPixelsScrolled(ev: WheelEvent): number { @@ -241,10 +243,6 @@ export class Viewport extends Disposable implements IViewport { return false; } this._viewportElement.scrollTop += deltaY; - const shouldBubbleEvent = this._bubbleScroll(deltaY); - if (!shouldBubbleEvent && ev.cancelable) { - ev.preventDefault(); - } - return shouldBubbleEvent; + return this._bubbleScroll(ev, deltaY); } } From f91aa75c80bae8855a9e57f61f35077447462df5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 22 Aug 2019 09:12:04 -0700 Subject: [PATCH 3/3] Indent second line of if condition --- src/browser/Viewport.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/Viewport.ts b/src/browser/Viewport.ts index 0d441ebe..4f9363d9 100644 --- a/src/browser/Viewport.ts +++ b/src/browser/Viewport.ts @@ -160,7 +160,7 @@ export class Viewport extends Disposable implements IViewport { private _bubbleScroll(ev: Event, amount: number): boolean { const scrollPosFromTop = this._viewportElement.scrollTop + this._lastRecordedViewportHeight; if ((amount < 0 && this._viewportElement.scrollTop !== 0) || - (amount > 0 && scrollPosFromTop < this._lastRecordedBufferHeight)) { + (amount > 0 && scrollPosFromTop < this._lastRecordedBufferHeight)) { if (ev.cancelable) { ev.preventDefault(); }