From 790e189a7fdcd215d78d1999879ab2fc7417e123 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 24 Sep 2024 10:30:52 -0700 Subject: [PATCH] [Locked Figure Labels] Add/edit/delete locked ellipse labels (#1655) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Add the ability to add/edit/delete labels for locked ellipses within the interactive graph editor. Issue: https://khanacademy.atlassian.net/browse/LEMS-2349 ## Test plan: - Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-ellipse-labels-flag - Go to locked figures - Open the locked ellipse settings - Confirm that the label settings are there - Play around with the label settings and confirm that it moves and updates with the ellipse image Author: nishasy Reviewers: catandthemachines, #perseus, benchristel, anakaren-rojas Required Reviewers: Approved By: catandthemachines Checks: ✅ codecov/patch, ✅ codecov/project, ✅ 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 builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1655 --- .changeset/dry-parents-invite.md | 6 + .../locked-ellipse-settings.test.tsx | 179 ++++++++++++++++++ .../locked-ellipse-settings.tsx | 110 ++++++++++- .../locked-figures/locked-line-settings.tsx | 2 +- .../locked-vector-settings.test.tsx | 5 +- .../locked-figures/locked-vector-settings.tsx | 2 +- 6 files changed, 293 insertions(+), 11 deletions(-) create mode 100644 .changeset/dry-parents-invite.md diff --git a/.changeset/dry-parents-invite.md b/.changeset/dry-parents-invite.md new file mode 100644 index 0000000000..4d8d8f87a2 --- /dev/null +++ b/.changeset/dry-parents-invite.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Locked Figure Labels] Add/edit/delete locked ellipse labels diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx index b54266bce7..3408c6396c 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.test.tsx @@ -3,18 +3,29 @@ import {render, screen} from "@testing-library/react"; import {userEvent as userEventLib} from "@testing-library/user-event"; import * as React from "react"; +import {flags} from "../../../__stories__/flags-for-api-options"; + import LockedEllipseSettings from "./locked-ellipse-settings"; import {getDefaultFigureForType} from "./util"; import type {UserEvent} from "@testing-library/user-event"; const defaultProps = { + flags: { + ...flags, + mafs: { + ...flags.mafs, + "locked-ellipse-settings": true, + }, + }, ...getDefaultFigureForType("ellipse"), onChangeProps: () => {}, onMove: () => {}, onRemove: () => {}, }; +const defaultLabel = getDefaultFigureForType("label"); + describe("LockedEllipseSettings", () => { let userEvent: UserEvent; beforeEach(() => { @@ -138,4 +149,172 @@ describe("LockedEllipseSettings", () => { // Assert expect(onToggle).toHaveBeenCalled(); }); + + describe("Labels", () => { + test("Updates the label coords when the ellipse center change", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const point1XInput = screen.getAllByLabelText("x coord")[0]; + // Change the x coord of the second point to 20 + await userEvent.type(point1XInput, "0"); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + center: [10, 1], + labels: [ + { + ...defaultLabel, + coord: [10, 1], + }, + ], + }); + }); + + test("Updates the label color when the ellipse color changes", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const colorSelect = screen.getByLabelText("color"); + await userEvent.click(colorSelect); + const colorOption = screen.getByText("pink"); + await userEvent.click(colorOption); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + color: "pink", + labels: [ + { + ...defaultLabel, + color: "pink", + }, + ], + }); + }); + + test("Updates the label when the label text changes", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const labelText = screen.getByLabelText("TeX"); + await userEvent.type(labelText, "!"); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + labels: [{...defaultLabel, text: "label text!"}], + }); + }); + + test("Removes label when delete button is clicked", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const deleteButton = screen.getByRole("button", { + name: "Delete locked label", + }); + await userEvent.click(deleteButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + labels: [], + }); + }); + + test("Adds a new label when the add label button is clicked", async () => { + // Arrange + const onChangeProps = jest.fn(); + render( + , + {wrapper: RenderStateRoot}, + ); + + // Act + const addLabelButton = screen.getByRole("button", { + name: "Add visible label", + }); + await userEvent.click(addLabelButton); + + // Assert + expect(onChangeProps).toHaveBeenCalledWith({ + labels: [ + { + ...defaultLabel, + text: "label text", + }, + { + ...defaultLabel, + // One unit down vertically from the first label. + coord: [0, -1], + }, + ], + }); + }); + }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx index c94578dede..8e327a6661 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx @@ -1,9 +1,11 @@ import {components, lockedFigureFillStyles} from "@khanacademy/perseus"; +import Button from "@khanacademy/wonder-blocks-button"; import {View} from "@khanacademy/wonder-blocks-core"; import {OptionItem, SingleSelect} from "@khanacademy/wonder-blocks-dropdown"; import {Strut} from "@khanacademy/wonder-blocks-layout"; -import {spacing} from "@khanacademy/wonder-blocks-tokens"; +import {spacing, color as wbColor} from "@khanacademy/wonder-blocks-tokens"; import {LabelMedium, LabelLarge} from "@khanacademy/wonder-blocks-typography"; +import plusCircle from "@phosphor-icons/core/regular/plus-circle.svg"; import {StyleSheet} from "aphrodite"; import * as React from "react"; @@ -15,6 +17,8 @@ import ColorSelect from "./color-select"; import EllipseSwatch from "./ellipse-swatch"; import LineStrokeSelect from "./line-stroke-select"; import LockedFigureSettingsActions from "./locked-figure-settings-actions"; +import LockedLabelSettings from "./locked-label-settings"; +import {getDefaultFigureForType} from "./util"; import type {LockedFigureSettingsCommonProps} from "./locked-figure-settings"; import type { @@ -22,6 +26,7 @@ import type { LockedFigureFillType, LockedEllipseType, LockedFigureColor, + LockedLabelType, } from "@khanacademy/perseus"; const {InfoTip} = components; @@ -36,10 +41,12 @@ export type Props = LockedFigureSettingsCommonProps & const LockedEllipseSettings = (props: Props) => { const { + flags, center, radius, angle, color, + labels, fillStyle, strokeStyle, expanded, @@ -49,10 +56,55 @@ const LockedEllipseSettings = (props: Props) => { onRemove, } = props; + function handleCenterChange(newCoord: Coord) { + const xOffset = newCoord[0] - center[0]; + const yOffset = newCoord[1] - center[1]; + + const newProps: Partial = { + center: newCoord, + }; + + // Update the coord by the same amount as the point for all labels + newProps.labels = labels.map((label) => ({ + ...label, + coord: [label.coord[0] + xOffset, label.coord[1] + yOffset], + })); + + onChangeProps(newProps); + } + function handleColorChange(newValue: LockedFigureColor) { - onChangeProps({color: newValue}); + const newProps: Partial = { + color: newValue, + }; + + // Update the color of the all labels to match the point + newProps.labels = labels.map((label) => ({ + ...label, + color: newValue, + })); + + onChangeProps(newProps); + } + + function handleLabelChange( + updatedLabel: LockedLabelType, + labelIndex: number, + ) { + const updatedLabels = [...labels]; + updatedLabels[labelIndex] = { + ...labels[labelIndex], + ...updatedLabel, + }; + + onChangeProps({labels: updatedLabels}); } + function handleLabelRemove(labelIndex: number) { + const updatedLabels = labels.filter((_, index) => index !== labelIndex); + + onChangeProps({labels: updatedLabels}); + } return ( { - onChangeProps({center: newCoords}) - } + onChange={handleCenterChange} /> @@ -147,6 +197,50 @@ const LockedEllipseSettings = (props: Props) => { } /> + {/* Visible Labels */} + {flags?.["mafs"]?.["locked-ellipse-labels"] && ( + <> + {labels.map((label, labelIndex) => ( + { + handleLabelChange(newLabel, labelIndex); + }} + onRemove={() => { + handleLabelRemove(labelIndex); + }} + containerStyle={styles.labelContainer} + /> + ))} + + + + )} + {/* Actions */} { const newLabel = { ...getDefaultFigureForType("label"), coord: labelLocation, - // Default to the same color as the point + // Default to the same color as the line color: lineColor, } satisfies LockedLabelType; diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx index 883bfbac00..52982bc5f5 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.test.tsx @@ -345,12 +345,9 @@ describe("Locked Vector Settings", () => { ); // Act - const addLabelButtons = screen.getAllByRole("button", { + const addLabelButton = screen.getByRole("button", { name: "Add visible label", }); - // The last button is the one for the whole line, not for - // the points the define the line. - const addLabelButton = addLabelButtons[addLabelButtons.length - 1]; await userEvent.click(addLabelButton); // Assert diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx index e6a343e20c..24280e9307 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx @@ -212,7 +212,7 @@ const LockedVectorSettings = (props: Props) => { const newLabel = { ...getDefaultFigureForType("label"), coord: labelLocation, - // Default to the same color as the point + // Default to the same color as the vector color: lineColor, } satisfies LockedLabelType;