Skip to content

Commit

Permalink
Make scaling state a handler of onScaleFactorChange event
Browse files Browse the repository at this point in the history
Currently, scaling state happens when the view range changes. This is not
the expected behaviour. States should only be scaled when the scale factor
changes. This commit moves the function that scales the states from
the view range change handlers to the scale factor change handlers. This
will also help make better code isolation for unit tests.

Signed-off-by: Hoang Thuan Pham <hoang.pham@calian.ca>
  • Loading branch information
hoangphamEclipse committed Jun 5, 2023
1 parent 08fb2e6 commit 5d40c37
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
10 changes: 8 additions & 2 deletions timeline-chart/src/layer/time-graph-chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export class TimeGraphChart extends TimeGraphChartLayer {

private _viewRangeChangedHandler: { (): void; (viewRange: TimelineChart.TimeGraphRange): void; (selectionRange: TimelineChart.TimeGraphRange): void };
private _zoomRangeChangedHandler: (zoomFactor: number) => void;
private _scaleFactorChangedHandler: (scaleFactor: number) => void;
private _mouseMoveHandler: { (event: MouseEvent): void; (event: Event): void };
private _mouseDownHandler: { (event: MouseEvent): void; (event: Event): void };
private _keyDownHandler: { (event: KeyboardEvent): void; (event: Event): void };
Expand Down Expand Up @@ -335,13 +336,18 @@ export class TimeGraphChart extends TimeGraphChartLayer {
});

this._viewRangeChangedHandler = () => {
this.scaleStateLabels();
this.updateZoomingSelection();
this.ensureRowLinesFitViewWidth();
};
this.unitController.onViewRangeChanged(this._viewRangeChangedHandler);
this.unitController.onViewRangeChanged(this._debouncedMaybeFetchNewData);

this._scaleFactorChangedHandler = () => {
this.scaleStateLabels();
}

this.stateController.onScaleFactorChange(this._scaleFactorChangedHandler);

/**
* We need to explicitly re-render every zoom change because of edge case:
* WorldRange = Absolute Range && ViewRange < WorldRange
Expand Down Expand Up @@ -372,7 +378,6 @@ export class TimeGraphChart extends TimeGraphChartLayer {
}

update() {
this.scaleStateLabels();
this.ensureRowLinesFitViewWidth();
this._debouncedMaybeFetchNewData();
}
Expand Down Expand Up @@ -413,6 +418,7 @@ export class TimeGraphChart extends TimeGraphChartLayer {

destroy() {
this.stateController.removeOnZoomChanged(this._zoomRangeChangedHandler);
this.stateController.removeOnScaleFactorChanged(this._scaleFactorChangedHandler);
this.unitController.removeViewRangeChangedHandler(this._debouncedMaybeFetchNewData);
this.unitController.removeViewRangeChangedHandler(this._viewRangeChangedHandler);
if (this._viewRangeChangedHandler) {
Expand Down
7 changes: 7 additions & 0 deletions timeline-chart/src/time-graph-state-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ export class TimeGraphStateController {
}
}

removeOnScaleFactorChanged(handler: (zoomFactor: number) => void) {
const index = this.scaleFactorChangedHandlers.indexOf(handler);
if (index > -1) {
this.scaleFactorChangedHandlers.splice(index, 1);
}
}

/**
It is not the width of the canvas display buffer but of the canvas element in browser. Can be different depending on the display pixel ratio.
*/
Expand Down

0 comments on commit 5d40c37

Please sign in to comment.