From 30f898c44954cf6f1d3d99038f476bd2038cb7c4 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Mon, 1 Jul 2024 14:31:20 -0700 Subject: [PATCH] [Interactive Graph Editor] Remove the use of graphKey for remounting (#1385) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: It turns out that the use of `graphKey` for remounting in `interactive-graph-editor.tsx` is redundant, since this is also being done by the `useEffect` in `mafs-graph.tsx`. `graphKey` was also causing a warning to show up in the logs, possibly from a remounting loop. Here, `graphKey` is being removed altogether to stop the remounting issue. [More info on Slack](https://khanacademy.slack.com/archives/C067UM1QAR4/p1719854190583499) Issue: none ## Test plan: Storybook - http://localhost:6006/?path=/story/perseuseditor-editorpage--demo - Add an interactive graph - Open the browser console - Move the graph in the "correct" preview (quickly! Don't hold for time before dragging) - Confirm the warning doesn't appear - Change properties of the graph (e.g. number of segments) - Confirm that the preview changes correctly - Repeat for every graph type Author: nishasy Reviewers: benchristel, mark-fitzgerald, Myranae, SonicScrewdriver Required Reviewers: Approved By: benchristel Checks: ✅ gerald, ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ✅ 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: https://github.com/Khan/perseus/pull/1385 --- .changeset/heavy-buckets-think.md | 5 +++++ .../interactive-graph-editor.test.tsx | 17 ----------------- .../src/widgets/interactive-graph-editor.tsx | 19 ------------------- 3 files changed, 5 insertions(+), 36 deletions(-) create mode 100644 .changeset/heavy-buckets-think.md diff --git a/.changeset/heavy-buckets-think.md b/.changeset/heavy-buckets-think.md new file mode 100644 index 0000000000..b72409a458 --- /dev/null +++ b/.changeset/heavy-buckets-think.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus-editor": patch +--- + +[Interactive Graph Editor] Remove the use of graphKey for remounting diff --git a/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx b/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx index 6aa0ac81bf..c39fcaf9e4 100644 --- a/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx +++ b/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx @@ -617,21 +617,4 @@ describe("InteractiveGraphEditor", () => { // Assert expect(ref.current?.getSaveWarnings()).toEqual([]); }); - - test("buildGraphKey returns the correct key", async () => { - // Arrange - const graph: PerseusGraphType = { - type: "polygon", - numSides: 4, - snapTo: "grid", - showAngles: true, - showSides: true, - }; - - // Act - const key = InteractiveGraphEditor.buildGraphKey(graph); - - // Assert - expect(key).toEqual("polygon:4:grid:true:true"); - }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor.tsx index ef853fae6b..3648302844 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor.tsx @@ -30,7 +30,6 @@ import type { PerseusInteractiveGraphWidgetOptions, APIOptionsWithDefaults, LockedFigure, - PerseusGraphType, } from "@khanacademy/perseus"; import type {PropsFor} from "@khanacademy/wonder-blocks-core"; @@ -199,20 +198,6 @@ class InteractiveGraphEditor extends React.Component { DeprecationMixin.UNSAFE_componentWillMount.call(this); } - static buildGraphKey(correct: PerseusGraphType) { - const testGraphKey: any[] = []; - for (const key in correct) { - if (correct[key]) { - typeof correct[key] === "number" || - typeof correct[key] === "string" || - typeof correct[key] === "boolean" - ? testGraphKey.push(correct[key]) - : testGraphKey.push(0); - } - } - return testGraphKey.join(":"); - } - render() { let graph; let equationString; @@ -262,9 +247,6 @@ class InteractiveGraphEditor extends React.Component { this.props.onChange({correct: correct}); }, } as const; - // This is used to force a remount of the graph component - // when there's a significant change - const graphKey = InteractiveGraphEditor.buildGraphKey(correct); graph = ( // There are a bunch of props that renderer.jsx passes to widgets via @@ -273,7 +255,6 @@ class InteractiveGraphEditor extends React.Component { // @ts-expect-error - TS2769 - No overload matches this call.