Skip to content

Commit

Permalink
Split StatefulMafsGraph and MafsGraph into separate files (#1397)
Browse files Browse the repository at this point in the history 
Each of these components used different helper functions and types defined
alongside them.  I've moved each component into a separate file along with its
associated functions and types. This makes the files more cohesive, so it's
easier to see which groups of entities go together.

Issue: none

## Test plan:
`yarn test`

Author: benchristel

Reviewers: handeyeco, jeremywiebe, mark-fitzgerald, Myranae, nishasy, SonicScrewdriver, nicolecomputer

Required Reviewers:

Approved By: handeyeco

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), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Jest Coverage (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: #1397
  • Loading branch information
@benchristel
benchristel authored Jul 9, 2024
1 parent fa19dbc commit 3108f93
Show file tree
Hide file tree
Showing 6 changed files with 299 additions and 281 deletions.
5 changes: 5 additions & 0 deletions .changeset/smooth-rice-speak.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Internal: split `MafsGraph` and `StatefulMafsGraph` into separate files.
2 changes: 1 addition & 1 deletion packages/perseus/src/widgets/interactive-graphs/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export {StatefulMafsGraph} from "./mafs-graph";
export {StatefulMafsGraph} from "./stateful-mafs-graph";
126 changes: 2 additions & 124 deletions packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import invariant from "tiny-invariant";
import {testDependencies} from "../../../../../testing/test-dependencies";
import * as Dependencies from "../../dependencies";

import {MafsGraph, StatefulMafsGraph} from "./mafs-graph";
import {MafsGraph} from "./mafs-graph";
import {movePoint} from "./reducer/interactive-graph-action";
import {interactiveGraphReducer} from "./reducer/interactive-graph-reducer";

import type {MafsGraphProps, StatefulMafsGraphProps} from "./mafs-graph";
import type {MafsGraphProps} from "./mafs-graph";
import type {InteractiveGraphState} from "./types";
import type {GraphRange} from "../../perseus-types";
import type {UserEvent} from "@testing-library/user-event";
Expand Down Expand Up @@ -40,26 +40,6 @@ function getBaseMafsGraphProps(): MafsGraphProps {
};
}

function getBaseStatefulMafsGraphProps(): StatefulMafsGraphProps {
return {
box: [400, 400],
step: [1, 1],
snapStep: [1, 1],
gridStep: [1, 1],
range: [
[-10, 10],
[-10, 10],
],
markings: "graph",
containerSizeClass: "small",
onChange: () => {},
showTooltips: false,
showProtractor: false,
labels: ["x", "y"],
graph: {type: "segment"},
};
}

function createFakeStore<S, A>(reducer: (state: S, action: A) => S, state: S) {
return {
dispatch(action: A) {
Expand All @@ -72,108 +52,6 @@ function createFakeStore<S, A>(reducer: (state: S, action: A) => S, state: S) {
};
}

describe("StatefulMafsGraph", () => {
let userEvent: UserEvent;
beforeEach(() => {
jest.spyOn(Dependencies, "getDependencies").mockReturnValue(
testDependencies,
);
userEvent = userEventLib.setup({
advanceTimers: jest.advanceTimersByTime,
});
});

it("renders", () => {
const {container} = render(
<StatefulMafsGraph {...getBaseStatefulMafsGraphProps()} />,
);

// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
const movablePoints = container.querySelectorAll(
"circle.movable-point-hitbox",
);

expect(movablePoints).not.toBe(0);
});

it("calls onChange when using graph", async () => {
const mockChangeHandler = jest.fn();

render(
<StatefulMafsGraph
{...getBaseStatefulMafsGraphProps()}
onChange={mockChangeHandler}
/>,
);

await userEvent.tab();
await userEvent.keyboard("{arrowup}");

expect(mockChangeHandler).toHaveBeenCalled();
});

it("re-renders when the graph type changes", () => {
// Arrange: render a segment graph
const segmentGraphProps: StatefulMafsGraphProps = {
...getBaseStatefulMafsGraphProps(),
graph: {type: "segment"},
};
const {rerender} = render(<StatefulMafsGraph {...segmentGraphProps} />);

// Act: rerender with a quadratic graph
const quadraticGraphProps: StatefulMafsGraphProps = {
...getBaseStatefulMafsGraphProps(),
graph: {type: "quadratic"},
};
rerender(<StatefulMafsGraph {...quadraticGraphProps} />);

// Assert: there should be 3 movable points (which define the quadratic
// function). If there are 2 points, it means we are still rendering
// the segment graph.
expect(screen.getAllByTestId("movable-point").length).toBe(3);
});

it("re-renders when the number of line segments on a segment graph changes", () => {
// Arrange: render a segment graph with one segment
const oneSegmentProps: StatefulMafsGraphProps = {
...getBaseStatefulMafsGraphProps(),
graph: {type: "segment", numSegments: 1},
};
const {rerender} = render(<StatefulMafsGraph {...oneSegmentProps} />);

// Act: rerender with two segments
const twoSegmentProps: StatefulMafsGraphProps = {
...getBaseStatefulMafsGraphProps(),
graph: {type: "segment", numSegments: 2},
};
rerender(<StatefulMafsGraph {...twoSegmentProps} />);

// Assert: there should be 4 movable points. If there are 2 points, it
// 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(
point: vec.Vector2,
range: GraphRange = [
Expand Down
190 changes: 34 additions & 156 deletions packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import {useLatestRef, View} from "@khanacademy/wonder-blocks-core";
import {View} from "@khanacademy/wonder-blocks-core";
import {UnreachableCaseError} from "@khanacademy/wonder-stuff-core";
import {Mafs} from "mafs";
import * as React from "react";
import {useEffect, useImperativeHandle, useRef} from "react";

import AxisLabels from "./axis-labels";
import GraphLockedLayer from "./graph-locked-layer";
Expand All @@ -24,168 +23,15 @@ import {Grid} from "./grid";
import {LegacyGrid} from "./legacy-grid";
import {X, Y} from "./math";
import {Protractor} from "./protractor";
import {initializeGraphState} from "./reducer/initialize-graph-state";
import {
changeRange,
changeSnapStep,
reinitialize,
type InteractiveGraphAction,
} from "./reducer/interactive-graph-action";
import {interactiveGraphReducer} from "./reducer/interactive-graph-reducer";
import {getGradableGraph, getRadius} from "./reducer/interactive-graph-state";
import {type InteractiveGraphAction} from "./reducer/interactive-graph-action";
import {GraphConfigContext} from "./reducer/use-graph-config";

import type {InteractiveGraphState, InteractiveGraphProps} from "./types";
import type {PerseusGraphType} from "../../perseus-types";
import type {Widget} from "../../renderer";
import type {vec} from "mafs";

import "mafs/core.css";
import "./mafs-styles.css";

export type StatefulMafsGraphProps = {
box: [number, number];
backgroundImage?: InteractiveGraphProps["backgroundImage"];
graph: PerseusGraphType;
lockedFigures?: InteractiveGraphProps["lockedFigures"];
range: InteractiveGraphProps["range"];
snapStep: InteractiveGraphProps["snapStep"];
step: InteractiveGraphProps["step"];
gridStep: InteractiveGraphProps["gridStep"];
containerSizeClass: InteractiveGraphProps["containerSizeClass"];
markings: InteractiveGraphProps["markings"];
onChange: InteractiveGraphProps["onChange"];
showTooltips: Required<InteractiveGraphProps["showTooltips"]>;
showProtractor: boolean;
labels: InteractiveGraphProps["labels"];
};

type MafsChange = {
graph: InteractiveGraphState;
};

const renderGraph = (props: {
state: InteractiveGraphState;
dispatch: (action: InteractiveGraphAction) => unknown;
}) => {
const {state, dispatch} = props;
const {type} = state;
switch (type) {
case "angle":
return <AngleGraph graphState={state} dispatch={dispatch} />;
case "segment":
return <SegmentGraph graphState={state} dispatch={dispatch} />;
case "linear-system":
return <LinearSystemGraph graphState={state} dispatch={dispatch} />;
case "linear":
return <LinearGraph graphState={state} dispatch={dispatch} />;
case "ray":
return <RayGraph graphState={state} dispatch={dispatch} />;
case "polygon":
return <PolygonGraph graphState={state} dispatch={dispatch} />;
case "point":
return <PointGraph graphState={state} dispatch={dispatch} />;
case "circle":
return <CircleGraph graphState={state} dispatch={dispatch} />;
case "quadratic":
return <QuadraticGraph graphState={state} dispatch={dispatch} />;
case "sinusoid":
return <SinusoidGraph graphState={state} dispatch={dispatch} />;
default:
return new UnreachableCaseError(type);
}
};

// Rather than be tightly bound to how data was structured in
// the legacy interactive graph, this lets us store state
// however we want and we just transform it before handing it off
// the the parent InteractiveGraph
function mafsStateToInteractiveGraph(state: MafsChange) {
if (state.graph.type === "circle") {
return {
...state,
graph: {
...state.graph,
radius: getRadius(state.graph),
},
};
}
return {
...state,
};
}

export const StatefulMafsGraph = React.forwardRef<
Partial<Widget>,
StatefulMafsGraphProps
>((props, ref) => {
const {onChange, graph} = props;

const [state, dispatch] = React.useReducer(
interactiveGraphReducer,
props,
initializeGraphState,
);

useImperativeHandle(ref, () => ({
getUserInput: () => getGradableGraph(state, graph),
}));

const prevState = useRef<InteractiveGraphState>(state);

useEffect(() => {
if (prevState.current !== state) {
onChange(mafsStateToInteractiveGraph({graph: state}));
}
prevState.current = state;
}, [onChange, state]);

// Destructuring first to keep useEffect from making excess calls
const [xSnap, ySnap] = props.snapStep;
useEffect(() => {
dispatch(changeSnapStep([xSnap, ySnap]));
}, [dispatch, xSnap, ySnap]);

// Destructuring first to keep useEffect from making excess calls
const [[xMinRange, xMaxRange], [yMinRange, yMaxRange]] = props.range;
useEffect(() => {
dispatch(
changeRange([
[xMinRange, xMaxRange],
[yMinRange, yMaxRange],
]),
);
}, [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(() => {
// This conditional prevents the state from being "reinitialized" right
// after the first render. This is an optimization, but also prevents
// a bug where the graph would be marked "incorrect" during grading
// even if the user never interacted with it.
if (latestPropsRef.current !== originalPropsRef.current) {
dispatch(reinitialize(latestPropsRef.current));
}
}, [
graph.type,
numSegments,
numSides,
snapTo,
showAngles,
showSides,
latestPropsRef,
]);

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

export type MafsGraphProps = {
box: [number, number];
backgroundImage?: InteractiveGraphProps["backgroundImage"];
Expand Down Expand Up @@ -291,3 +137,35 @@ export const MafsGraph = (props: MafsGraphProps) => {
</GraphConfigContext.Provider>
);
};

const renderGraph = (props: {
state: InteractiveGraphState;
dispatch: (action: InteractiveGraphAction) => unknown;
}) => {
const {state, dispatch} = props;
const {type} = state;
switch (type) {
case "angle":
return <AngleGraph graphState={state} dispatch={dispatch} />;
case "segment":
return <SegmentGraph graphState={state} dispatch={dispatch} />;
case "linear-system":
return <LinearSystemGraph graphState={state} dispatch={dispatch} />;
case "linear":
return <LinearGraph graphState={state} dispatch={dispatch} />;
case "ray":
return <RayGraph graphState={state} dispatch={dispatch} />;
case "polygon":
return <PolygonGraph graphState={state} dispatch={dispatch} />;
case "point":
return <PointGraph graphState={state} dispatch={dispatch} />;
case "circle":
return <CircleGraph graphState={state} dispatch={dispatch} />;
case "quadratic":
return <QuadraticGraph graphState={state} dispatch={dispatch} />;
case "sinusoid":
return <SinusoidGraph graphState={state} dispatch={dispatch} />;
default:
return new UnreachableCaseError(type);
}
};
Loading

0 comments on commit 3108f93

Please sign in to comment.