From a542bac24099925c00c9991dcd538abd4999f8da Mon Sep 17 00:00:00 2001 From: Peter Affenzeller Date: Wed, 15 Jan 2020 08:31:44 +0100 Subject: [PATCH] fix(chart): Fixes an issue where timestamp or range reopened after resizing the window. When closing a timestamp or range and then resizing the window, the overlay is re-opened. This PR fixes that issue by splitting up the stream that fires when a timestamp/range changes and the window resize stream and only updating the overlay on resize if it actually exists. Fixes #472 --- .../selection-area/selection-area.e2e.ts | 80 +++++++++++++++++++ .../src/selection-area/selection-area.ts | 65 +++++++++++---- 2 files changed, 131 insertions(+), 14 deletions(-) diff --git a/apps/components-e2e/src/components/chart/selection-area/selection-area.e2e.ts b/apps/components-e2e/src/components/chart/selection-area/selection-area.e2e.ts index 321ed83de1..78a6d573ed 100644 --- a/apps/components-e2e/src/components/chart/selection-area/selection-area.e2e.ts +++ b/apps/components-e2e/src/components/chart/selection-area/selection-area.e2e.ts @@ -256,3 +256,83 @@ test('should create a range out of the timestamp with shift + left arrow key', a .expect(overlayText.textContent) .match(/Jul 31 \d{2}:20 — \d{2}:25/g); }); + +test('should not re-open a timestamp in case of a window resize', async (testController: TestController) => { + await createTimestamp( + { x: 450, y: 100 }, + chartClickTargets[1], + testController, + ) + .expect(timestampSelection.exists) + .ok() + .expect(overlay.exists) + .ok() + .click(closeButton, { speed: 0.3 }) + .expect(timestampSelection.exists) + .notOk() + .expect(overlay.exists) + .notOk() + .resizeWindow(1000, 600) + .wait(250) + .expect(timestampSelection.exists) + .notOk() + .expect(overlay.exists) + .notOk(); +}); + +test('should not re-open a range in case of a window resize', async (testController: TestController) => { + await createRange(550, { x: 310, y: 100 }, testController) + .expect(rangeSelection.exists) + .ok() + .expect(overlay.exists) + .ok() + .click(closeButton, { speed: 0.3 }) + .expect(rangeSelection.exists) + .notOk() + .expect(overlay.exists) + .notOk() + .resizeWindow(1000, 600) + .wait(250) + .expect(rangeSelection.exists) + .notOk() + .expect(overlay.exists) + .notOk(); +}); + +test('should keep existing timestamp open in case of a window resize', async (testController: TestController) => { + await createTimestamp( + { x: 450, y: 100 }, + chartClickTargets[1], + testController, + ) + .expect(timestampSelection.exists) + .ok() + .expect(overlay.exists) + .ok() + .expect(overlayText.textContent) + .match(/Jul 31, \d{2}:19/g) + .resizeWindow(1000, 600) + .wait(250) + .expect(timestampSelection.exists) + .ok() + .expect(overlay.exists) + .ok() + .expect(overlayText.textContent) + .match(/Jul 31, \d{2}:19/g); +}); + +test('should keep existing range open in case of a window resize', async (testController: TestController) => { + await createRange(550, { x: 310, y: 100 }, testController) + .expect(rangeSelection.exists) + .ok() + .expect(overlayText.textContent) + .match(/Jul 31 \d{2}:17 — \d{2}:23/g) + .resizeWindow(1000, 600) + .wait(250) + .expect(rangeSelection.exists) + .ok() + .expect(overlay.exists) + .ok() + .expect(overlayText.textContent) + .match(/Jul 31 \d{2}:17 — \d{2}:23/g); +}); diff --git a/components/chart/src/selection-area/selection-area.ts b/components/chart/src/selection-area/selection-area.ts index 4308290628..cca7ef0d28 100644 --- a/components/chart/src/selection-area/selection-area.ts +++ b/components/chart/src/selection-area/selection-area.ts @@ -332,6 +332,7 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy { .subscribe(() => { if (this._chart._range) { this._chart._range.focus(); + this._chart._range._reflectRangeReleased(true); } }); } @@ -459,6 +460,11 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy { /** If there is an overlay open it will dispose it and destroy it */ private _closeOverlay(): void { + // remove class showing the arrows of the range handles + if (this._chart._range) { + this._chart._range._reflectRangeReleased(false); + } + if (this._overlayRef) { this._overlayRef.dispose(); } @@ -806,26 +812,38 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy { // handling of the overlay for the range if (this._chart._range) { - combineLatest([ - merge( - getElementRefStream( - this._chart._range._stateChanges, - this._destroy$, - this._chart._range._rangeElementRef, - this._zone, - ), - this._dragHandle$.pipe( - getElementRef(this._chart._range._rangeElementRef), - ), + const elementReferenceStream = getElementRefStream( + this._chart._range._stateChanges, + this._destroy$, + this._chart._range._rangeElementRef, + this._zone, + ); + merge( + elementReferenceStream, + this._dragHandle$.pipe( + getElementRef(this._chart._range._rangeElementRef), ), - this._viewportBoundaries$, - ]) + ) + .pipe(takeUntil(this._destroy$)) + .subscribe(ref => { + if (this._chart._range && this._chart._range._overlayTemplate) { + this._updateOrCreateOverlay(this._chart._range, ref); + } + }); + + // update the overlay position if an overlay exists + // and the viewport size changes + combineLatest([elementReferenceStream, this._viewportBoundaries$]) .pipe( throttleTime(0, animationFrameScheduler), takeUntil(this._destroy$), ) .subscribe(([ref, viewPortOffset]) => { - if (this._chart._range && this._chart._range._overlayTemplate) { + if ( + this._overlayRef && + this._chart._range && + this._chart._range._overlayTemplate + ) { this._updateOrCreateOverlay( this._chart._range, ref, @@ -837,6 +855,24 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy { // handling of the overlay for the timestamp if (this._chart._timestamp) { + getElementRefStream( + this._chart._timestamp._stateChanges, + this._destroy$, + this._chart._timestamp._timestampElementRef, + this._zone, + ) + .pipe(takeUntil(this._destroy$)) + .subscribe(ref => { + if ( + this._chart._timestamp && + this._chart._timestamp._overlayTemplate + ) { + this._updateOrCreateOverlay(this._chart._timestamp, ref); + } + }); + + // update the overlay position if an overlay exists + // and the viewport size changes combineLatest([ getElementRefStream( this._chart._timestamp._stateChanges, @@ -852,6 +888,7 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy { ) .subscribe(([ref, viewPortOffset]) => { if ( + this._overlayRef && this._chart._timestamp && this._chart._timestamp._overlayTemplate ) {