From 182c8f6600bbefa817c4553e3827498b8d425bbe Mon Sep 17 00:00:00 2001 From: Matthew Date: Tue, 13 Aug 2024 14:16:32 -0500 Subject: [PATCH] WB-ify ExpressionEditor (#1480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Move ExpressionEditor to using WB. Issue: Part of [LEMS-2222](https://khanacademy.atlassian.net/browse/LEMS-2222) ## Test plan: Behavior shouldn't change, just move components over to WB. [LEMS-2222]: https://khanacademy.atlassian.net/browse/LEMS-2222?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Author: handeyeco Reviewers: handeyeco, anakaren-rojas, jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (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), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1480 --- .changeset/orange-pants-buy.md | 6 + .../src/components/sortable.tsx | 17 +- .../src/styles/perseus-editor.less | 111 ------- .../__tests__/expression-editor.test.tsx | 16 +- .../__tests__/numeric-input-editor.test.tsx | 2 +- .../src/widgets/expression-editor.tsx | 291 +++++++++++------- .../src/widgets/numeric-input-editor.tsx | 10 +- .../perseus/src/components/text-input.tsx | 1 - packages/perseus/src/index.ts | 1 + 9 files changed, 213 insertions(+), 242 deletions(-) create mode 100644 .changeset/orange-pants-buy.md diff --git a/.changeset/orange-pants-buy.md b/.changeset/orange-pants-buy.md new file mode 100644 index 0000000000..03c7c84292 --- /dev/null +++ b/.changeset/orange-pants-buy.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +Refactor ExpressionEditor to use Wonder-Blocks diff --git a/packages/perseus-editor/src/components/sortable.tsx b/packages/perseus-editor/src/components/sortable.tsx index dd103f3cae..6ecdcc07d8 100644 --- a/packages/perseus-editor/src/components/sortable.tsx +++ b/packages/perseus-editor/src/components/sortable.tsx @@ -1,3 +1,4 @@ +import {css, StyleSheet} from "aphrodite"; import createReactClass from "create-react-class"; import PropTypes from "prop-types"; import * as React from "react"; @@ -5,7 +6,12 @@ import ReactDOM from "react-dom"; const PT = PropTypes; -// Takes an array of components to sort +/** + * Takes an array of components to sort. + * As of 08/05/24, there are two sortable components + * (one in perseus and one in perseus-editor). + * As far as I can tell, this one is only used in ExpressionEditor. + */ // eslint-disable-next-line react/no-unsafe const SortableArea = createReactClass({ propTypes: { @@ -162,6 +168,7 @@ const SortableItem = createReactClass({ e.preventDefault(); }, render: function () { + // I think these might be getting styles from Webapp let dragState = "sortable-disabled"; if (this.props.dragging) { dragState = "sortable-dragging"; @@ -172,7 +179,7 @@ const SortableItem = createReactClass({ return (
  • { await userEvent.click( screen.getByRole("checkbox", { - name: "Use × for rendering multiplication instead of a center dot.", + name: "Use × instead of ⋅ for multiplication", }), ); @@ -87,12 +87,18 @@ describe("expression-editor", () => { act(() => jest.runOnlyPendingTimers()); const input = screen.getByRole("textbox", { - name: "Function variables:", + name: "Function variables", }); - await userEvent.type(input, "x"); + // make sure we can add things + await userEvent.type(input, "x y z"); - expect(onChangeMock).toBeCalledWith({functions: ["x"]}); + expect(onChangeMock).lastCalledWith({functions: ["x", "y", "z"]}); + + // make sure we can remove things + await userEvent.type(input, "{backspace}"); + + expect(onChangeMock).lastCalledWith({functions: ["x", "y"]}); }); it("should toggle division checkbox", async () => { @@ -103,7 +109,7 @@ describe("expression-editor", () => { await userEvent.click( screen.getByRole("checkbox", { - name: "show \\div button", + name: "show ÷ button", }), ); diff --git a/packages/perseus-editor/src/widgets/__tests__/numeric-input-editor.test.tsx b/packages/perseus-editor/src/widgets/__tests__/numeric-input-editor.test.tsx index 0858fdc222..468e7ba95e 100644 --- a/packages/perseus-editor/src/widgets/__tests__/numeric-input-editor.test.tsx +++ b/packages/perseus-editor/src/widgets/__tests__/numeric-input-editor.test.tsx @@ -117,7 +117,7 @@ describe("numeric-input-editor", () => { render(); const input = screen.getByRole("textbox", { - name: "Label text:", + name: "Aria label", }); await userEvent.type(input, "a"); diff --git a/packages/perseus-editor/src/widgets/expression-editor.tsx b/packages/perseus-editor/src/widgets/expression-editor.tsx index bd40e6b50a..cf02abb74e 100644 --- a/packages/perseus-editor/src/widgets/expression-editor.tsx +++ b/packages/perseus-editor/src/widgets/expression-editor.tsx @@ -3,12 +3,19 @@ import * as KAS from "@khanacademy/kas"; import { components, Changeable, - Dependencies, Expression, PerseusExpressionAnswerFormConsidered, } from "@khanacademy/perseus"; -import {Checkbox} from "@khanacademy/wonder-blocks-form"; +import Button from "@khanacademy/wonder-blocks-button"; +import {Checkbox, LabeledTextField} from "@khanacademy/wonder-blocks-form"; +import {Strut} from "@khanacademy/wonder-blocks-layout"; +import {color, spacing} from "@khanacademy/wonder-blocks-tokens"; +import { + HeadingSmall, + HeadingXSmall, +} from "@khanacademy/wonder-blocks-typography"; import {isTruthy} from "@khanacademy/wonder-stuff-core"; +import {css, StyleSheet} from "aphrodite"; // eslint-disable-next-line import/no-extraneous-dependencies import lens from "hubble"; import * as React from "react"; @@ -16,9 +23,12 @@ import _ from "underscore"; import SortableArea from "../components/sortable"; -import type {PerseusExpressionWidgetOptions} from "@khanacademy/perseus"; +import type { + PerseusExpressionWidgetOptions, + LegacyButtonSets, +} from "@khanacademy/perseus"; -const {InfoTip, TextInput} = components; +const {InfoTip} = components; type Props = { widgetId?: any; @@ -37,7 +47,7 @@ type DefaultProps = { functions: Props["functions"]; }; -const buttonSetsList: any = [ +const buttonSetsList: LegacyButtonSets = [ "basic", "trig", "prealgebra", @@ -77,7 +87,13 @@ const _makeNewKey = (answerForms: ReadonlyArray) => { return usedKeys.length; }; -class ExpressionEditor extends React.Component { +type State = { + // this is to help the "functions" input feel natural + // while still allowing us to to store the functions as an array + functionsInternal: string; +}; + +class ExpressionEditor extends React.Component { static widgetName = "expression" as const; static defaultProps: DefaultProps = { @@ -87,6 +103,13 @@ class ExpressionEditor extends React.Component { functions: ["f", "g", "h"], }; + constructor(props) { + super(props); + this.state = { + functionsInternal: this.props.functions.join(" "), + }; + } + change(...args) { return Changeable.change.apply(this, args); } @@ -135,7 +158,6 @@ class ExpressionEditor extends React.Component { ); @@ -143,47 +165,38 @@ class ExpressionEditor extends React.Component { const buttonSetChoices = buttonSetsList.map((name) => { // The first one gets special cased to always be checked, disabled, // and float left. - const isFirst = name === "basic"; - const checked = - this.props.buttonSets.includes( - name as PerseusExpressionWidgetOptions["buttonSets"][number], - ) || isFirst; + const isBasic = name === "basic"; + const checked = this.props.buttonSets.includes(name) || isBasic; return ( - + this.handleButtonSet(name)} + /> ); }); - const {TeX} = Dependencies.getDependencies(); // OldExpression only - buttonSetChoices.unshift( - , + , ); return ( -
    -

    Global Options

    - -
    - +
    + Global Options + +
    +

    Optional visible text; strongly encouraged to help @@ -194,14 +207,12 @@ class ExpressionEditor extends React.Component {

    -
    - +
    +

    Label text that's read by screen readers. Highly @@ -218,9 +229,24 @@ class ExpressionEditor extends React.Component {

    -
    +
    + + +

    + Single-letter variables listed here will be + interpreted as functions. This let us know that f(x) + means "f of x" and not "f times x". +

    +     +
    + +
    { this.props.onChange({times: value}); @@ -236,30 +262,12 @@ class ExpressionEditor extends React.Component {
    -
    - - -

    - Single-letter variables listed here will be - interpreted as functions. This let us know that f(x) - means "f of x" and not "f times x". -

    -     -
    - -
    -
    Button sets:
    +
    + Button Sets {buttonSetChoices}
    -

    Answers

    + Answers

    student responses area matched against these from top to @@ -269,14 +277,9 @@ class ExpressionEditor extends React.Component { {sortable}

    - +
    ); @@ -452,11 +455,10 @@ class ExpressionEditor extends React.Component { }; // called when the function variables change - handleFunctions: (arg1: React.ChangeEvent) => void = ( - e, - ) => { + handleFunctions: (value: string) => void = (value) => { + this.setState({functionsInternal: value}); const newProps: Record = {}; - newProps.functions = e.target.value.split(/[ ,]+/).filter(isTruthy); + newProps.functions = value.split(/[ ,]+/).filter(isTruthy); this.props.onChange(newProps); }; } @@ -491,58 +493,62 @@ class AnswerOption extends React.Component< > { state = {deleteFocused: false}; - handleDeleteBlur = () => { - this.setState({deleteFocused: false}); - }; - change = (...args) => { return Changeable.change.apply(this, args); }; render(): React.ReactNode { - let removeButton: React.ReactNode | null = null; - if (this.state.deleteFocused) { - removeButton = ( - - ); - } else { - removeButton = ( - - ); - } + + + + + ) : ( + + ); + + const answerStatusCss = css( + styles.answerStatus, + this.props.considered === "wrong" && styles.answerStatusWrong, + this.props.considered === "correct" && styles.answerStatusCorrect, + this.props.considered === "ungraded" && styles.answerStatusUngraded, + ); return ( -
    -
    +
    +
    -
    -
    -
    +
    +
    + -
    +
    -
    +
    -
    +
    -
    {removeButton}
    +
    + {removeButton} +
    ); @@ -588,6 +596,10 @@ class AnswerOption extends React.Component< this.props.onDelete(); }; + handleCancelDelete = () => { + this.setState({deleteFocused: false}); + }; + handleDelete = () => { this.setState({deleteFocused: true}); }; @@ -602,3 +614,48 @@ class AnswerOption extends React.Component< } export default ExpressionEditor; + +const styles = StyleSheet.create({ + paddedX: { + paddingLeft: spacing.xSmall_8, + paddingRight: spacing.xSmall_8, + }, + paddedY: { + paddingTop: spacing.xxSmall_6, + paddingBottom: spacing.xxSmall_6, + }, + answerOption: { + border: "1px solid #ddd", + borderRadius: "3px", + display: "flex", + }, + answerHandle: { + // textured draggy handle + background: + "url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAeCAYAAADkftS9AAAAIklEQVQoU2M4c+bMfxAGAgYYmwGrIIiDjrELjpo5aiZeMwF+yNnOs5KSvgAAAABJRU5ErkJggg==) no-repeat 50% 50%", + borderRight: "1px solid #ddd", + cursor: "move", + width: "20px", + minWidth: "20px", + }, + answerStatus: { + border: "none", + userSelect: "none", + width: "100px", + paddingTop: spacing.small_12, + paddingBottom: spacing.small_12, + }, + answerStatusWrong: { + backgroundColor: color.fadedRed16, + }, + answerStatusCorrect: { + backgroundColor: color.fadedGreen16, + }, + answerStatusUngraded: { + backgroundColor: color.fadedBlue16, + }, + answerBody: {}, + buttonRow: { + display: "flex", + }, +}); diff --git a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index 8fb01b35cb..97eaf5c92b 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -259,7 +259,7 @@ class NumericInputEditor extends React.Component { const labelText = (