Skip to content

Commit

Permalink
conditionally add wheel event listener
Browse files Browse the repository at this point in the history
Only add the wheel event listener when required. This offers a small performance improvement for users who disable this option in `handleScroll` and `handleScale` options.
  • Loading branch information
SlicedSilver committed Sep 14, 2022
1 parent 2ba2dc7 commit 3992003
Showing 1 changed file with 24 additions and 3 deletions.
27 changes: 24 additions & 3 deletions src/gui/chart-widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ export class ChartWidget implements IDestroyable {
this._element.appendChild(this._tableElement);

this._onWheelBound = this._onMousewheel.bind(this);
this._element.addEventListener('wheel', this._onWheelBound, { passive: false });

if (this._shouldSubscribeMouseWheel(this._options)) {
this._setMouseWheelEventListener(true);
}
this._model = new ChartModel(
this._invalidateHandler.bind(this),
this._options
Expand Down Expand Up @@ -129,7 +130,7 @@ export class ChartWidget implements IDestroyable {
}

public destroy(): void {
this._element.removeEventListener('wheel', this._onWheelBound);
this._setMouseWheelEventListener(false);
if (this._drawRafId !== 0) {
window.cancelAnimationFrame(this._drawRafId);
}
Expand Down Expand Up @@ -200,10 +201,18 @@ export class ChartWidget implements IDestroyable {
}

public applyOptions(options: DeepPartial<ChartOptionsInternal>): void {
const currentlyHasMouseWheelListener = this._shouldSubscribeMouseWheel(this._options);
const shouldHaveMouseWheelListener = this._shouldSubscribeMouseWheel(options);

// we don't need to merge options here because it's done in chart model
// and since both model and widget share the same object it will be done automatically for widget as well
// not ideal solution for sure, but it work's for now ¯\_(ツ)_/¯
this._model.applyOptions(options);

if (shouldHaveMouseWheelListener !== currentlyHasMouseWheelListener) {
this._setMouseWheelEventListener(shouldHaveMouseWheelListener);
}

this._updateTimeAxisVisibility();

const width = options.width || this._width;
Expand Down Expand Up @@ -406,6 +415,18 @@ export class ChartWidget implements IDestroyable {
}
}

private _shouldSubscribeMouseWheel(options: DeepPartial<ChartOptionsInternal>): boolean {
return Boolean(options?.handleScroll?.mouseWheel || options?.handleScale?.mouseWheel);
}

private _setMouseWheelEventListener(add: boolean): void {
if (add) {
this._element.addEventListener('wheel', this._onWheelBound, { passive: false });
return;
}
this._element.removeEventListener('wheel', this._onWheelBound);
}

private _onMousewheel(event: WheelEvent): void {
let deltaX = event.deltaX / 100;
let deltaY = -(event.deltaY / 100);
Expand Down

0 comments on commit 3992003

Please sign in to comment.