From 7396d11583b1f555937461997cc68602d505d38d Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Tue, 3 Dec 2024 16:35:20 -0800 Subject: [PATCH 01/12] Add headings for "Common Settings" and "Answers" sections. Move editor components to appropriate sections under headings. --- .../perseus-editor/src/components/heading.tsx | 1 + .../src/widgets/numeric-input-editor.tsx | 77 ++++++++++++------- 2 files changed, 49 insertions(+), 29 deletions(-) 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/widgets/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index cc3928e509..bc00b909b7 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -5,8 +5,10 @@ import { EditorJsonify, Util, PerseusI18nContext, - iconTrash, + iconTrash, LockedLabelType, } from "@khanacademy/perseus"; +import Heading from "../components/heading"; +import Button from "@khanacademy/wonder-blocks-button"; import {Checkbox} from "@khanacademy/wonder-blocks-form"; import * as React from "react"; import _ from "underscore"; @@ -15,6 +17,7 @@ import Editor from "../editor"; import {iconGear} from "../styles/icon-paths"; import type {APIOptionsWithDefaults} from "@khanacademy/perseus"; +import {getDefaultFigureForType} from "./interactive-graph-editor/locked-figures/util"; type ChangeFn = typeof Changeable.change; @@ -100,6 +103,8 @@ type Props = PerseusNumericInputWidgetOptions & { type State = { lastStatus: string; showOptions: boolean[]; + showSettings: boolean; + showAnswers: boolean; }; class NumericInputEditor extends React.Component { @@ -122,6 +127,8 @@ class NumericInputEditor extends React.Component { this.state = { lastStatus: "wrong", showOptions: _.map(this.props.answers, () => false), + showSettings: true, + showAnswers: true, }; } @@ -135,6 +142,15 @@ class NumericInputEditor extends React.Component { this.setState({showOptions: showOptions}); }; + onToggleAccordion = (accordionName: string) => { + return () => { + const toggleName = `show${accordionName}`; + const newState = {[toggleName]: !this.state[toggleName]}; + // @ts-expect-error + this.setState(newState); + } + } + onTrashAnswer = (choiceIndex) => { if (choiceIndex >= 0 && choiceIndex < this.props.answers.length) { const answers = this.props.answers.slice(0); @@ -397,24 +413,6 @@ class NumericInputEditor extends React.Component { ); - const addAnswerButton = ( - - ); - const instructions = { wrong: "(address the mistake/misconception)", ungraded: "(explain in detail to avoid confusion)", @@ -602,16 +600,37 @@ class NumericInputEditor extends React.Component { 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 && ( + <> +
User input
+
+ Message shown to user on attempt +
+ {generateInputAnswerEditors()} + + + )}
); } From b88706db9473e4c0846aa42189ae26d19767644c Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Wed, 4 Dec 2024 16:05:39 -0800 Subject: [PATCH 02/12] Individual answers moved to their own accordion. --- .../src/styles/perseus-editor.less | 5 + .../src/widgets/numeric-input-editor.tsx | 334 ++++++++++-------- 2 files changed, 184 insertions(+), 155 deletions(-) diff --git a/packages/perseus-editor/src/styles/perseus-editor.less b/packages/perseus-editor/src/styles/perseus-editor.less index a7f12136be..375ee8042d 100644 --- a/packages/perseus-editor/src/styles/perseus-editor.less +++ b/packages/perseus-editor/src/styles/perseus-editor.less @@ -224,6 +224,11 @@ .categorizer-container { overflow-x: scroll; } + + .section-accordion { + display: flex; + flex-direction: row; + } } .perseus-widget-editor-title-id > svg { diff --git a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index bc00b909b7..790d91e39e 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -5,19 +5,23 @@ import { EditorJsonify, Util, PerseusI18nContext, - iconTrash, LockedLabelType, + iconTrash, } from "@khanacademy/perseus"; -import Heading from "../components/heading"; import Button from "@khanacademy/wonder-blocks-button"; +import {View} from "@khanacademy/wonder-blocks-core"; 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 * 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"; -import {getDefaultFigureForType} from "./interactive-graph-editor/locked-figures/util"; type ChangeFn = typeof Changeable.change; @@ -103,6 +107,7 @@ type Props = PerseusNumericInputWidgetOptions & { type State = { lastStatus: string; showOptions: boolean[]; + showAnswerDetails: boolean[]; showSettings: boolean; showAnswers: boolean; }; @@ -127,6 +132,7 @@ 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, }; @@ -142,14 +148,20 @@ class NumericInputEditor extends React.Component { this.setState({showOptions: showOptions}); }; - onToggleAccordion = (accordionName: string) => { + 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 = {[toggleName]: !this.state[toggleName]}; // @ts-expect-error this.setState(newState); - } - } + }; + }; onTrashAnswer = (choiceIndex) => { if (choiceIndex >= 0 && choiceIndex < this.props.answers.length) { @@ -211,6 +223,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}); }; @@ -443,157 +457,170 @@ class NumericInputEditor extends React.Component { ); return (
-
{ + this.onToggleAnswers(i); + }} + header={ +
+ Answer Option + +
} > - { - // 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" && ( -
+ { + // 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} - -
- {editor} -
- {this.state.showOptions[i] && ( -
- {maxError(i)} - {answer.status === "correct" && - unsimplifiedAnswers(i)} - {suggestedAnswerTypes(i)} + ) : null} + - )} +
+ {editor} +
+ {this.state.showOptions[i] && ( +
+ {maxError(i)} + {answer.status === "correct" && + unsimplifiedAnswers(i)} + {suggestedAnswerTypes(i)} +
+ )} +
); }); @@ -604,7 +631,7 @@ class NumericInputEditor extends React.Component { title="Common Settings" isCollapsible={true} isOpen={this.state.showSettings} - onToggle={this.onToggleAccordion("Settings")} + onToggle={this.onToggleHeading("Settings")} /> {this.state.showSettings && inputSize} {this.state.showSettings && rightAlign} @@ -614,7 +641,7 @@ class NumericInputEditor extends React.Component { title="Answers" isCollapsible={true} isOpen={this.state.showAnswers} - onToggle={this.onToggleAccordion("Answers")} + onToggle={this.onToggleHeading("Answers")} /> {this.state.showAnswers && ( <> @@ -623,10 +650,7 @@ class NumericInputEditor extends React.Component { Message shown to user on attempt
{generateInputAnswerEditors()} - From c8ac23e710f30cea2c862eaa3beeb35570dde8f1 Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Wed, 4 Dec 2024 16:49:52 -0800 Subject: [PATCH 03/12] Move delete button to bottom of individual answer panel. --- .../src/styles/perseus-editor.less | 5 ++ .../src/widgets/numeric-input-editor.tsx | 58 +++++++++++++------ 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/packages/perseus-editor/src/styles/perseus-editor.less b/packages/perseus-editor/src/styles/perseus-editor.less index 375ee8042d..21f89f30fd 100644 --- a/packages/perseus-editor/src/styles/perseus-editor.less +++ b/packages/perseus-editor/src/styles/perseus-editor.less @@ -229,6 +229,11 @@ 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/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index 790d91e39e..13f1e6fe4b 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -8,11 +8,11 @@ import { iconTrash, } from "@khanacademy/perseus"; import Button from "@khanacademy/wonder-blocks-button"; -import {View} from "@khanacademy/wonder-blocks-core"; 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"; @@ -455,6 +455,17 @@ 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 (
{ }} header={
- Answer Option + {answerHeading}
} @@ -576,22 +587,22 @@ class NumericInputEditor extends React.Component { > {answer.status} - { - // preventDefault ensures that href="#" - // doesn't scroll to the top of the page - e.preventDefault(); - this.onTrashAnswer(i); - }} - onKeyDown={(e) => - this.onSpace(e, this.onTrashAnswer) - } - > - - + {/* {*/} + {/* // preventDefault ensures that href="#"*/} + {/* // doesn't scroll to the top of the page*/} + {/* e.preventDefault();*/} + {/* this.onTrashAnswer(i);*/} + {/* }}*/} + {/* onKeyDown={(e) =>*/} + {/* this.onSpace(e, this.onTrashAnswer)*/} + {/* }*/} + {/*>*/} + {/* */} + {/**/} { {suggestedAnswerTypes(i)}
)} +
); From 4af63e03f294dbab5f8130959fa7f39e6d855385 Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Wed, 4 Dec 2024 16:57:10 -0800 Subject: [PATCH 04/12] Move settings labels to individual answer sections. --- .../src/widgets/numeric-input-editor.tsx | 26 ++++--------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index 13f1e6fe4b..486e59c3c6 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -480,6 +480,7 @@ class NumericInputEditor extends React.Component {
} > +
User input
{ > {answer.status} - {/* {*/} - {/* // preventDefault ensures that href="#"*/} - {/* // doesn't scroll to the top of the page*/} - {/* e.preventDefault();*/} - {/* this.onTrashAnswer(i);*/} - {/* }}*/} - {/* onKeyDown={(e) =>*/} - {/* this.onSpace(e, this.onTrashAnswer)*/} - {/* }*/} - {/*>*/} - {/* */} - {/**/} {
+
+ Message shown to user on attempt +
{editor}
{this.state.showOptions[i] && (
- {maxError(i)} + {maxError(i)} {answer.status === "correct" && unsimplifiedAnswers(i)} {suggestedAnswerTypes(i)} @@ -667,10 +655,6 @@ class NumericInputEditor extends React.Component { /> {this.state.showAnswers && ( <> -
User input
-
- Message shown to user on attempt -
{generateInputAnswerEditors()}
{this.state.showOptions[i] && (
- {maxError(i)} + {maxError(i)} {answer.status === "correct" && unsimplifiedAnswers(i)} {suggestedAnswerTypes(i)} From 3a03430b9a143181ffcb03c87835881c486bf66b Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Thu, 5 Dec 2024 10:01:05 -0800 Subject: [PATCH 06/12] Changeset --- .changeset/nice-turkeys-dress.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/nice-turkeys-dress.md 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 From b7abe6e46a404fbeebe5498543cb6137d9612efe Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Thu, 5 Dec 2024 10:14:06 -0800 Subject: [PATCH 07/12] Remove TS comment (no good way to resolve). --- packages/perseus-editor/src/widgets/numeric-input-editor.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index 0ae5f32230..33457f63e8 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -157,7 +157,6 @@ class NumericInputEditor extends React.Component { return () => { const toggleName = `show${accordionName}`; const newState = {[toggleName]: !this.state[toggleName]}; - // @ts-expect-error this.setState(newState); }; }; From 17bad8261cb27ad5cc101565e937ea47160650dd Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald Date: Thu, 5 Dec 2024 12:18:37 -0800 Subject: [PATCH 08/12] Adjust for Typescript issue. --- packages/perseus-editor/src/widgets/numeric-input-editor.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx index 33457f63e8..742cab7e5f 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -156,7 +156,8 @@ class NumericInputEditor extends React.Component { onToggleHeading = (accordionName: string) => { return () => { const toggleName = `show${accordionName}`; - const newState = {[toggleName]: !this.state[toggleName]}; + const newState = {...this.state}; + newState[toggleName] === !newState[toggleName]; this.setState(newState); }; }; @@ -639,7 +640,7 @@ class NumericInputEditor extends React.Component { return (
Date: Fri, 6 Dec 2024 10:22:25 -0800 Subject: [PATCH 09/12] Remove toggle button for additional answer options, and always show those options. Update unit tests accordingly. --- .../__tests__/numeric-input-editor.test.tsx | 27 ----------------- .../src/widgets/numeric-input-editor.tsx | 30 ++++--------------- 2 files changed, 6 insertions(+), 51 deletions(-) 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 84f4722133..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 @@ -102,7 +102,6 @@ describe("numeric-input-editor", () => { wrapper: RenderStateRoot, }); - await userEvent.click(screen.getByLabelText("Toggle options")); await userEvent.click( screen.getByRole("checkbox", { name: "Strictly match only these formats", @@ -143,20 +142,6 @@ describe("numeric-input-editor", () => { ); }); - it("should be possible to toggle options", async () => { - render( {}} />, { - wrapper: RenderStateRoot, - }); - - 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(); @@ -164,9 +149,6 @@ describe("numeric-input-editor", () => { wrapper: RenderStateRoot, }); - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); await userEvent.click(screen.getByRole("button", {name: "ungraded"})); expect(onChangeMock).toBeCalledWith( @@ -185,9 +167,6 @@ describe("numeric-input-editor", () => { wrapper: RenderStateRoot, }); - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); await userEvent.click(screen.getByRole("button", {name: "accepted"})); expect(onChangeMock).toBeCalledWith( @@ -206,9 +185,6 @@ describe("numeric-input-editor", () => { wrapper: RenderStateRoot, }); - await userEvent.click( - screen.getByRole("link", {name: "Toggle options"}), - ); await userEvent.click(screen.getByRole("button", {name: "wrong"})); expect(onChangeMock).toBeCalledWith( @@ -237,9 +213,6 @@ describe("numeric-input-editor", () => { 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 742cab7e5f..ea2c8d3bdc 100644 --- a/packages/perseus-editor/src/widgets/numeric-input-editor.tsx +++ b/packages/perseus-editor/src/widgets/numeric-input-editor.tsx @@ -590,22 +590,6 @@ class NumericInputEditor extends React.Component { > {answer.status} - { - // preventDefault ensures that href="#" - // doesn't scroll to the top of the page - e.preventDefault(); - this.onToggleOptions(i); - }} - onKeyDown={(e) => - this.onSpace(e, this.onToggleOptions) - } - > - -
@@ -613,14 +597,12 @@ class NumericInputEditor extends React.Component {
{editor}
- {this.state.showOptions[i] && ( -
- {maxError(i)} - {answer.status === "correct" && - unsimplifiedAnswers(i)} - {suggestedAnswerTypes(i)} -
- )} +
+ {maxError(i)} + {answer.status === "correct" && + unsimplifiedAnswers(i)} + {suggestedAnswerTypes(i)} +