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.

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

Interactive Graph Bug Fix: Ensure that we're bounding and snapping BEFORE checking if the new destination results in a valid graph. #1356

Merged
merged 3 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/smooth-worms-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

A fix for performance issues related to Sinusoid and Quadratic graphs
Original file line number Diff line number Diff line change
Expand Up @@ -282,28 +282,38 @@ function doMovePoint(
};
}
case "sinusoid": {
// First, we need to verify that the new coordinates are not on the same vertical line
// If they are, we don't want to move the point
// First, we need to make sure to bound the new coordinates to the graph range
const destination = action.destination;
const boundDestination = boundAndSnapToGrid(destination, state);

// Then, we need to verify that the new coordinates are not on the same
// vertical line. If they are, then we don't want to move the point
const newCoords: vec.Vector2[] = [...state.coords];
newCoords[action.index] = action.destination;
newCoords[action.index] = boundDestination;
if (newCoords[0][0] === newCoords[1][0]) {
return state;
}

return {
...state,
hasBeenInteractedWith: true,
coords: setAtIndex({
array: state.coords,
index: action.index,
newValue: boundAndSnapToGrid(destination, state),
newValue: boundDestination,
}),
};
}
case "quadratic": {
// Set up the new coords and check if the quadratic coefficients are valid
const newCoords: QuadraticCoords = [...state.coords];
newCoords[action.index] = action.destination;

// Bind the new destination to the graph range/snapStep and then get the quadratic coefficients
const boundDestination = boundAndSnapToGrid(
action.destination,
state,
);
newCoords[action.index] = boundDestination;
const QuadraticCoefficients = getQuadraticCoefficients(newCoords);

// If the new destination results in an invalid quadratic equation, we don't want to move the point
Expand All @@ -317,7 +327,7 @@ function doMovePoint(
coords: setAtIndex({
array: state.coords,
index: action.index,
newValue: boundAndSnapToGrid(action.destination, state),
newValue: boundDestination,
}),
};
}
Expand Down
Loading