Skip to content

Commit

Permalink
Reset interactive graph state when Polygon's options change (#1364)
Browse files Browse the repository at this point in the history
## Summary:
The right-side preview in the exercise editor did not reflect changes to the number of sides in a polygon. Here we reinitialize graph state when the props for number of sides changes as that was not happening previously. 

Manual Testing in the ZND:

https://github.com/Khan/perseus/assets/60857422/c4f42f42-fbc4-4805-b5f7-5059408253c2



Issue: LEMS-2106

## Test plan:
- In a [ZND](https://prod-znd-240624-tamara-b.khanacademy.org/devadmin/content/courses/x6df8cbb5b9ebc9c6?lang=en), navigate to the exercise editor. Add an interactive graph widget. Change the graph type to Polygon. Confirm changing the number of sides changes the preview on the right.
- Confirm all tests pass
- Confirm all checks pass

Author: Myranae

Reviewers: nishasy, benchristel

Required Reviewers:

Approved By: nishasy, benchristel

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), 🚫 Upload Coverage, ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), 🚫 Check builds for changes in size (ubuntu-latest, 20.x), 🚫 Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1364
  • Loading branch information
Myranae authored Jun 25, 2024
1 parent 9615106 commit 35651e0
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/calm-llamas-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Fix a bug in the exercise editor where the preview did not update after a change to the number of sides in polygon graphs
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,7 @@ export const polygonQuestionDefaultCorrect: PerseusRenderer = {
showProtractor: false,
graph: {
showSides: true,
showAngles: false,
snapTo: "grid",
type: "polygon",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,26 @@ describe("StatefulMafsGraph", () => {
// means we are still rendering a single segment.
expect(screen.getAllByTestId("movable-point").length).toBe(4);
});

it("re-renders when the number of sides on a polygon graph changes", () => {
// Arrange: render a polygon graph with three sides
const threeSidesProps: StatefulMafsGraphProps = {
...getBaseStatefulMafsGraphProps(),
graph: {type: "polygon", numSides: 3},
};
const {rerender} = render(<StatefulMafsGraph {...threeSidesProps} />);

// Act: rerender with four sides
const fourSidesProps: StatefulMafsGraphProps = {
...getBaseStatefulMafsGraphProps(),
graph: {type: "polygon", numSides: 4},
};
rerender(<StatefulMafsGraph {...fourSidesProps} />);

// Assert: there should be 4 movable points. If there are 3 points, it
// means we are still rendering only 3 sides.
expect(screen.getAllByTestId("movable-point").length).toBe(4);
});
});

function graphToPixel(
Expand Down
15 changes: 14 additions & 1 deletion packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ export const StatefulMafsGraph = React.forwardRef<
}, [dispatch, xMinRange, xMaxRange, yMinRange, yMaxRange]);

const numSegments = graph.type === "segment" ? graph.numSegments : null;
const numSides = graph.type === "polygon" ? graph.numSides : null;
const snapTo = graph.type === "polygon" ? graph.snapTo : null;
const showAngles = graph.type === "polygon" ? graph.showAngles : null;
const showSides = graph.type === "polygon" ? graph.showSides : null;

const originalPropsRef = useRef(props);
const latestPropsRef = useLatestRef(props);
useEffect(() => {
Expand All @@ -162,7 +167,15 @@ export const StatefulMafsGraph = React.forwardRef<
if (latestPropsRef.current !== originalPropsRef.current) {
dispatch(reinitialize(latestPropsRef.current));
}
}, [graph.type, numSegments, latestPropsRef]);
}, [
graph.type,
numSegments,
numSides,
snapTo,
showAngles,
showSides,
latestPropsRef,
]);

return <MafsGraph {...props} state={state} dispatch={dispatch} />;
});
Expand Down

0 comments on commit 35651e0

Please sign in to comment.