Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
fix(chart): Fixes an issue where timestamp or range reopened after
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@peter-affenzeller @tomheller
peter-affenzeller authored and tomheller committed Feb 24, 2020
1 parent fdcdd1f commit 24c45b8
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 14 deletions.
80 changes: 80 additions & 0 deletions apps/components-e2e/src/components/chart/selection-area/selection-area.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}:24/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}:24/g);
});
65 changes: 51 additions & 14 deletions components/chart/src/selection-area/selection-area.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {
.subscribe(() => {
if (this._chart._range) {
this._chart._range.focus();
this._chart._range._reflectRangeReleased(true);
}
});
}
Expand Down Expand Up @@ -461,6 +462,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();
}
Expand Down Expand Up @@ -808,26 +814,38 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {

// handling of the overlay for the range
if (this._chart._range) {
combineLatest([
merge(
getElementRefStream<HTMLDivElement>(
this._chart._range._stateChanges,
this._destroy$,
this._chart._range._rangeElementRef,
this._zone,
),
this._dragHandle$.pipe(
getElementRef(this._chart._range._rangeElementRef),
),
const elementReferenceStream = getElementRefStream<HTMLDivElement>(
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,
Expand All @@ -839,6 +857,24 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {

// handling of the overlay for the timestamp
if (this._chart._timestamp) {
getElementRefStream<HTMLDivElement>(
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<HTMLDivElement>(
this._chart._timestamp._stateChanges,
Expand All @@ -854,6 +890,7 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {
)
.subscribe(([ref, viewPortOffset]) => {
if (
this._overlayRef &&
this._chart._timestamp &&
this._chart._timestamp._overlayTemplate
) {
Expand Down

0 comments on commit 24c45b8

Please sign in to comment.