diff --git a/.changeset/nice-turkeys-dress.md b/.changeset/nice-turkeys-dress.md new file mode 100644 index 0000000000..a21090ca25 --- /dev/null +++ b/.changeset/nice-turkeys-dress.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus-editor": minor +--- + +[Numeric Input] - Adjust editor to organize settings more logically diff --git a/packages/perseus-editor/src/components/heading.tsx b/packages/perseus-editor/src/components/heading.tsx index 864031b8dc..9618ed4140 100644 --- a/packages/perseus-editor/src/components/heading.tsx +++ b/packages/perseus-editor/src/components/heading.tsx @@ -51,6 +51,7 @@ const styles = StyleSheet.create({ marginInline: -10, backgroundColor: color.offBlack8, padding: spacing.xSmall_8, + width: "calc(100% + 20px)", }, heading: { flexDirection: "row", diff --git a/packages/perseus-editor/src/styles/perseus-editor.less b/packages/perseus-editor/src/styles/perseus-editor.less index a7f12136be..21f89f30fd 100644 --- a/packages/perseus-editor/src/styles/perseus-editor.less +++ b/packages/perseus-editor/src/styles/perseus-editor.less @@ -224,6 +224,16 @@ .categorizer-container { overflow-x: scroll; } + + .section-accordion { + display: flex; + flex-direction: row; + } + + .delete-item-button { + align-self: center; + padding-right: 0.5em; + } } .perseus-widget-editor-title-id > svg { 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 468e7ba95e..c9e17f17fb 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 @@ -1,4 +1,5 @@ import {Dependencies} from "@khanacademy/perseus"; +import {RenderStateRoot} from "@khanacademy/wonder-blocks-core"; import {render, screen, waitFor} from "@testing-library/react"; import {userEvent as userEventLib} from "@testing-library/user-event"; import * as React from "react"; @@ -21,7 +22,9 @@ describe("numeric-input-editor", () => { }); it("should render", async () => { - render( undefined} />); + render( undefined} />, { + wrapper: RenderStateRoot, + }); await waitFor(async () => expect( @@ -33,7 +36,9 @@ describe("numeric-input-editor", () => { it("should be possible to select normal width", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); await userEvent.click( screen.getByRole("button", {name: "Normal (80px)"}), @@ -48,7 +53,9 @@ describe("numeric-input-editor", () => { it("should be possible to select small width", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); await userEvent.click( screen.getByRole("button", {name: "Small (40px)"}), @@ -63,7 +70,9 @@ describe("numeric-input-editor", () => { it("should be possible to select right alignment", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); await userEvent.click( screen.getByRole("checkbox", {name: "Right alignment"}), @@ -75,7 +84,9 @@ describe("numeric-input-editor", () => { it("should be possible to select coefficient", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); await userEvent.click( screen.getByRole("checkbox", {name: "Coefficient"}), @@ -87,9 +98,10 @@ describe("numeric-input-editor", () => { it("should be possible to select strictly match only these formats", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); - await userEvent.click(screen.getByLabelText("Toggle options")); await userEvent.click( screen.getByRole("checkbox", { name: "Strictly match only these formats", @@ -114,7 +126,9 @@ describe("numeric-input-editor", () => { it("should be possible to update label text", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); const input = screen.getByRole("textbox", { name: "Aria label", @@ -128,26 +142,13 @@ describe("numeric-input-editor", () => { ); }); - it("should be possible to toggle options", async () => { - render( {}} />); - - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); - - expect( - screen.getByText("Unsimplified answers are"), - ).toBeInTheDocument(); - }); - it("should be possible to set unsimplified answers to ungraded", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); await userEvent.click(screen.getByRole("button", {name: "ungraded"})); expect(onChangeMock).toBeCalledWith( @@ -162,11 +163,10 @@ describe("numeric-input-editor", () => { it("should be possible to set unsimplified answers to accepted", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); await userEvent.click(screen.getByRole("button", {name: "accepted"})); expect(onChangeMock).toBeCalledWith( @@ -181,11 +181,10 @@ describe("numeric-input-editor", () => { it("should be possible to set unsimplified answers to wrong", async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); await userEvent.click(screen.getByRole("button", {name: "wrong"})); expect(onChangeMock).toBeCalledWith( @@ -210,11 +209,10 @@ describe("numeric-input-editor", () => { it(`should be possible to set suggested answer format to: ${name}`, async () => { const onChangeMock = jest.fn(); - render(); + render(, { + wrapper: RenderStateRoot, + }); - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); await userEvent.click(screen.getByTitle(name)); expect(onChangeMock).toBeCalledWith( diff --git a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index 2d3a6dd9a9..f7387c52bf 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -5,27 +5,26 @@ import { EditorJsonify, Util, PerseusI18nContext, - iconTrash, } from "@khanacademy/perseus"; +import Button from "@khanacademy/wonder-blocks-button"; import {Checkbox} from "@khanacademy/wonder-blocks-form"; +import {Strut} from "@khanacademy/wonder-blocks-layout"; +import {spacing} from "@khanacademy/wonder-blocks-tokens"; +import {LabelLarge} from "@khanacademy/wonder-blocks-typography"; +import trashIcon from "@phosphor-icons/core/bold/trash-bold.svg"; import * as React from "react"; import _ from "underscore"; +import Heading from "../components/heading"; +import PerseusEditorAccordion from "../components/perseus-editor-accordion"; import Editor from "../editor"; -import {iconGear} from "../styles/icon-paths"; import type {APIOptionsWithDefaults} from "@khanacademy/perseus"; type ChangeFn = typeof Changeable.change; -const { - ButtonGroup, - InfoTip, - InlineIcon, - MultiButtonGroup, - NumberInput, - TextInput, -} = components; +const {ButtonGroup, InfoTip, MultiButtonGroup, NumberInput, TextInput} = + components; const {firstNumericalParse} = Util; // NOTE(john): Copied from perseus-types.d.ts in the Perseus package. @@ -100,6 +99,9 @@ type Props = PerseusNumericInputWidgetOptions & { type State = { lastStatus: string; showOptions: boolean[]; + showAnswerDetails: boolean[]; + showSettings: boolean; + showAnswers: boolean; }; class NumericInputEditor extends React.Component { @@ -122,6 +124,9 @@ class NumericInputEditor extends React.Component { this.state = { lastStatus: "wrong", showOptions: _.map(this.props.answers, () => false), + showAnswerDetails: _.map(this.props.answers, () => true), + showSettings: true, + showAnswers: true, }; } @@ -135,6 +140,21 @@ class NumericInputEditor extends React.Component { this.setState({showOptions: showOptions}); }; + onToggleAnswers = (answerIndex) => { + const showAnswerDetails = this.state.showAnswerDetails.slice(); + showAnswerDetails[answerIndex] = !showAnswerDetails[answerIndex]; + this.setState({showAnswerDetails: showAnswerDetails}); + }; + + onToggleHeading = (accordionName: string) => { + return () => { + const toggleName = `show${accordionName}`; + const newState = {...this.state}; + newState[toggleName] = !newState[toggleName]; + this.setState(newState); + }; + }; + onTrashAnswer = (choiceIndex) => { if (choiceIndex >= 0 && choiceIndex < this.props.answers.length) { const answers = this.props.answers.slice(0); @@ -195,6 +215,8 @@ class NumericInputEditor extends React.Component { addAnswer = () => { const lastAnswer: any = initAnswer(this.state.lastStatus); const answers = this.props.answers.concat(lastAnswer); + const showAnswerDetails = this.state.showAnswerDetails.concat(true); + this.setState({showAnswerDetails: showAnswerDetails}); this.props.onChange({answers: answers}); }; @@ -400,24 +422,6 @@ class NumericInputEditor extends React.Component { ); - const addAnswerButton = ( - - ); - const instructions = { wrong: "(address the mistake/misconception)", ungraded: "(explain in detail to avoid confusion)", @@ -446,175 +450,194 @@ class NumericInputEditor extends React.Component { }} /> ); + const statusProper = + answer.status.charAt(0).toUpperCase() + + answer.status.slice(1); + const answerRangeText = answer.maxError + ? `± ${answer.maxError}` + : ""; + const answerHeading = + answer.value === null + ? "New Answer" + : `${statusProper} answer: ${answer.value} ${answerRangeText}`; + return (
-
{ + this.onToggleAnswers(i); + }} + header={ +
+ {answerHeading} + +
} > - { - // NOTE(charlie): The mobile web expression - // editor relies on this automatic answer - // form resolution for determining when to - // show the Pi symbol. If we get rid of it, - // we should also disable Pi for - // NumericInput and require problems that - // use Pi to build on Expression. - // Alternatively, we could store answers - // as plaintext and parse them to determine - // whether or not to reveal Pi on the - // keypad (right now, answers are stored as - // resolved values, like '0.125' rather - // than '1/8'). - let forms; - if (format === "pi") { - forms = ["pi"]; - } else if (format === "mixed") { - forms = ["proper", "mixed"]; - } else if ( - format === "proper" || - format === "improper" - ) { - forms = ["proper", "improper"]; - } - this.updateAnswer(i, { - value: firstNumericalParse( - newValue, - this.context.strings, - ), - answerForms: forms, - }); - }} - onChange={(newValue) => { - this.updateAnswer(i, { - value: firstNumericalParse( - newValue, - this.context.strings, - ), - }); - }} - /> - {answer.strict && ( -
- ≡ -
- )} - {answer.simplify !== "required" && - answer.status === "correct" && ( -
User input
+
+ { + // NOTE(charlie): The mobile web expression + // editor relies on this automatic answer + // form resolution for determining when to + // show the Pi symbol. If we get rid of it, + // we should also disable Pi for + // NumericInput and require problems that + // use Pi to build on Expression. + // Alternatively, we could store answers + // as plaintext and parse them to determine + // whether or not to reveal Pi on the + // keypad (right now, answers are stored as + // resolved values, like '0.125' rather + // than '1/8'). + let forms; + if (format === "pi") { + forms = ["pi"]; + } else if (format === "mixed") { + forms = ["proper", "mixed"]; + } else if ( + format === "proper" || + format === "improper" + ) { + forms = ["proper", "improper"]; } - title="accepts unsimplified answers" + this.updateAnswer(i, { + value: firstNumericalParse( + newValue, + this.context.strings, + ), + answerForms: forms, + }); + }} + onChange={(newValue) => { + this.updateAnswer(i, { + value: firstNumericalParse( + newValue, + this.context.strings, + ), + }); + }} + /> + {answer.strict && ( +
- ‰ + ≡
)} - {answer.maxError ? ( -
-
- ± + {answer.simplify !== "required" && + answer.status === "correct" && ( +
+ ‰ +
+ )} + {answer.maxError ? ( +
+
+ ± +
+
- + ) : null} + +
{maxError(i)} {answer.status === "correct" && unsimplifiedAnswers(i)} {suggestedAnswerTypes(i)}
- )} + +
); }); return (
-
User input
-
- Message shown to user on attempt -
- {generateInputAnswerEditors()} - {addAnswerButton} - {inputSize} - {rightAlign} - {coefficientCheck} - {labelText} + + {this.state.showSettings && inputSize} + {this.state.showSettings && rightAlign} + {this.state.showSettings && coefficientCheck} + {this.state.showSettings && labelText} + + {this.state.showAnswers && ( + <> + {generateInputAnswerEditors()} + + + )}
); }