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 the range was destroyed on click.
Browse files Browse the repository at this point in the history
Fixes #100. Fixes an issue where the chart selection of a time-frame was destroyed when a click somewhere else in the chart was performed.
  • Loading branch information
Lukas Holzer authored and lukasholzer committed Feb 4, 2020
1 parent 050a325 commit a941b12
Show file tree
Hide file tree
Showing 12 changed files with 219 additions and 80 deletions.
44 changes: 44 additions & 0 deletions apps/components-e2e/src/components/chart/chart-base.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* @license
* Copyright 2020 Dynatrace LLC
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { map } from 'rxjs/operators';

import { DataService } from '../../services/data.service';
import { options } from './chart-options';

export abstract class DtE2EChartBase {
validRange = false;

options = options;

series$ = this._dataService
.getFixture<{ data: Highcharts.IndividualSeriesOptions[] }>(
'/data-small.json',
)
.pipe(map(result => result.data));

constructor(private _dataService: DataService) {}

rangeValidChanges(valid: boolean): void {
this.validRange = valid;
}

/** emits when the selection gets closed */
abstract closed(): void;

/** emits when the value changes */
abstract valueChanges(_value: number | [number, number]): void;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* @license
* Copyright 2020 Dynatrace LLC
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { Selector } from 'testcafe';
import { waitForAngular } from '../../../../utils';
import {
createRange,
rangeSelection,
selection,
overlayText,
} from '../selection-area.po';

const closeCounter = Selector('.closed-counter');

fixture('Selection Area Range Only')
.page('http://localhost:4200/chart/selection-area/range')
.beforeEach(async (testController: TestController) => {
await testController.resizeWindow(1200, 800);
await waitForAngular();
});

test('should not have an initial range selection', async (testController: TestController) => {
await testController
.expect(selection.exists)
.notOk()
.expect(rangeSelection.exists)
.notOk()
.expect(closeCounter.textContent)
.eql('0');
});

test('should not close the range when a click is performed somewhere else in the chart', async () => {
await createRange(520, { x: 310, y: 100 })
.wait(500)
.expect(rangeSelection.exists)
.ok()
.expect(overlayText.textContent)
.match(/Jul 31 \d{2}:17 — \d{2}:23/g)
.click(Selector('.highcharts-plot-background'), {
speed: 0.3,
offsetX: 10,
offsetY: 10,
})
.expect(rangeSelection.exists)
.ok()
.expect(overlayText.textContent)
.match(/Jul 31 \d{2}:17 — \d{2}:23/g);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<dt-chart [options]="options" [series]="series$ | async">
<dt-chart-range
aria-label-close="Close the selection"
aria-label-left-handle="The left handle to update the selection"
aria-label-right-handle="The right handle to update the selection"
aria-label-selected-area="The selected time-frame"
(valid)="rangeValidChanges($event)"
(valueChanges)="valueChanges($event)"
(closed)="closed()"
>
<button
aria-label="Apply the selection"
dt-button
dtChartSelectionAreaAction
[disabled]="!validRange"
i18n
>
Apply
</button>
</dt-chart-range>
</dt-chart>

<div class="closed-counter">{{ closedCounter }}</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* @license
* Copyright 2020 Dynatrace LLC
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { Component } from '@angular/core';
import { DtE2EChartBase } from '../../chart-base';
import { DataService } from '../../../../services/data.service';

@Component({
selector: 'dt-e2e-range',
templateUrl: './range.html',
styles: [
`
:host {
display: block;
width: 1200px;
}
`,
],
})
export class DtE2ERange extends DtE2EChartBase {
closedCounter = 0;

constructor(dataService: DataService) {
super(dataService);
}

closed(): void {
this.closedCounter++;
}

valueChanges(_value: number | [number, number]): void {
// emits when the value changes
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,28 @@ import { DtButtonModule } from '@dynatrace/barista-components/button';
import { DtChartModule } from '@dynatrace/barista-components/chart';

import { DtE2ESelectionArea } from './selection-area';
import { DtE2ERange } from './range/range';
import { DataService } from '../../../services/data.service';

const routes: Route[] = [
{
path: '',
pathMatch: 'full',
component: DtE2ESelectionArea,
},
{
path: 'range',
component: DtE2ERange,
},
];

@NgModule({
declarations: [DtE2ESelectionArea],
declarations: [DtE2ESelectionArea, DtE2ERange],
imports: [
CommonModule,
DtChartModule,
DtButtonModule,
RouterModule.forChild(routes),
],
providers: [DataService],
})
export class DtE2ESelectionAreaModule {}
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,9 @@
* limitations under the License.
*/

// tslint:disable no-magic-numbers
import { Component } from '@angular/core';
import { map } from 'rxjs/operators';

import { DtE2EChartBase } from '../chart-base';
import { DataService } from '../../../services/data.service';
import { options } from './chart-options';
import { Observable } from 'rxjs';

@Component({
selector: 'dt-e2e-selection-area',
Expand All @@ -34,25 +30,9 @@ import { Observable } from 'rxjs';
`,
],
})
export class DtE2ESelectionArea {
validRange = false;

options = options;
// Added type here due to missing support for type inference on windows with typescript 3.4.5
// error TS2742: The inferred type of 'series$' cannot be named without a reference to '...@types/highcharts'.
// This is likely not portable. A type annotation is necessary.
series$: Observable<
Highcharts.IndividualSeriesOptions[]
> = this._dataService
.getFixture<{ data: Highcharts.IndividualSeriesOptions[] }>(
'/data-small.json',
)
.pipe(map(result => result.data));

constructor(private _dataService: DataService) {}

rangeValidChanges(valid: boolean): void {
this.validRange = valid;
export class DtE2ESelectionArea extends DtE2EChartBase {
constructor(dataService: DataService) {
super(dataService);
}

closed(): void {
Expand Down
8 changes: 3 additions & 5 deletions apps/components-e2e/src/components/drawer/drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,15 @@
*/

import { Component, ElementRef, ViewChild } from '@angular/core';
import { map } from 'rxjs/operators';

import { DtChartRange } from '@dynatrace/barista-components/chart';
import {
DtDrawer,
DtDrawerContainer,
} from '@dynatrace/barista-components/drawer';

import { DataService } from '../../services/data.service';
import { options } from '../chart/selection-area/chart-options';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { DataService } from '../../services/data.service';
import { options } from '../chart/chart-options';

@Component({
selector: 'dt-e2e-drawer',
Expand Down
2 changes: 1 addition & 1 deletion components/chart/src/range/range.scss
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ $range-invalid-color: $gray-300;
position: absolute;
right: -9px;
top: 0%;
transition: opacity 400ms ease-out;
transform: translateY(-50%) scaleX(-1);

:host.dt-chart-range-released & {
transition: opacity 400ms ease-out;
opacity: 1;
top: 50%;
}
Expand Down
52 changes: 26 additions & 26 deletions components/chart/src/selection-area/selection-area.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,23 @@ import {
ViewEncapsulation,
} from '@angular/core';
import {
EMPTY,
Observable,
Subject,
addCssClass,
DtFlexibleConnectedPositionStrategy,
DtViewportResizer,
getElementBoundingClientRect,
readKeyCode,
removeCssClass,
ViewportBoundaries,
} from '@dynatrace/barista-components/core';
import {
animationFrameScheduler,
combineLatest,
EMPTY,
fromEvent,
merge,
Observable,
of,
Subject,
} from 'rxjs';
import {
concatMapTo,
Expand All @@ -68,17 +77,6 @@ import {
throttleTime,
withLatestFrom,
} from 'rxjs/operators';

import {
DtFlexibleConnectedPositionStrategy,
DtViewportResizer,
ViewportBoundaries,
addCssClass,
getElementBoundingClientRect,
readKeyCode,
removeCssClass,
} from '@dynatrace/barista-components/core';

import { DtChart } from '../chart';
import { clampRange } from '../range/clamp-range';
import { DtChartRange, RangeStateChangedEvent } from '../range/range';
Expand Down Expand Up @@ -506,6 +504,8 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {
share(),
);

const touchStartAndMouseDown$ = merge(this._mousedown$, touchStart$);

const touchEnd$ = getTouchEndStream(this._elementRef.nativeElement).pipe(
share(),
);
Expand Down Expand Up @@ -726,9 +726,10 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {
// On a mousedown the range and the timestamp have to be hidden.
const startShowingTimestamp$ = this._click$.pipe(mapTo(true));
const startShowingRange$ = this._drag$.pipe(mapTo(true));
const hideTimestampAndRange$ = merge(this._mousedown$, touchStart$).pipe(
mapTo(false),
);
let hideTimestampAndRange$ =
this._chart._range && !this._chart._timestamp
? EMPTY
: touchStartAndMouseDown$.pipe(mapTo(false));

merge(startShowingRange$, hideTimestampAndRange$)
.pipe(
Expand Down Expand Up @@ -782,10 +783,11 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {
// listen for a _closeOverlay event if there is a timestamp or a range

merge(
this._chart._timestamp ? this._chart._timestamp._closeOverlay : of(null),
this._chart._range ? this._chart._range._closeOverlay : of(null),
this._mousedown$,
touchStart$,
this._chart._timestamp ? this._chart._timestamp._closeOverlay : EMPTY,
this._chart._range ? this._chart._range._closeOverlay : EMPTY,
this._chart._range && !this._chart._timestamp
? EMPTY
: touchStartAndMouseDown$,
)
.pipe(takeUntil(this._destroy$))
.subscribe(() => {
Expand Down Expand Up @@ -872,11 +874,9 @@ export class DtChartSelectionArea implements AfterContentInit, OnDestroy {
);

const showHairline$ = hover$.pipe(mapTo(true));
const hideHairline$ = merge(
this._chart._range ? this._mousedown$ : of(null),
this._dragHandle$,
mouseOut$,
).pipe(mapTo(false));
const hideHairline$ = merge(this._drag$, this._dragHandle$, mouseOut$).pipe(
mapTo(false),
);

merge(showHairline$, hideHairline$)
.pipe(
Expand Down
3 changes: 2 additions & 1 deletion components/chart/src/selection-area/streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,9 @@ export function getRangeCreateStream(
dragMove$: Observable<{ x: number; y: number }>,
targetWidth: number,
): Observable<{ left: number; width: number }> {
return combineLatest([dragStart$, dragMove$]).pipe(
return dragStart$.pipe(
observeOn(animationFrameScheduler),
switchMap(start => dragMove$.pipe(map(move => [start, move]))),
map(([startPosition, endPosition]) =>
calculatePosition(
DtSelectionAreaEventTarget.Origin,
Expand Down
Loading

0 comments on commit a941b12

Please sign in to comment.