Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add relativeCoords average to PointerTracker #2939

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Add relativeCoords average to PointerTracker #2939

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions src/web/tools/PointerTracker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export default class PointerTracker {

private lastMovedPointerId: number;

private cachedAverages: { x: number; y: number } = { x: 0, y: 0 };
private cachedAbsoluteAverages: { x: number; y: number } = { x: 0, y: 0 };
private cachedRelativeAverages: { x: number; y: number } = { x: 0, y: 0 };

public constructor() {
this.lastMovedPointerId = NaN;
Expand Down Expand Up @@ -50,7 +51,8 @@ export default class PointerTracker {
this.trackedPointers.set(event.pointerId, newElement);
this.mapTouchEventId(event.pointerId);

this.cachedAverages = this.getAbsoluteCoordsAverage();
this.cachedAbsoluteAverages = this.getAbsoluteCoordsAverage();
this.cachedRelativeAverages = this.getRelativeCoordsAverage();
}

public removeFromTracker(pointerId: number): void {
Expand Down Expand Up @@ -80,7 +82,8 @@ export default class PointerTracker {

this.trackedPointers.set(event.pointerId, element);

this.cachedAverages = this.getAbsoluteCoordsAverage();
this.cachedAbsoluteAverages = this.getAbsoluteCoordsAverage();
this.cachedRelativeAverages = this.getRelativeCoordsAverage();
}

//Mapping TouchEvents ID
Expand Down Expand Up @@ -160,8 +163,22 @@ export default class PointerTracker {
const avgY = coordsSum.y / this.trackedPointers.size;

const averages = {
x: isNaN(avgX) ? this.cachedAverages.x : avgX,
y: isNaN(avgY) ? this.cachedAverages.y : avgY,
x: isNaN(avgX) ? this.cachedAbsoluteAverages.x : avgX,
y: isNaN(avgY) ? this.cachedAbsoluteAverages.y : avgY,
};

return averages;
}

public getRelativeCoordsAverage() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I see two options:

  • We can leave 2 methods, one for absolute and one for relative coords
  • We can merge them into one method

The second option could look like this:

public getAverageCoords(relative = false) {
  // calculate average
}

It can be done in different ways (default value, optional argument, required argument), but you get the point. Let me know what do you think. cc @j-piasecki, @latekvo

Copy link
Contributor

@latekvo latekvo Jun 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to follow the convention, right below there are getAbsoluteCoordsSum() and getRelativeCoordsSum() functions.

And while this creates a bit more duplicate code, I also think it's more readable than having a single function with an optional boolean argument.
Perhaps the second option would work fine with a positional argument or an enum, since these are more verbose, but i think it would be simpler to just go with the first option.

But of course it's best to wait for j-piasecki's opinion 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think keeping them separate makes sense. They are not that long and, unless I'm missing something, the only part that would apply to both cases would be calculating the average so we wouldn't get that much from merging the two.

const coordsSum = this.getRelativeCoordsSum();

const avgX = coordsSum.x / this.trackedPointers.size;
const avgY = coordsSum.y / this.trackedPointers.size;

const averages = {
x: isNaN(avgX) ? this.cachedRelativeAverages.x : avgX,
y: isNaN(avgY) ? this.cachedRelativeAverages.y : avgY,
};

return averages;
Expand Down
Loading