Skip to content

Commit

Permalink
[Interactive Graph Editor] Add UI for updating starting coordinates (…
Browse files Browse the repository at this point in the history
…linear and ray) (#1382)

## Summary:
Implement a UI in the interactive graph editor to allow conten authors
to set custom starting coordinates for interactive graphs. This is a
prerequisite for hint mode.

Note: There was some funky stuff happening when I tried using `coords`.
I added a `startCoords` field to the graph types, and that seemed to fix
all my issues. This change is included in this PR.

Included in this PR
- The creation of the StartCoordSettings component
- The flag that this feature is behind
- The UI itself, only for linear and ray graphs
- Added `startCoords` field

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

## Test plan:
Storybook existing starting coords
- http://localhost:6006/?path=/story/perseuseditor-editorpage--interactive-graph-linear-with-starting-coords
- Confirm that the UI reflects the custom coords
- Change the coords
- Confirm that the graph changes
- Change the graph type to Ray
- Confirm that the coords change to the ray initial state coords
- Confirm that the "correct" preview is still interact-able and does not interfere with the start coords preview 
- Press "Use default start coords" and confirm it uses the start coords

Storybook new graph
- http://localhost:6006/?path=/story/perseuseditor-editorpage--demo
- Add a new interactive graph
- Select the linear graph type
- Confirm that it sets the default coords in the UI
- Confirm that the "correct" preview is still interact-able and does not interfere with the start coords preview

Copy/pasting
- Press the "+ Add a hint" button at the bottom of the page
- Copy/paste the interactive graph ("[[interactive-graph 1]]")
  into the hint text box
- Confirm that the graph is copied correctly with the same
  start coords
- Confirm that the "correct" preview is still interact-able and does not interfere with the start coords preview
- Press "Use default start coords" and confirm it uses the start coords


![Screenshot 2024-06-28 at 5 18 33 PM](https://github.com/Khan/perseus/assets/13231763/90ba9c4c-6029-4684-8701-90d38b655a15)

Author: nishasy

Reviewers: nishasy, catandthemachines, benchristel, jeremywiebe, SonicScrewdriver, Myranae

Required Reviewers:

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

Pull Request URL: #1382
  • Loading branch information
nishasy authored Jul 15, 2024
1 parent a8a1a0f commit f392dcf
Show file tree
Hide file tree
Showing 14 changed files with 419 additions and 19 deletions.
6 changes: 6 additions & 0 deletions .changeset/chilly-planets-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-editor": minor
---

[Interactive Graph Editor] Implement UI to edit start coordinates for linear and ray graphs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const flags = {
linear: true,
"linear-system": true,
ray: true,
"start-coords-ui": true,
"interactive-graph-locked-features-m2": true,
"interactive-graph-locked-features-m2b": true,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export const MafsWithLockedFiguresCurrent = (): React.ReactElement => {
flags: {
mafs: {
...flags.mafs,
"start-coords-ui": false,
"interactive-graph-locked-features-m2": false,
"interactive-graph-locked-features-m2b": false,
},
Expand All @@ -148,6 +149,7 @@ export const MafsWithLockedFiguresM2Flag = (): React.ReactElement => {
flags: {
mafs: {
...flags.mafs,
"start-coords-ui": false,
"interactive-graph-locked-features-m2": true,
"interactive-graph-locked-features-m2b": false,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
import {Dependencies} from "@khanacademy/perseus";
import {render, screen} from "@testing-library/react";
import {userEvent as userEventLib} from "@testing-library/user-event";
import * as React from "react";

import {testDependencies} from "../../../../../testing/test-dependencies";
import StartCoordSettings from "../start-coord-settings";

import type {Range} from "@khanacademy/perseus";

const defaultProps = {
range: [
[-10, 10],
[-10, 10],
] satisfies [Range, Range],
step: [1, 1] satisfies [number, number],
};
describe("StartCoordSettings", () => {
let userEvent;
beforeEach(() => {
userEvent = userEventLib.setup({
advanceTimers: jest.advanceTimersByTime,
});

jest.spyOn(Dependencies, "getDependencies").mockReturnValue(
testDependencies,
);
});

test("clicking the heading toggles the settings", async () => {
// Arrange

// Act
render(
<StartCoordSettings
{...defaultProps}
type="linear"
onChange={() => {}}
/>,
);

const heading = screen.getByText("Start coordinates");

// Assert
expect(screen.getByText("Point 1")).toBeInTheDocument();
expect(screen.getByText("Point 2")).toBeInTheDocument();

await userEvent.click(heading);

expect(screen.queryByText("Point 1")).not.toBeInTheDocument();
expect(screen.queryByText("Point 2")).not.toBeInTheDocument();

await userEvent.click(heading);

expect(screen.getByText("Point 1")).toBeInTheDocument();
expect(screen.getByText("Point 2")).toBeInTheDocument();
});

describe.each`
type
${"linear"}
${"ray"}
`(`graphs with CollinearTuple startCoords`, ({type}) => {
test(`shows the start coordinates UI for ${type}`, () => {
// Arrange

// Act
render(
<StartCoordSettings
{...defaultProps}
type={type}
onChange={() => {}}
/>,
);

const resetButton = screen.getByRole("button", {
name: "Use default start coords",
});

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

test.each`
segmentIndex | coord
${0} | ${"x"}
${0} | ${"y"}
${1} | ${"x"}
${1} | ${"y"}
`(
`calls onChange when $coord coord is changed (segment $segmentIndex) for type ${type}`,
async ({segmentIndex, coord}) => {
// Arrange
const onChangeMock = jest.fn();

// Act
render(
<StartCoordSettings
{...defaultProps}
type={type}
onChange={onChangeMock}
/>,
);

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

const expectedCoords = [
[-5, 5],
[5, 5],
];
expectedCoords[segmentIndex][coord === "x" ? 0 : 1] = 101;

expect(onChangeMock).toHaveBeenLastCalledWith(expectedCoords);
},
);

test(`calls onChange when reset button is clicked for type ${type}`, async () => {
// Arrange
const onChangeMock = jest.fn();

// Act
render(
<StartCoordSettings
{...defaultProps}
startCoords={[
[-15, 15],
[15, 15],
]}
type={type}
onChange={onChangeMock}
/>,
);

// Assert
const resetButton = screen.getByRole("button", {
name: "Use default start coords",
});
await userEvent.click(resetButton);

expect(onChangeMock).toHaveBeenLastCalledWith([
[-5, 5],
[5, 5],
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ const CoordinatePairInput = (props: Props) => {
coord[1].toString(),
]);

// Update the local state when the props change. (Such as when the graph
// type is changed, and the coordinates are reset.)
React.useEffect(() => {
setCoordState([coord[0].toString(), coord[1].toString()]);
}, [coord]);

function handleCoordChange(newValue, coordIndex) {
// Update the local state (update the input field value).
const newCoordState = [...coordState];
Expand Down
121 changes: 121 additions & 0 deletions packages/perseus-editor/src/components/start-coord-settings.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import {getLineCoords} from "@khanacademy/perseus";
import Button from "@khanacademy/wonder-blocks-button";
import {View} from "@khanacademy/wonder-blocks-core";
import {Strut} from "@khanacademy/wonder-blocks-layout";
import {color, spacing} from "@khanacademy/wonder-blocks-tokens";
import {LabelLarge} from "@khanacademy/wonder-blocks-typography";
import arrowCounterClockwise from "@phosphor-icons/core/bold/arrow-counter-clockwise-bold.svg";
import {StyleSheet} from "aphrodite";
import * as React from "react";

import CoordinatePairInput from "./coordinate-pair-input";
import Heading from "./heading";

import type {
PerseusGraphType,
Range,
CollinearTuple,
} from "@khanacademy/perseus";

type Props = PerseusGraphType & {
range: [x: Range, y: Range];
step: [x: number, y: number];
onChange: (startCoords: CollinearTuple) => void;
};

type PropsInner = {
type: PerseusGraphType["type"];
startCoords: CollinearTuple;
onChange: (startCoords: CollinearTuple) => void;
};

const StartCoordSettingsInner = (props: PropsInner) => {
const {type, startCoords, onChange} = props;

// Check if coords is of type CollinearTuple
switch (type) {
case "linear":
case "ray":
return (
<>
<View style={styles.tile}>
<LabelLarge>Point 1</LabelLarge>
<CoordinatePairInput
coord={startCoords[0]}
onChange={(value) =>
onChange([value, startCoords[1]])
}
/>
</View>
<View style={styles.tile}>
<LabelLarge>Point 2</LabelLarge>
<CoordinatePairInput
coord={startCoords[1]}
onChange={(value) =>
onChange([startCoords[0], value])
}
/>
</View>
</>
);
default:
return null;
}
};

const StartCoordSettings = (props: Props) => {
const {type, range, step, onChange} = props;
const [isOpen, setIsOpen] = React.useState(true);

if (type !== "linear" && type !== "ray") {
return null;
}

const defaultStartCoords = getLineCoords({type: type}, range, step);

return (
<View style={styles.container}>
{/* Heading for the collapsible section */}
<Heading
title="Start coordinates"
isOpen={isOpen}
onToggle={() => setIsOpen(!isOpen)}
/>

{/* Start coordinates main UI */}
{isOpen && (
<>
<StartCoordSettingsInner
type={type}
startCoords={props.startCoords ?? defaultStartCoords}
onChange={onChange}
/>

{/* Button to reset to default */}
<Strut size={spacing.small_12} />
<Button
startIcon={arrowCounterClockwise}
kind="tertiary"
size="small"
onClick={() => {
onChange(defaultStartCoords);
}}
>
Use default start coords
</Button>
</>
)}
</View>
);
};

const styles = StyleSheet.create({
tile: {
backgroundColor: color.fadedBlue8,
marginTop: spacing.xSmall_8,
padding: spacing.small_12,
borderRadius: spacing.xSmall_8,
},
});

export default StartCoordSettings;
Loading

0 comments on commit f392dcf

Please sign in to comment.