Skip to content

Commit

Permalink
[Hint Mode: Start Coords] Add start coords UI for polygon graphs (sna…
Browse files Browse the repository at this point in the history
…p to grid only) (#1488)

## Summary:
Add the UI to specify start coords for Polygon graph type (not unlimited, snap to grid only)

- Add the polygon graph type go start-coords-settings.tsx
  - No need to create a separate start-coord-polygon.tsx file because
    it's identical to start-coord-point.tsx. Reusing that component.
- Update the selection logic in interactive-graph-editor.tsx so it
  properly updates the graph with the new snapTo value. (It was stuck
  on the previously selected value before.)
- Add the start coords UI polygon flag
- Update the flag tests so they actually test what they're supposed to

Issue: https://khanacademy.atlassian.net/browse/LEMS-2072

## Test plan:
`yarn jest`

Storybook
- Polygon story without default graph settings
  - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-polygon
- For default graph settings, go to a different story and select polygon from the dropdown
  - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-point

<img width="379" alt="Screenshot 2024-08-05 at 5 29 35 PM" src="https://github.com/user-attachments/assets/712d2114-4644-46b2-a693-73c788564ae7">

Author: nishasy

Reviewers: nishasy, Myranae

Required Reviewers:

Approved By: Myranae

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 builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1488
  • Loading branch information
nishasy authored Aug 6, 2024
1 parent 6f20129 commit 0bec013
Show file tree
Hide file tree
Showing 11 changed files with 235 additions and 7 deletions.
6 changes: 6 additions & 0 deletions .changeset/unlucky-lamps-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-editor": minor
---

[Hint Mode: Start Coords] Add start coords UI for polygon graphs (snap to grid only)
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const flags = {
"start-coords-ui-phase-1": true,
"start-coords-ui-phase-2": true,
"start-coords-ui-point": true,
"start-coords-ui-polygon": true,
},
} satisfies APIOptions["flags"];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,4 +606,90 @@ describe("StartCoordSettings", () => {
},
);
});

describe("polygon graph", () => {
test("shows the start coordinates UI: 3 sides (default)", () => {
// Arrange

// Act
render(
<StartCoordsSettings
{...defaultProps}
type="polygon"
onChange={() => {}}
/>,
{wrapper: RenderStateRoot},
);

// Assert
expect(screen.getByText("Start coordinates")).toBeInTheDocument();
expect(screen.getByText("Point 1:")).toBeInTheDocument();
expect(screen.getByText("Point 2:")).toBeInTheDocument();
expect(screen.getByText("Point 3:")).toBeInTheDocument();
});

test("shows the start coordinates UI: 6 sides", () => {
// Arrange

// Act
render(
<StartCoordsSettings
{...defaultProps}
type="polygon"
numSides={6}
onChange={() => {}}
/>,
{wrapper: RenderStateRoot},
);

// Assert
expect(screen.getByText("Start coordinates")).toBeInTheDocument();
expect(screen.getByText("Point 1:")).toBeInTheDocument();
expect(screen.getByText("Point 2:")).toBeInTheDocument();
expect(screen.getByText("Point 3:")).toBeInTheDocument();
expect(screen.getByText("Point 4:")).toBeInTheDocument();
expect(screen.getByText("Point 5:")).toBeInTheDocument();
expect(screen.getByText("Point 6:")).toBeInTheDocument();
});

test.each`
pointIndex | coord
${0} | ${"x"}
${0} | ${"y"}
${1} | ${"x"}
${1} | ${"y"}
${2} | ${"x"}
${2} | ${"y"}
`(
"calls onChange when $coord coord is changed (line $pointIndex)",
async ({pointIndex, coord}) => {
// Arrange
const onChangeMock = jest.fn();
render(
<StartCoordsSettings
{...defaultProps}
type="polygon"
onChange={onChangeMock}
/>,
);

// Act
const input = screen.getAllByRole("spinbutton", {
name: `${coord}`,
})[pointIndex];
await userEvent.clear(input);
await userEvent.type(input, "101");

// Assert
const expectedCoords = [
[3, -2],
[0, 4],
[-3, -2],
];
expectedCoords[pointIndex][coord === "x" ? 0 : 1] = 101;

expect(onChangeMock).toHaveBeenLastCalledWith(expectedCoords);
},
);
});
});
39 changes: 39 additions & 0 deletions packages/perseus-editor/src/components/__tests__/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,45 @@ describe("getDefaultGraphStartCoords", () => {
[5, 0],
]);
});

test("should get default start coords for a polygon graph, triangle (default)", () => {
// Arrange
const graph: PerseusGraphType = {type: "polygon"};
const range = [
[-10, 10],
[-10, 10],
] satisfies [Range, Range];
const step = [1, 1] satisfies [number, number];

// Act
const defaultCoords = getDefaultGraphStartCoords(graph, range, step);

expect(defaultCoords).toEqual([
[3, -2],
[0, 4],
[-3, -2],
]);
});

test("should get default start coords for a polygon graph, square", () => {
// Arrange
const graph: PerseusGraphType = {type: "polygon", numSides: 4};
const range = [
[-10, 10],
[-10, 10],
] satisfies [Range, Range];
const step = [1, 1] satisfies [number, number];

// Act
const defaultCoords = getDefaultGraphStartCoords(graph, range, step);

expect(defaultCoords).toEqual([
[3, -3],
[3, 3],
[-3, 3],
[-3, -3],
]);
});
});

describe("getSinusoidEquation", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
getLineCoords,
getLinearSystemCoords,
getPointCoords,
getPolygonCoords,
getQuadraticCoords,
getSegmentCoords,
getSinusoidCoords,
Expand Down Expand Up @@ -91,8 +92,13 @@ const StartCoordsSettingsInner = (props: Props) => {
onChange={onChange}
/>
);
// Graphs with startCoords of type ReadonlyArray<Coord>
case "point":
const pointCoords = getPointCoords(props, range, step);
case "polygon":
const pointCoords =
type === "point"
? getPointCoords(props, range, step)
: getPolygonCoords(props, range, step);
return (
<StartCoordsPoint
startCoords={pointCoords}
Expand Down
19 changes: 19 additions & 0 deletions packages/perseus-editor/src/components/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
getLineCoords,
getLinearSystemCoords,
getPointCoords,
getPolygonCoords,
getQuadraticCoords,
getSegmentCoords,
getSinusoidCoords,
Expand Down Expand Up @@ -201,6 +202,12 @@ export function getDefaultGraphStartCoords(
range,
step,
);
case "polygon":
return getPolygonCoords(
{...graph, startCoords: undefined},
range,
step,
);
default:
return undefined;
}
Expand Down Expand Up @@ -275,6 +282,7 @@ export const shouldShowStartCoordsUI = (flags, graph) => {
const startCoordsPhase1 = flags?.mafs?.["start-coords-ui-phase-1"];
const startCoordsPhase2 = flags?.mafs?.["start-coords-ui-phase-2"];
const startCoordsPoint = flags?.mafs?.["start-coords-ui-point"];
const startCoordsPolygon = flags?.mafs?.["start-coords-ui-polygon"];

if (startCoordsPhase1 && startCoordsUiPhase1Types.includes(graph.type)) {
return true;
Expand All @@ -292,5 +300,16 @@ export const shouldShowStartCoordsUI = (flags, graph) => {
return true;
}

if (
startCoordsPolygon &&
graph.type === "polygon" &&
graph.numPoints !== "unlimited" &&
// Pre-initialized graph with undefined snapTo value
// initializes to snapTo="grid"
(graph.snapTo === "grid" || graph.snapTo === undefined)
) {
return true;
}

return false;
};
Original file line number Diff line number Diff line change
Expand Up @@ -700,9 +700,10 @@ describe("InteractiveGraphEditor", () => {
...flags,
mafs: {
...flags.mafs,
"start-coords-ui-phase-1": shouldRender,
"start-coords-ui-phase-1": true,
"start-coords-ui-phase-2": false,
"start-coords-ui-point": false,
"start-coords-ui-polygon": false,
},
},
}}
Expand Down Expand Up @@ -738,7 +739,7 @@ describe("InteractiveGraphEditor", () => {
${"linear-system"} | ${false}
${"segment"} | ${false}
${"circle"} | ${false}
${"quadratic"} | ${false}
${"quadratic"} | ${true}
${"sinusoid"} | ${true}
${"polygon"} | ${false}
${"angle"} | ${false}
Expand All @@ -759,8 +760,9 @@ describe("InteractiveGraphEditor", () => {
mafs: {
...flags.mafs,
"start-coords-ui-phase-1": false,
"start-coords-ui-phase-2": shouldRender,
"start-coords-ui-phase-2": true,
"start-coords-ui-point": false,
"start-coords-ui-polygon": false,
},
},
}}
Expand Down Expand Up @@ -818,7 +820,67 @@ describe("InteractiveGraphEditor", () => {
...flags.mafs,
"start-coords-ui-phase-1": false,
"start-coords-ui-phase-2": false,
"start-coords-ui-point": shouldRender,
"start-coords-ui-point": true,
"start-coords-ui-polygon": false,
},
},
}}
graph={{type}}
correct={{type}}
/>,
{
wrapper: RenderStateRoot,
},
);

// Assert
if (shouldRender) {
expect(
await screen.findByRole("button", {
name: "Use default start coordinates",
}),
).toBeInTheDocument();
} else {
expect(
screen.queryByRole("button", {
name: "Use default start coordinates",
}),
).toBeNull();
}
},
);

test.each`
type | shouldRender
${"linear"} | ${false}
${"ray"} | ${false}
${"linear-system"} | ${false}
${"segment"} | ${false}
${"circle"} | ${false}
${"quadratic"} | ${false}
${"sinusoid"} | ${false}
${"polygon"} | ${true}
${"angle"} | ${false}
${"point"} | ${false}
`(
"should render for $type graphs if polygon flag is on: $shouldRender",
async ({type, shouldRender}) => {
// Arrange

// Act
render(
<InteractiveGraphEditor
{...baseProps}
apiOptions={{
...ApiOptions.defaults,
flags: {
...flags,
mafs: {
...flags.mafs,
"start-coords-ui-phase-1": false,
"start-coords-ui-phase-2": false,
"start-coords-ui-point": false,
"start-coords-ui-polygon": true,
},
},
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,10 @@ class InteractiveGraphEditor extends React.Component<Props> {
coords: null,
};

this.props.onChange({correct: graph});
this.props.onChange({
correct: graph,
graph: graph,
});
}}
style={styles.singleSelectShort}
>
Expand Down
1 change: 1 addition & 0 deletions packages/perseus/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export {
getLineCoords,
getLinearSystemCoords,
getPointCoords,
getPolygonCoords,
getSegmentCoords,
getSinusoidCoords,
getQuadraticCoords,
Expand Down
5 changes: 5 additions & 0 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ export const InteractiveGraphEditorFlags = [
* Includes point graph.
*/
"start-coords-ui-point",
/**
* Enables the UI for setting the start coordinates of a graph.
* Includes polygon graph.
*/
"start-coords-ui-polygon",
] as const;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export function getLinearSystemCoords(
);
}

function getPolygonCoords(
export function getPolygonCoords(
graph: PerseusGraphTypePolygon,
range: [x: Interval, y: Interval],
step: [x: number, y: number],
Expand Down

0 comments on commit 0bec013

Please sign in to comment.