From 0fede5c3174484d48db2da79a38d633f6facbfe7 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 16 Jan 2025 13:10:55 -0800 Subject: [PATCH 01/11] Move Numeric Input's UI related code to a functional component (#2108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a lot of old patterns in Perseus, and we would like to start to updating to more modern methods. This PR updates the Numeric Input logic in the following ways: 1. Moves all UI Related Numeric Input logic to a functional component, and creates a new `numeric-input.class.tsx` file for housing the static / class methods. 2. Removes all string refs related to Numeric Input in both the InputWithExamples, SimpleKeypadInput, and Tooltip components 3. Adds a little more specificity to method parameters Issue: LEMS-2443 - Manual Testing - Automated Tests - Landing onto a feature branch for future QA Regression pass Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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) Pull Request URL: https://github.com/Khan/perseus/pull/2108 --- .changeset/smart-countries-hunt.md | 6 + .../src/components/input/math-input.tsx | 3 + .../src/components/input-with-examples.tsx | 17 +- .../src/components/simple-keypad-input.tsx | 25 +- packages/perseus/src/components/tooltip.tsx | 2 - .../numeric-input/prompt-utils.ts | 2 +- .../graded-group-set-jipt.test.ts.snap | 6 +- .../graded-group-set.test.ts.snap | 2 +- .../group/__snapshots__/group.test.tsx.snap | 4 +- .../__snapshots__/numeric-input.test.ts.snap | 46 +- .../src/widgets/numeric-input/index.ts | 2 +- .../numeric-input/numeric-input.class.tsx | 276 ++++++++++ .../numeric-input/numeric-input.stories.tsx | 2 +- .../numeric-input/numeric-input.test.ts | 4 +- .../widgets/numeric-input/numeric-input.tsx | 479 ++++-------------- .../src/widgets/numeric-input/utils.test.ts | 84 +++ .../src/widgets/numeric-input/utils.ts | 71 +++ 17 files changed, 592 insertions(+), 439 deletions(-) create mode 100644 .changeset/smart-countries-hunt.md create mode 100644 packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx create mode 100644 packages/perseus/src/widgets/numeric-input/utils.test.ts create mode 100644 packages/perseus/src/widgets/numeric-input/utils.ts diff --git a/.changeset/smart-countries-hunt.md b/.changeset/smart-countries-hunt.md new file mode 100644 index 0000000000..fcb293c3fc --- /dev/null +++ b/.changeset/smart-countries-hunt.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/math-input": minor +"@khanacademy/perseus": minor +--- + +Refactoring Numeric Input to move UI-logic to functional component. diff --git a/packages/math-input/src/components/input/math-input.tsx b/packages/math-input/src/components/input/math-input.tsx index 2a9f5cfb52..496cade97f 100644 --- a/packages/math-input/src/components/input/math-input.tsx +++ b/packages/math-input/src/components/input/math-input.tsx @@ -353,6 +353,9 @@ class MathInput extends React.Component { }); }; + // [Jan 2025] Third: While testing, I've discovered that we likely don't + // need to be passing setKeypadActive here at all. Removing the parameter + // still results in the same behavior. focus: (setKeypadActive: KeypadContextType["setKeypadActive"]) => void = ( setKeypadActive, ) => { diff --git a/packages/perseus/src/components/input-with-examples.tsx b/packages/perseus/src/components/input-with-examples.tsx index 1b8b70645e..6a8a575851 100644 --- a/packages/perseus/src/components/input-with-examples.tsx +++ b/packages/perseus/src/components/input-with-examples.tsx @@ -51,6 +51,8 @@ class InputWithExamples extends React.Component { static contextType = PerseusI18nContext; declare context: React.ContextType; + inputRef: React.RefObject; + static defaultProps: DefaultProps = { shouldShowExamples: true, onFocus: function () {}, @@ -65,6 +67,11 @@ class InputWithExamples extends React.Component { showExamples: false, }; + constructor(props: Props) { + super(props); + this.inputRef = React.createRef(); + } + _getUniqueId: () => string = () => { return `input-with-examples-${btoa(this.props.id).replace(/=/g, "")}`; }; @@ -98,7 +105,7 @@ class InputWithExamples extends React.Component { const inputProps = { id: id, "aria-describedby": ariaId, - ref: "input", + ref: this.inputRef, className: this._getInputClassName(), labelText: this.props.labelText, value: this.props.value, @@ -148,15 +155,11 @@ class InputWithExamples extends React.Component { }; focus: () => void = () => { - // eslint-disable-next-line react/no-string-refs - // @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'. - this.refs.input.focus(); + this.inputRef.current?.focus(); }; blur: () => void = () => { - // eslint-disable-next-line react/no-string-refs - // @ts-expect-error - TS2339 - Property 'blur' does not exist on type 'ReactInstance'. - this.refs.input.blur(); + this.inputRef.current?.blur(); }; handleChange: (arg1: any) => void = (e) => { diff --git a/packages/perseus/src/components/simple-keypad-input.tsx b/packages/perseus/src/components/simple-keypad-input.tsx index 0889a92934..f1cb199cc6 100644 --- a/packages/perseus/src/components/simple-keypad-input.tsx +++ b/packages/perseus/src/components/simple-keypad-input.tsx @@ -8,6 +8,7 @@ * interface to `math-input`'s MathInput component. */ +import {KeypadContext} from "@khanacademy/keypad-context"; import { KeypadInput, KeypadType, @@ -17,7 +18,15 @@ import PropTypes from "prop-types"; import * as React from "react"; export default class SimpleKeypadInput extends React.Component { + static contextType = KeypadContext; + declare context: React.ContextType; _isMounted = false; + inputRef: React.RefObject; + + constructor(props: any) { + super(props); + this.inputRef = React.createRef(); + } componentDidMount() { // TODO(scottgrant): This is a hack to remove the deprecated call to @@ -30,18 +39,13 @@ export default class SimpleKeypadInput extends React.Component { } focus() { - // @ts-expect-error - TS2339 - Property 'focus' does not exist on type 'ReactInstance'. - this.refs.input.focus(); // eslint-disable-line react/no-string-refs + // The inputRef is a ref to a MathInput, which + // also controls the keypad state during focus events. + this.inputRef.current?.focus(this.context.setKeypadActive); } blur() { - // eslint-disable-next-line react/no-string-refs - // @ts-expect-error - TS2339 - Property 'blur' does not exist on type 'ReactInstance'. - if (typeof this.refs.input?.blur === "function") { - // eslint-disable-next-line react/no-string-refs - // @ts-expect-error - TS2339 - Property 'blur' does not exist on type 'ReactInstance'. - this.refs.input?.blur(); - } + this.inputRef.current?.blur(); } getValue(): string | number { @@ -59,8 +63,7 @@ export default class SimpleKeypadInput extends React.Component { return ( // @ts-expect-error - TS2769 - No overload matches this call. { if (keypadElement) { diff --git a/packages/perseus/src/components/tooltip.tsx b/packages/perseus/src/components/tooltip.tsx index 21756dcf70..a112ce586c 100644 --- a/packages/perseus/src/components/tooltip.tsx +++ b/packages/perseus/src/components/tooltip.tsx @@ -356,8 +356,6 @@ class Tooltip extends React.Component { {/* The contents of the tooltip */}
-
+
- + + + -
@@ -101,7 +103,7 @@ exports[`numeric-input widget Should render predictably: after interaction 1`] = autocapitalize="off" autocomplete="off" autocorrect="off" - class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_ylhnsi" + class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_ylhnsi" id="input-with-examples-bnVtZXJpYy1pbnB1dCAx" type="text" value="1252" @@ -319,7 +321,7 @@ exports[`numeric-input widget Should render predictably: first render 1`] = ` autocapitalize="off" autocomplete="off" autocorrect="off" - class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo" + class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo" id="input-with-examples-bnVtZXJpYy1pbnB1dCAx" type="text" value="" @@ -537,7 +539,7 @@ exports[`numeric-input widget Should render tooltip as list when multiple format autocapitalize="off" autocomplete="off" autocorrect="off" - class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo" + class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo" id="input-with-examples-bnVtZXJpYy1pbnB1dCAx" type="text" value="" @@ -702,7 +704,7 @@ exports[`numeric-input widget Should render tooltip when format option is given: autocapitalize="off" autocomplete="off" autocorrect="off" - class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-input_1y6ajxo" + class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_53h0n9-o_O-defaultFocus_9n1kv3-o_O-inputWithExamples_1y6ajxo" id="input-with-examples-bnVtZXJpYy1pbnB1dCAx" type="text" value="" diff --git a/packages/perseus/src/widgets/numeric-input/index.ts b/packages/perseus/src/widgets/numeric-input/index.ts index b4020aa5c3..4297e3379b 100644 --- a/packages/perseus/src/widgets/numeric-input/index.ts +++ b/packages/perseus/src/widgets/numeric-input/index.ts @@ -1 +1 @@ -export {default} from "./numeric-input"; +export {default} from "./numeric-input.class"; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx new file mode 100644 index 0000000000..e782cc403f --- /dev/null +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx @@ -0,0 +1,276 @@ +import {KhanMath} from "@khanacademy/kmath"; +import {linterContextDefault} from "@khanacademy/perseus-linter"; +import * as React from "react"; +import _ from "underscore"; + +import {ApiOptions} from "../../perseus-api"; +import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/numeric-input/prompt-utils"; + +import {NumericInputComponent} from "./numeric-input"; +import {scoreNumericInput} from "@khanacademy/perseus-score"; +import {NumericExampleStrings} from "./utils"; + +import type InputWithExamples from "../../components/input-with-examples"; +import type SimpleKeypadInput from "../../components/simple-keypad-input"; +import type {FocusPath, Widget, WidgetExports, WidgetProps} from "../../types"; +import type { + PerseusNumericInputRubric, + PerseusNumericInputUserInput, +} from "@khanacademy/perseus-score"; +import type {NumericInputPromptJSON} from "../../widget-ai-utils/numeric-input/prompt-utils"; +import type { + PerseusNumericInputWidgetOptions, + PerseusNumericInputAnswerForm, + MathFormat, +} from "@khanacademy/perseus-core"; +import type {PropsFor} from "@khanacademy/wonder-blocks-core"; +import type {RefObject} from "react"; + +type ExternalProps = WidgetProps< + PerseusNumericInputWidgetOptions, + PerseusNumericInputRubric +>; + +export type NumericInputProps = ExternalProps & { + size: NonNullable; + rightAlign: NonNullable; + apiOptions: NonNullable; + coefficient: NonNullable; + answerForms: NonNullable; + labelText: string; + linterContext: NonNullable; + currentValue: string; +}; + +type DefaultProps = { + currentValue: NumericInputProps["currentValue"]; + size: NumericInputProps["size"]; + rightAlign: NumericInputProps["rightAlign"]; + apiOptions: NumericInputProps["apiOptions"]; + coefficient: NumericInputProps["coefficient"]; + answerForms: NumericInputProps["answerForms"]; + labelText: NumericInputProps["labelText"]; + linterContext: NumericInputProps["linterContext"]; +}; + +// Assert that the PerseusNumericInputWidgetOptions parsed from JSON can be passed +// as props to this component. This ensures that the PerseusMatrixWidgetOptions +// stays in sync with the prop types. The PropsFor type takes +// defaultProps into account, which is important because +// PerseusNumericInputWidgetOptions has optional fields which receive defaults +// via defaultProps. +0 as any as WidgetProps< + PerseusNumericInputWidgetOptions, + PerseusNumericInputRubric +> satisfies PropsFor; + +/** + * The NumericInput widget is a numeric input field that supports a variety of + * answer forms, including integers, decimals, fractions, and mixed numbers. + * + * [Jan 2025] We're currenly migrating from class-based components to functional + * components. While we cannot fully migrate this component yet, we can start + * by using the functional component for the rendering the UI of the widget. + */ +export class NumericInput + extends React.Component + implements Widget +{ + inputRef: RefObject; + + static defaultProps: DefaultProps = { + currentValue: "", + size: "normal", + rightAlign: false, + apiOptions: ApiOptions.defaults, + coefficient: false, + answerForms: [], + labelText: "", + linterContext: linterContextDefault, + }; + + static getUserInputFromProps( + props: NumericInputProps, + ): PerseusNumericInputUserInput { + return { + currentValue: props.currentValue, + }; + } + + constructor(props: NumericInputProps) { + super(props); + // Create a ref that we can pass down to the input component so that we + // can call focus on it when necessary. + this.inputRef = React.createRef< + SimpleKeypadInput | InputWithExamples + >(); + } + + focus: () => boolean = () => { + this.inputRef.current?.focus(); + return true; + }; + + focusInputPath: () => void = () => { + this.inputRef.current?.focus(); + }; + + blurInputPath: () => void = () => { + this.inputRef.current?.blur(); + }; + + getInputPaths: () => ReadonlyArray> = () => { + // The widget itself is an input, so we return a single empty list to + // indicate this. + /* c8 ignore next */ + return [[]]; + }; + + /** + * Sets the value of the input at the given path. + */ + setInputValue: ( + path: FocusPath, + newValue: string, + cb?: () => unknown | null | undefined, + ) => void = (path, newValue, cb) => { + this.props.onChange({currentValue: newValue}, cb); + }; + + /** + * Returns the value the user has currently input for this widget. + */ + getUserInput(): PerseusNumericInputUserInput { + return NumericInput.getUserInputFromProps(this.props); + } + + /** + * Returns the JSON representation of the prompt for this widget. + * This is used by the AI to determine the prompt for the widget. + */ + getPromptJSON(): NumericInputPromptJSON { + return _getPromptJSON(this.props, this.getUserInput()); + } + + render(): React.ReactNode { + return ; + } +} + +// TODO(thomas): Currently we receive a list of lists of acceptable answer types +// and union them down into a single set. It's worth considering whether it +// wouldn't make more sense to have a single set of acceptable answer types for +// a given *problem* rather than for each possible [correct/wrong] *answer*. +// When should two answers to a problem take different answer types? +// See D27790 for more discussion. +export const unionAnswerForms: ( + answerFormsList: ReadonlyArray< + ReadonlyArray + >, +) => ReadonlyArray = function (answerFormsList) { + // Takes a list of lists of answer forms, and returns a list of the forms + // in each of these lists in the same order that they're listed in the + // `formExamples` forms from above. + + // uniqueBy takes a list of elements and a function which compares whether + // two elements are equal, and returns a list of unique elements. This is + // just a helper function here, but works generally. + const uniqueBy = function (list, iteratee: any) { + // @ts-expect-error - TS2347 - Untyped function calls may not accept type arguments. + return list.reduce>((uniqueList, element) => { + // For each element, decide whether it's already in the list of + // unique items. + const inList = _.find(uniqueList, iteratee.bind(null, element)); + if (inList) { + return uniqueList; + } + return uniqueList.concat([element]); + }, []); + }; + + // Pull out all of the forms from the different lists. + const allForms = answerFormsList.flat(); + // Pull out the unique forms using uniqueBy. + const uniqueForms = uniqueBy(allForms, _.isEqual); + // Sort them by the order they appear in the `formExamples` list. + const formExampleKeys = Object.keys(NumericExampleStrings); + return _.sortBy(uniqueForms, (form) => { + return formExampleKeys.indexOf(form.name); + }); +}; + +type RenderProps = { + answerForms: ReadonlyArray<{ + simplify: "required" | "correct" | "enforced" | null | undefined; + name: "integer" | "decimal" | "proper" | "improper" | "mixed" | "pi"; + }>; + labelText: string; + size: "normal" | "small"; + coefficient: boolean; + rightAlign?: boolean; + static: boolean; +}; + +const propsTransform = function ( + widgetOptions: PerseusNumericInputWidgetOptions, +): RenderProps { + const rendererProps = _.extend(_.omit(widgetOptions, "answers"), { + answerForms: unionAnswerForms( + // Pull out the name of each form and whether that form has + // required simplification. + widgetOptions.answers.map((answer) => { + // @ts-expect-error - TS2345 - Argument of type 'readonly MathFormat[] | undefined' is not assignable to parameter of type 'Collection'. + return _.map(answer.answerForms, (form) => { + return { + simplify: answer.simplify, + name: form, + }; + }); + }), + ), + }); + + return rendererProps; +}; + +export default { + name: "numeric-input", + displayName: "Numeric input", + defaultAlignment: "inline-block", + accessible: true, + widget: NumericInput, + transform: propsTransform, + isLintable: true, + // TODO(LEMS-2656): remove TS suppression + // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusNumericInputUserInput'. + scorer: scoreNumericInput, + + // TODO(LEMS-2656): remove TS suppression + // @ts-expect-error: Type 'Rubric' is not assignable to type 'PerseusNumericInputRubric' + getOneCorrectAnswerFromRubric( + rubric: PerseusNumericInputRubric, + ): string | null | undefined { + const correctAnswers = rubric.answers.filter( + (answer) => answer.status === "correct", + ); + const answerStrings = correctAnswers.map((answer) => { + // Either get the first answer form or default to decimal + const format: MathFormat = + answer.answerForms && answer.answerForms[0] + ? answer.answerForms[0] + : "decimal"; + + let answerString = KhanMath.toNumericString(answer.value!, format); + if (answer.maxError) { + answerString += + " \u00B1 " + + KhanMath.toNumericString(answer.maxError, format); + } + return answerString; + }); + if (answerStrings.length === 0) { + return; + } + return answerStrings[0]; + }, +} satisfies WidgetExports; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx index ec1678b1cf..0222b37690 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx @@ -3,7 +3,7 @@ import * as React from "react"; import {RendererWithDebugUI} from "../../../../../testing/renderer-with-debug-ui"; -import {NumericInput} from "./numeric-input"; +import {NumericInput} from "./numeric-input.class"; import {question1} from "./numeric-input.testdata"; type StoryArgs = { diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts index 0996e0fe9e..eae8966e56 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts @@ -6,7 +6,9 @@ import * as Dependencies from "../../dependencies"; import {scorePerseusItemTesting} from "../../util/test-utils"; import {renderQuestion} from "../__testutils__/renderQuestion"; -import NumericInputWidgetExport, {unionAnswerForms} from "./numeric-input"; +import NumericInputWidgetExport, { + unionAnswerForms, +} from "./numeric-input.class"; import { question1AndAnswer, multipleAnswers, diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx index 583a317488..4fbffacf19 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx @@ -1,418 +1,123 @@ -import {KhanMath} from "@khanacademy/kmath"; -import {linterContextDefault} from "@khanacademy/perseus-linter"; -import {scoreNumericInput} from "@khanacademy/perseus-score"; import {StyleSheet} from "aphrodite"; import * as React from "react"; +import { + forwardRef, + useContext, + useImperativeHandle, + useRef, + useState, +} from "react"; import _ from "underscore"; import {PerseusI18nContext} from "../../components/i18n-context"; import InputWithExamples from "../../components/input-with-examples"; import SimpleKeypadInput from "../../components/simple-keypad-input"; -import {ApiOptions} from "../../perseus-api"; -import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/numeric-input/prompt-utils"; -import type {PerseusStrings} from "../../strings"; -import type {FocusPath, Widget, WidgetExports, WidgetProps} from "../../types"; -import type {NumericInputPromptJSON} from "../../widget-ai-utils/numeric-input/prompt-utils"; -import type { - PerseusNumericInputWidgetOptions, - PerseusNumericInputAnswerForm, -} from "@khanacademy/perseus-core"; -import type { - PerseusNumericInputRubric, - PerseusNumericInputUserInput, -} from "@khanacademy/perseus-score"; -import type {PropsFor} from "@khanacademy/wonder-blocks-core"; - -const formExamples: { - [key: string]: ( - arg1: PerseusNumericInputAnswerForm, - strings: PerseusStrings, - ) => string; -} = { - integer: (form, strings: PerseusStrings) => strings.integerExample, - proper: (form, strings: PerseusStrings) => - form.simplify === "optional" - ? strings.properExample - : strings.simplifiedProperExample, - improper: (form, strings: PerseusStrings) => - form.simplify === "optional" - ? strings.improperExample - : strings.simplifiedImproperExample, - mixed: (form, strings: PerseusStrings) => strings.mixedExample, - decimal: (form, strings: PerseusStrings) => strings.decimalExample, - pi: (form, strings: PerseusStrings) => strings.piExample, -}; - -type ExternalProps = WidgetProps< - PerseusNumericInputWidgetOptions, - PerseusNumericInputRubric ->; - -type Props = ExternalProps & { - size: NonNullable; - rightAlign: NonNullable; - apiOptions: NonNullable; - coefficient: NonNullable; - answerForms: NonNullable; - labelText: string; - linterContext: NonNullable; - currentValue: string; -}; - -type DefaultProps = { - currentValue: Props["currentValue"]; - size: Props["size"]; - rightAlign: Props["rightAlign"]; - apiOptions: Props["apiOptions"]; - coefficient: Props["coefficient"]; - answerForms: Props["answerForms"]; - labelText: Props["labelText"]; - linterContext: Props["linterContext"]; -}; - -// Assert that the PerseusNumericInputWidgetOptions parsed from JSON can be passed -// as props to this component. This ensures that the PerseusMatrixWidgetOptions -// stays in sync with the prop types. The PropsFor type takes -// defaultProps into account, which is important because -// PerseusNumericInputWidgetOptions has optional fields which receive defaults -// via defaultProps. -0 as any as WidgetProps< - PerseusNumericInputWidgetOptions, - PerseusNumericInputRubric -> satisfies PropsFor; - -type State = { - // keeps track of the other set of values when switching - // between 0 and finite solutions - previousValues: ReadonlyArray; - isFocused: boolean; -}; - -export class NumericInput - extends React.Component - implements Widget -{ - static contextType = PerseusI18nContext; - declare context: React.ContextType; - - inputRef: SimpleKeypadInput | InputWithExamples | null | undefined; - - static defaultProps: DefaultProps = { - currentValue: "", - size: "normal", - rightAlign: false, - apiOptions: ApiOptions.defaults, - coefficient: false, - answerForms: [], - labelText: "", - linterContext: linterContextDefault, - }; - - static getUserInputFromProps(props: Props): PerseusNumericInputUserInput { - return { - currentValue: props.currentValue, - }; - } - - state: State = { - // keeps track of the other set of values when switching - // between 0 and finite solutions - previousValues: [""], - isFocused: false, - }; - - /** - * Generates a string that demonstrates how to input the various supported - * answer forms. - */ - examples(): ReadonlyArray { - // if the set of specified forms are empty, allow all forms - const forms = - this.props.answerForms?.length !== 0 - ? this.props.answerForms - : Object.keys(formExamples).map((name) => { - return { - name: name, - simplify: "required", - } as PerseusNumericInputAnswerForm; - }); - - let examples = _.map(forms, (form) => { - return formExamples[form.name](form, this.context.strings); - }); - // Ensure no duplicate tooltip text from simplified and unsimplified - // versions of the same format - examples = _.uniq(examples); - - return [this.context.strings.yourAnswer].concat(examples); - } - - shouldShowExamples: () => boolean = () => { - const noFormsAccepted = this.props.answerForms?.length === 0; - // To check if all answer forms are accepted, we must first - // find the *names* of all accepted forms, and see if they are - // all present, ignoring duplicates - const answerFormNames: ReadonlyArray = _.uniq( - this.props.answerForms?.map((form) => form.name), - ); - const allFormsAccepted = - answerFormNames.length >= Object.keys(formExamples).length; - return !noFormsAccepted && !allFormsAccepted; - }; - - focus: () => boolean = () => { - this.inputRef?.focus(); - return true; - }; - - focusInputPath: () => void = () => { - this.inputRef?.focus(); - }; - - blurInputPath: () => void = () => { - this.inputRef?.blur(); - }; - - getInputPaths: () => ReadonlyArray> = () => { - // The widget itself is an input, so we return a single empty list to - // indicate this. - /* c8 ignore next */ - return [[]]; - }; - - setInputValue: ( - arg1: FocusPath, - arg2: string, - arg3?: () => unknown | null | undefined, - ) => void = (path, newValue, cb) => { - /* c8 ignore next */ - this.props.onChange( - { - currentValue: newValue, +import {type NumericInputProps} from "./numeric-input.class"; +import {generateExamples, shouldShowExamples} from "./utils"; + +type InputRefType = SimpleKeypadInput | InputWithExamples | null; + +/** + * The NumericInputComponent is a child component of the NumericInput class + * component. It is responsible for rendering the UI elements of the Numeric + * Input widget. + */ +export const NumericInputComponent = forwardRef( + (props: NumericInputProps, ref) => { + const context = useContext(PerseusI18nContext); + const inputRef = useRef(null); + const [isFocused, setIsFocused] = useState(false); + + // Pass the focus and blur methods to the Numeric Input Class component + useImperativeHandle(ref, () => ({ + focus: () => { + if (inputRef.current) { + inputRef.current.focus(); + setIsFocused(true); + } }, - cb, - ); - }; - - getUserInput(): PerseusNumericInputUserInput { - return NumericInput.getUserInputFromProps(this.props); - } - - getPromptJSON(): NumericInputPromptJSON { - return _getPromptJSON(this.props, this.getUserInput()); - } - - handleChange: ( - arg1: string, - arg2?: () => unknown | null | undefined, - ) => void = (newValue, cb) => { - this.props.onChange({currentValue: newValue}, cb); - this.props.trackInteraction(); - }; - - _handleFocus: () => void = () => { - this.props.onFocus([]); - this.setState((currentState) => { - return {...currentState, isFocused: true}; - }); - }; - - _handleBlur: () => void = () => { - this.props.onBlur([]); - this.setState((currentState) => { - return {...currentState, isFocused: false}; - }); - }; + blur: () => { + if (inputRef.current) { + inputRef.current.blur(); + setIsFocused(false); + } + }, + })); + + const handleChange = ( + newValue: string, + cb?: () => unknown | null | undefined, + ): void => { + props.onChange({currentValue: newValue}, cb); + props.trackInteraction(); + }; - render(): React.ReactNode { - let labelText = this.props.labelText; - if (labelText == null || labelText === "") { - labelText = this.context.strings.yourAnswerLabel; - } + const handleFocus = (): void => { + props.onFocus([]); + setIsFocused(true); + }; - // To right align a custom keypad we need to wrap it. - const maybeRightAlignKeypadInput = ( - keypadInput: React.ReactElement< - React.ComponentProps - >, - ) => { - return this.props.rightAlign ? ( -
{keypadInput}
- ) : ( - keypadInput - ); + const handleBlur = (): void => { + props.onBlur([]); + setIsFocused(false); }; - if (this.props.apiOptions.customKeypad) { - // TODO(charlie): Support "Review Mode". - return maybeRightAlignKeypadInput( - (this.inputRef = ref)} - value={this.props.currentValue} - keypadElement={this.props.keypadElement} - onChange={this.handleChange} - onFocus={this._handleFocus} - onBlur={this._handleBlur} - />, - ); + // If the labelText is not provided by the Content Creators, use the default label text + let labelText = props.labelText; + if (labelText == null || labelText === "") { + labelText = context.strings.yourAnswerLabel; } - // Note: This is _very_ similar to what `input-number.jsx` does. If - // you modify this, double-check if you also need to modify that - // component. + // Styles for the InputWithExamples const styles = StyleSheet.create({ - input: { + inputWithExamples: { borderRadius: "3px", - borderWidth: this.state.isFocused ? "2px" : "1px", + borderWidth: isFocused ? "2px" : "1px", display: "inline-block", fontFamily: `Symbola, "Times New Roman", serif`, fontSize: "18px", height: "32px", lineHeight: "18px", - padding: this.state.isFocused ? "4px" : "4px 5px", // account for added focus border thickness - textAlign: this.props.rightAlign ? "right" : "left", - width: this.props.size === "small" ? 40 : 80, + padding: isFocused ? "4px" : "4px 5px", + textAlign: props.rightAlign ? "right" : "left", + width: props.size === "small" ? 40 : 80, }, }); + // (mobile-only) If the custom keypad is enabled, use the SimpleKeypadInput component + if (props.apiOptions.customKeypad) { + const alignmentClass = props.rightAlign + ? "perseus-input-right-align" + : undefined; + return ( +
+ } + value={props.currentValue} + keypadElement={props.keypadElement} + onChange={handleChange} + onFocus={handleFocus} + onBlur={handleBlur} + /> +
+ ); + } + // (desktop-only) Otherwise, use the InputWithExamples component return ( (this.inputRef = ref)} - value={this.props.currentValue} - onChange={this.handleChange} + ref={inputRef as React.RefObject} + value={props.currentValue} + onChange={handleChange} labelText={labelText} - examples={this.examples()} - shouldShowExamples={this.shouldShowExamples()} - onFocus={this._handleFocus} - onBlur={this._handleBlur} - id={this.props.widgetId} - disabled={this.props.apiOptions.readOnly} - style={styles.input} + examples={generateExamples(props.answerForms, context.strings)} + shouldShowExamples={shouldShowExamples(props.answerForms)} + onFocus={handleFocus} + onBlur={handleBlur} + id={props.widgetId} + disabled={props.apiOptions.readOnly} + style={styles.inputWithExamples} /> ); - } -} - -// TODO(thomas): Currently we receive a list of lists of acceptable answer types -// and union them down into a single set. It's worth considering whether it -// wouldn't make more sense to have a single set of acceptable answer types for -// a given *problem* rather than for each possible [correct/wrong] *answer*. -// When should two answers to a problem take different answer types? -// See D27790 for more discussion. -export const unionAnswerForms: ( - arg1: ReadonlyArray>, -) => ReadonlyArray = function (answerFormsList) { - // Takes a list of lists of answer forms, and returns a list of the forms - // in each of these lists in the same order that they're listed in the - // `formExamples` forms from above. - - // uniqueBy takes a list of elements and a function which compares whether - // two elements are equal, and returns a list of unique elements. This is - // just a helper function here, but works generally. - const uniqueBy = function (list, iteratee: any) { - // @ts-expect-error - TS2347 - Untyped function calls may not accept type arguments. - return list.reduce>((uniqueList, element) => { - // For each element, decide whether it's already in the list of - // unique items. - const inList = _.find(uniqueList, iteratee.bind(null, element)); - if (inList) { - return uniqueList; - } - return uniqueList.concat([element]); - }, []); - }; - - // Pull out all of the forms from the different lists. - const allForms = answerFormsList.flat(); - // Pull out the unique forms using uniqueBy. - const uniqueForms = uniqueBy(allForms, _.isEqual); - // Sort them by the order they appear in the `formExamples` list. - const formExampleKeys = Object.keys(formExamples); - return _.sortBy(uniqueForms, (form) => { - return formExampleKeys.indexOf(form.name); - }); -}; - -type RenderProps = { - answerForms: ReadonlyArray<{ - simplify: "required" | "correct" | "enforced" | null | undefined; - name: "integer" | "decimal" | "proper" | "improper" | "mixed" | "pi"; - }>; - labelText: string; - size: "normal" | "small"; - coefficient: boolean; - rightAlign?: boolean; - static: boolean; -}; - -const propsTransform = function ( - widgetOptions: PerseusNumericInputWidgetOptions, -): RenderProps { - const rendererProps = _.extend(_.omit(widgetOptions, "answers"), { - answerForms: unionAnswerForms( - // Pull out the name of each form and whether that form has - // required simplification. - widgetOptions.answers.map((answer) => { - // @ts-expect-error - TS2345 - Argument of type 'readonly MathFormat[] | undefined' is not assignable to parameter of type 'Collection'. - return _.map(answer.answerForms, (form) => { - return { - simplify: answer.simplify, - name: form, - }; - }); - }), - ), - }); - - return rendererProps; -}; - -export default { - name: "numeric-input", - displayName: "Numeric input", - defaultAlignment: "inline-block", - accessible: true, - widget: NumericInput, - transform: propsTransform, - isLintable: true, - // TODO(LEMS-2656): remove TS suppression - // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusNumericInputUserInput'. - scorer: scoreNumericInput, - - // TODO(LEMS-2656): remove TS suppression - // @ts-expect-error: Type 'Rubric' is not assignable to type 'PerseusNumericInputRubric' - getOneCorrectAnswerFromRubric( - rubric: PerseusNumericInputRubric, - ): string | null | undefined { - const correctAnswers = rubric.answers.filter( - (answer) => answer.status === "correct", - ); - const answerStrings = correctAnswers.map((answer) => { - // Figure out how this answer is supposed to be - // displayed - let format = "decimal"; - if (answer.answerForms && answer.answerForms[0]) { - // NOTE(johnsullivan): This isn't exactly ideal, but - // it does behave well for all the currently known - // problems. See D14742 for some discussion on - // alternate strategies. - format = answer.answerForms[0]; - } - - // @ts-expect-error - TS2345 - Argument of type 'string' is not assignable to parameter of type 'MathFormat | undefined'. - let answerString = KhanMath.toNumericString(answer.value, format); - if (answer.maxError) { - answerString += - " \u00B1 " + - // @ts-expect-error - TS2345 - Argument of type 'string' is not assignable to parameter of type 'MathFormat | undefined'. - KhanMath.toNumericString(answer.maxError, format); - } - return answerString; - }); - if (answerStrings.length === 0) { - return; - } - return answerStrings[0]; }, -} satisfies WidgetExports; +); diff --git a/packages/perseus/src/widgets/numeric-input/utils.test.ts b/packages/perseus/src/widgets/numeric-input/utils.test.ts new file mode 100644 index 0000000000..2e3dc3200a --- /dev/null +++ b/packages/perseus/src/widgets/numeric-input/utils.test.ts @@ -0,0 +1,84 @@ +import {generateExamples, shouldShowExamples} from "./utils"; + +import type {PerseusStrings} from "../../strings"; +import type {PerseusNumericInputAnswerForm} from "@khanacademy/perseus-core"; + +describe("generateExamples", () => { + it("returns an array of examples", () => { + const answerForms: readonly PerseusNumericInputAnswerForm[] = [ + { + name: "integer", + simplify: "optional", + }, + { + name: "proper", + simplify: "required", + }, + ]; + const strings: Partial = { + yourAnswer: "Your answer", + integerExample: "Integer example", + properExample: "Proper example", + simplifiedProperExample: "Simplified proper example", + }; + const expected = [ + "Your answer", + "Integer example", + "Simplified proper example", + ]; + expect( + generateExamples(answerForms, strings as PerseusStrings), + ).toEqual(expected); + }); +}); + +describe("shouldShowExamples", () => { + it("returns true when not all forms are accepted", () => { + const answerForms: readonly PerseusNumericInputAnswerForm[] = [ + { + name: "integer", + simplify: "optional", + }, + { + name: "proper", + simplify: "required", + }, + ]; + expect(shouldShowExamples(answerForms)).toBe(true); + }); + + it("returns false when all forms are accepted", () => { + const answerForms: readonly PerseusNumericInputAnswerForm[] = [ + { + name: "integer", + simplify: "optional", + }, + { + name: "proper", + simplify: "required", + }, + { + name: "improper", + simplify: "optional", + }, + { + name: "mixed", + simplify: "required", + }, + { + name: "decimal", + simplify: "optional", + }, + { + name: "pi", + simplify: "required", + }, + ]; + expect(shouldShowExamples(answerForms)).toBe(false); + }); + + it("returns false when no forms are accepted", () => { + const answerForms = []; + expect(shouldShowExamples(answerForms)).toBe(false); + }); +}); diff --git a/packages/perseus/src/widgets/numeric-input/utils.ts b/packages/perseus/src/widgets/numeric-input/utils.ts new file mode 100644 index 0000000000..dd4f13e580 --- /dev/null +++ b/packages/perseus/src/widgets/numeric-input/utils.ts @@ -0,0 +1,71 @@ +import _ from "underscore"; + +import type {PerseusStrings} from "../../strings"; +import type {PerseusNumericInputAnswerForm} from "@khanacademy/perseus-core"; + +/** + * The full list of available strings for the numeric input widget, + * based on whether the Content Creator has specified that the answer must be simplified. + */ +export const NumericExampleStrings: { + [key: string]: ( + form: PerseusNumericInputAnswerForm, + strings: PerseusStrings, + ) => string; +} = { + integer: (form, strings: PerseusStrings) => strings.integerExample, + proper: (form, strings: PerseusStrings) => + form.simplify === "optional" + ? strings.properExample + : strings.simplifiedProperExample, + improper: (form, strings: PerseusStrings) => + form.simplify === "optional" + ? strings.improperExample + : strings.simplifiedImproperExample, + mixed: (form, strings: PerseusStrings) => strings.mixedExample, + decimal: (form, strings: PerseusStrings) => strings.decimalExample, + pi: (form, strings: PerseusStrings) => strings.piExample, +}; + +/** + * Generates the specific set of examples for the current question. + * This string is shown as examples to the user in a tooltip. + */ +export const generateExamples = ( + answerForms: readonly PerseusNumericInputAnswerForm[], + strings: PerseusStrings, +): ReadonlyArray => { + const forms = + answerForms?.length !== 0 + ? answerForms + : Object.keys(NumericExampleStrings).map((name) => { + return { + name: name, + simplify: "required", + } as PerseusNumericInputAnswerForm; + }); + + let examples = _.map(forms, (form) => { + return NumericExampleStrings[form.name](form, strings); + }); + examples = _.uniq(examples); + + return [strings.yourAnswer].concat(examples); +}; + +/** + * Determines whether to show examples of how to input + * the various supported answer forms. We do not show examples + * if all forms are accepted or if no forms are accepted. + */ +export const shouldShowExamples = ( + answerForms: readonly PerseusNumericInputAnswerForm[], +): boolean => { + const noFormsAccepted = answerForms?.length === 0; + const answerFormNames: ReadonlyArray = _.uniq( + answerForms?.map((form) => form.name), + ); + const allFormsAccepted = + answerFormNames.length >= Object.keys(NumericExampleStrings).length; + return !noFormsAccepted && !allFormsAccepted; +}; From 4ca8c00f6b2c97325fe7513c539c896ec80cc2ae Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 23 Jan 2025 12:21:37 -0800 Subject: [PATCH 02/11] Refactoring Numeric Input helper functions to remove underscore (#2128) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR is part of the Numeric Input Project, and will be landed onto the feature branch for a full QA pass. The intended goal was to remove all cases of underscore in the Numeric Input component, and to improve the commenting / documentation of the code. Some things to note: - I've changed the logic of `generateExamples` to match `shouldShowExamples`, as we were generating a list of all possible examples and simply not displaying it. This seemed unnecessary and we can exit both functions early. - I've added more specific types for `PerseusNumericInputWidgetOptions.simplify` Issue: LEMS-2446 - New tests - Manual testing in storybook Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald, jeremywiebe Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/2128 --- .changeset/sharp-peaches-love.md | 7 + packages/perseus-core/src/data-schema.ts | 16 +- .../perseus-parsers/numeric-input-widget.ts | 18 +- .../parse-perseus-json-snapshot.test.ts.snap | 4 +- .../numeric-input/score-numeric-input.test.ts | 34 +- .../src/components/input-with-examples.tsx | 2 +- .../graded-group-set-jipt.test.ts.snap | 381 +----------------- .../graded-group-set.test.ts.snap | 127 +----- .../group/__snapshots__/group.test.tsx.snap | 254 +----------- .../__snapshots__/numeric-input.test.ts.snap | 254 +----------- .../numeric-input/numeric-input.class.tsx | 68 +--- .../numeric-input/numeric-input.test.ts | 38 +- .../widgets/numeric-input/numeric-input.tsx | 1 - .../src/widgets/numeric-input/utils.test.ts | 145 ++++++- .../src/widgets/numeric-input/utils.ts | 92 ++++- 15 files changed, 286 insertions(+), 1155 deletions(-) create mode 100644 .changeset/sharp-peaches-love.md diff --git a/.changeset/sharp-peaches-love.md b/.changeset/sharp-peaches-love.md new file mode 100644 index 0000000000..4beddbc884 --- /dev/null +++ b/.changeset/sharp-peaches-love.md @@ -0,0 +1,7 @@ +--- +"@khanacademy/math-input": minor +"@khanacademy/perseus": minor +"@khanacademy/perseus-core": minor +--- + +Refactoring Numeric Input helper functions to remove underscore, improve documentation, and add tests. diff --git a/packages/perseus-core/src/data-schema.ts b/packages/perseus-core/src/data-schema.ts index d60e3af3ca..d3f41751ce 100644 --- a/packages/perseus-core/src/data-schema.ts +++ b/packages/perseus-core/src/data-schema.ts @@ -1178,16 +1178,16 @@ export type MathFormat = | "pi"; export type PerseusNumericInputAnswerForm = { - simplify: - | "required" - | "correct" - | "enforced" - | "optional" - | null - | undefined; + simplify: PerseusNumericInputSimplify | null | undefined; name: MathFormat; }; +export type PerseusNumericInputSimplify = + | "required" + | "correct" + | "enforced" + | "optional"; + export type PerseusNumericInputWidgetOptions = { // A list of all the possible correct and incorrect answers answers: ReadonlyArray; @@ -1224,7 +1224,7 @@ export type PerseusNumericInputAnswer = { // NOTE: perseus_data.go says this is non-nullable even though we handle null values. maxError: number | null | undefined; // Unsimplified answers are Ungraded, Accepted, or Wrong. Options: "required", "correct", or "enforced" - simplify: string | null | undefined; + simplify: PerseusNumericInputSimplify | null | undefined; }; export type PerseusNumberLineWidgetOptions = { diff --git a/packages/perseus-core/src/parse-perseus-json/perseus-parsers/numeric-input-widget.ts b/packages/perseus-core/src/parse-perseus-json/perseus-parsers/numeric-input-widget.ts index 6c4191258a..15e07526b1 100644 --- a/packages/perseus-core/src/parse-perseus-json/perseus-parsers/numeric-input-widget.ts +++ b/packages/perseus-core/src/parse-perseus-json/perseus-parsers/numeric-input-widget.ts @@ -29,6 +29,13 @@ const parseMathFormat = enumeration( "pi", ); +const parseSimplify = enumeration( + "required", + "correct", + "enforced", + "optional", +); + export const parseNumericInputWidget: Parser = parseWidget( constant("numeric-input"), object({ @@ -48,8 +55,15 @@ export const parseNumericInputWidget: Parser = parseWidget( // the data, we should simplify `simplify`. simplify: optional( nullable( - union(string).or( - pipeParsers(boolean).then(convert(String)).parser, + union(parseSimplify).or( + pipeParsers(boolean).then( + convert((value) => { + if (typeof value === "boolean") { + return value ? "required" : "optional"; + } + return value; + }), + ).parser, ).parser, ), ), diff --git a/packages/perseus-core/src/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap b/packages/perseus-core/src/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap index 48cd27db6f..de4da55d33 100644 --- a/packages/perseus-core/src/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap +++ b/packages/perseus-core/src/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap @@ -5579,7 +5579,7 @@ exports[`parseAndTypecheckPerseusItem correctly parses data/numeric-input-answer { "maxError": 0, "message": "", - "simplify": "true", + "simplify": "required", "status": "correct", "strict": false, "value": 1.125, @@ -6082,7 +6082,7 @@ exports[`parseAndTypecheckPerseusItem correctly parses data/numeric-input-with-s { "maxError": 0, "message": "", - "simplify": "false", + "simplify": "optional", "status": "correct", "strict": false, "value": 2.6, diff --git a/packages/perseus-score/src/widgets/numeric-input/score-numeric-input.test.ts b/packages/perseus-score/src/widgets/numeric-input/score-numeric-input.test.ts index e00565beb3..8e7d58be52 100644 --- a/packages/perseus-score/src/widgets/numeric-input/score-numeric-input.test.ts +++ b/packages/perseus-score/src/widgets/numeric-input/score-numeric-input.test.ts @@ -36,7 +36,7 @@ describe("scoreNumericInput", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -60,7 +60,7 @@ describe("scoreNumericInput", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -149,7 +149,7 @@ describe("scoreNumericInput", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: true, message: "", }, @@ -173,7 +173,7 @@ describe("scoreNumericInput", () => { value: 1, status: "correct", maxError: 0.2, - simplify: "", + simplify: "optional", strict: true, message: "", }, @@ -197,7 +197,7 @@ describe("scoreNumericInput", () => { value: 1, status: "correct", maxError: 0.2, - simplify: "", + simplify: "optional", strict: true, message: "", }, @@ -223,7 +223,7 @@ describe("scoreNumericInput", () => { value: 4, status: "wrong", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -232,7 +232,7 @@ describe("scoreNumericInput", () => { value: 10, status: "correct", maxError: 10, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -267,7 +267,7 @@ describe("scoreNumericInput", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -275,7 +275,7 @@ describe("scoreNumericInput", () => { value: -1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -313,7 +313,7 @@ describe("scoreNumericInput", () => { // This answer is missing its value field. status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -322,7 +322,7 @@ describe("scoreNumericInput", () => { value: 0.5, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -346,7 +346,7 @@ describe("scoreNumericInput", () => { value: null, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -355,7 +355,7 @@ describe("scoreNumericInput", () => { value: 0.5, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -375,7 +375,7 @@ describe("scoreNumericInput", () => { value: 0.2, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -398,7 +398,7 @@ describe("scoreNumericInput", () => { value: 1.2, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -421,7 +421,7 @@ describe("scoreNumericInput", () => { value: 1.1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -441,7 +441,7 @@ describe("scoreNumericInput", () => { value: 0.9, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, diff --git a/packages/perseus/src/components/input-with-examples.tsx b/packages/perseus/src/components/input-with-examples.tsx index 6a8a575851..3010ddcfa6 100644 --- a/packages/perseus/src/components/input-with-examples.tsx +++ b/packages/perseus/src/components/input-with-examples.tsx @@ -93,7 +93,7 @@ class InputWithExamples extends React.Component { const ariaId = `aria-for-${id}`; // Generate text from a known set of format options that will read well in a screen reader const examplesAria = - this.props.examples.length === 7 + this.props.examples.length === 0 ? "" : `${this.props.examples[0]} ${this.props.examples.slice(1).join(", or\n")}` diff --git a/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set-jipt.test.ts.snap b/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set-jipt.test.ts.snap index b6fcd2a412..4429d79533 100644 --- a/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set-jipt.test.ts.snap +++ b/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set-jipt.test.ts.snap @@ -109,132 +109,11 @@ exports[`graded-group-set should render all graded groups 1`] = ` class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
@@ -352,132 +231,11 @@ exports[`graded-group-set should render all graded groups 1`] = ` class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
@@ -595,132 +353,11 @@ exports[`graded-group-set should render all graded groups 1`] = ` class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
diff --git a/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set.test.ts.snap b/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set.test.ts.snap index b53156c59c..c5338f44a3 100644 --- a/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set.test.ts.snap +++ b/packages/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set.test.ts.snap @@ -175,132 +175,11 @@ exports[`graded group widget should snapshot 1`] = ` class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
diff --git a/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap b/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap index 41f660e5d7..c068d366ed 100644 --- a/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap +++ b/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap @@ -802,132 +802,11 @@ exports[`group widget should snapshot: initial render 1`] = ` class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
@@ -1023,132 +902,11 @@ exports[`group widget should snapshot: initial render 1`] = ` class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
diff --git a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap index b65dcea189..87aa1d4461 100644 --- a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap +++ b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap @@ -142,132 +142,11 @@ exports[`numeric-input widget Should render predictably: after interaction 1`] = class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
@@ -360,132 +239,11 @@ exports[`numeric-input widget Should render predictably: first render 1`] = ` class="perseus-renderer perseus-renderer-responsive" >
-
- - Your answer should be - - -
-
-
-
    -
  • - an integer, like - - - - 6 - - - -
    • -
  • - a - - simplified proper - - fraction, like - - - - 3/5 - - - -
    • -
  • - a - - simplified improper - - fraction, like - - - - 7/4 - - - -
    • -
  • - a mixed number, like - - - - 1\\ 3/4 - - - -
    • -
  • - an - - exact - - decimal, like - - - - 0.75 - - - -
    • -
  • - a multiple of pi, like - - - - 12\\ \\text{pi} - - - - or - - - - 2/3\\ \\text{pi} - - - -
    • -
+ +
diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx index e782cc403f..44b8fede57 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx @@ -1,14 +1,12 @@ import {KhanMath} from "@khanacademy/kmath"; import {linterContextDefault} from "@khanacademy/perseus-linter"; import * as React from "react"; -import _ from "underscore"; import {ApiOptions} from "../../perseus-api"; import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/numeric-input/prompt-utils"; import {NumericInputComponent} from "./numeric-input"; -import {scoreNumericInput} from "@khanacademy/perseus-score"; -import {NumericExampleStrings} from "./utils"; +import {unionAnswerForms} from "./utils"; import type InputWithExamples from "../../components/input-with-examples"; import type SimpleKeypadInput from "../../components/simple-keypad-input"; @@ -157,70 +155,26 @@ export class NumericInput } } -// TODO(thomas): Currently we receive a list of lists of acceptable answer types -// and union them down into a single set. It's worth considering whether it -// wouldn't make more sense to have a single set of acceptable answer types for -// a given *problem* rather than for each possible [correct/wrong] *answer*. -// When should two answers to a problem take different answer types? -// See D27790 for more discussion. -export const unionAnswerForms: ( - answerFormsList: ReadonlyArray< - ReadonlyArray - >, -) => ReadonlyArray = function (answerFormsList) { - // Takes a list of lists of answer forms, and returns a list of the forms - // in each of these lists in the same order that they're listed in the - // `formExamples` forms from above. - - // uniqueBy takes a list of elements and a function which compares whether - // two elements are equal, and returns a list of unique elements. This is - // just a helper function here, but works generally. - const uniqueBy = function (list, iteratee: any) { - // @ts-expect-error - TS2347 - Untyped function calls may not accept type arguments. - return list.reduce>((uniqueList, element) => { - // For each element, decide whether it's already in the list of - // unique items. - const inList = _.find(uniqueList, iteratee.bind(null, element)); - if (inList) { - return uniqueList; - } - return uniqueList.concat([element]); - }, []); - }; - - // Pull out all of the forms from the different lists. - const allForms = answerFormsList.flat(); - // Pull out the unique forms using uniqueBy. - const uniqueForms = uniqueBy(allForms, _.isEqual); - // Sort them by the order they appear in the `formExamples` list. - const formExampleKeys = Object.keys(NumericExampleStrings); - return _.sortBy(uniqueForms, (form) => { - return formExampleKeys.indexOf(form.name); - }); -}; - type RenderProps = { - answerForms: ReadonlyArray<{ - simplify: "required" | "correct" | "enforced" | null | undefined; - name: "integer" | "decimal" | "proper" | "improper" | "mixed" | "pi"; - }>; - labelText: string; - size: "normal" | "small"; + answerForms: ReadonlyArray; + labelText?: string; + size: string; coefficient: boolean; rightAlign?: boolean; static: boolean; }; +// Transforms the widget options to the props used to render the widget. const propsTransform = function ( widgetOptions: PerseusNumericInputWidgetOptions, ): RenderProps { - const rendererProps = _.extend(_.omit(widgetOptions, "answers"), { + // Omit the answers from the widget options since they are + // not needed for rendering the widget. + const {answers: _, ...rendererProps} = { + ...widgetOptions, answerForms: unionAnswerForms( - // Pull out the name of each form and whether that form has - // required simplification. widgetOptions.answers.map((answer) => { - // @ts-expect-error - TS2345 - Argument of type 'readonly MathFormat[] | undefined' is not assignable to parameter of type 'Collection'. - return _.map(answer.answerForms, (form) => { + return (answer.answerForms || []).map((form) => { return { simplify: answer.simplify, name: form, @@ -228,7 +182,7 @@ const propsTransform = function ( }); }), ), - }); + }; return rendererProps; }; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts index eae8966e56..ec3431ccbb 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts @@ -6,9 +6,7 @@ import * as Dependencies from "../../dependencies"; import {scorePerseusItemTesting} from "../../util/test-utils"; import {renderQuestion} from "../__testutils__/renderQuestion"; -import NumericInputWidgetExport, { - unionAnswerForms, -} from "./numeric-input.class"; +import NumericInputWidgetExport from "./numeric-input.class"; import { question1AndAnswer, multipleAnswers, @@ -213,7 +211,7 @@ describe("static function getOneCorrectAnswerFromRubric", () => { value: 1.0, maxError: 0.2, answerForms: ["decimal"], - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -376,35 +374,3 @@ describe("Numeric input widget", () => { ); }); }); - -describe("unionAnswerForms utility function", () => { - beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); - }); - - it("removes duplicates", () => { - // arrange - const forms = [ - [ - { - simplify: "required" as const, - name: "integer", - } as const, - ], - [ - { - simplify: "required" as const, - name: "integer", - } as const, - ], - ]; - - // act - const result = unionAnswerForms(forms); - - // assert - expect(result).toHaveLength(1); - }); -}); diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx index 4fbffacf19..5fd5c04099 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx @@ -7,7 +7,6 @@ import { useRef, useState, } from "react"; -import _ from "underscore"; import {PerseusI18nContext} from "../../components/i18n-context"; import InputWithExamples from "../../components/input-with-examples"; diff --git a/packages/perseus/src/widgets/numeric-input/utils.test.ts b/packages/perseus/src/widgets/numeric-input/utils.test.ts index 2e3dc3200a..e4139bf17f 100644 --- a/packages/perseus/src/widgets/numeric-input/utils.test.ts +++ b/packages/perseus/src/widgets/numeric-input/utils.test.ts @@ -1,10 +1,38 @@ -import {generateExamples, shouldShowExamples} from "./utils"; +import {mockStrings} from "../../strings"; + +import {generateExamples, shouldShowExamples, unionAnswerForms} from "./utils"; -import type {PerseusStrings} from "../../strings"; import type {PerseusNumericInputAnswerForm} from "@khanacademy/perseus-core"; describe("generateExamples", () => { it("returns an array of examples", () => { + // Arrange + const answerForms: readonly PerseusNumericInputAnswerForm[] = [ + { + name: "integer", + simplify: "optional", + }, + { + name: "proper", + simplify: "required", + }, + ]; + + const expected = [ + "**Your answer should be** ", + "an integer, like $6$", + "a *simplified proper* fraction, like $3/5$", + ]; + + // Act + const result = generateExamples(answerForms, mockStrings); + + // Assert + expect(result).toEqual(expected); + }); + + it("returns only unique entries in an array of examples", () => { + // Arrange const answerForms: readonly PerseusNumericInputAnswerForm[] = [ { name: "integer", @@ -14,26 +42,42 @@ describe("generateExamples", () => { name: "proper", simplify: "required", }, + { + name: "integer", + simplify: "required", + }, ]; - const strings: Partial = { - yourAnswer: "Your answer", - integerExample: "Integer example", - properExample: "Proper example", - simplifiedProperExample: "Simplified proper example", - }; + const expected = [ - "Your answer", - "Integer example", - "Simplified proper example", + "**Your answer should be** ", + "an integer, like $6$", + "a *simplified proper* fraction, like $3/5$", ]; - expect( - generateExamples(answerForms, strings as PerseusStrings), - ).toEqual(expected); + + // Act + const result = generateExamples(answerForms, mockStrings); + + // Assert + expect(result).toEqual(expected); + }); + + it("returns an empty array if no answer forms are provided", () => { + // Arrange + const answerForms: readonly PerseusNumericInputAnswerForm[] = []; + + const expected = []; + + // Act + const result = generateExamples(answerForms, mockStrings); + + // Assert + expect(result).toEqual(expected); }); }); describe("shouldShowExamples", () => { - it("returns true when not all forms are accepted", () => { + it("returns true when only some forms are accepted", () => { + // Arrange const answerForms: readonly PerseusNumericInputAnswerForm[] = [ { name: "integer", @@ -44,10 +88,16 @@ describe("shouldShowExamples", () => { simplify: "required", }, ]; - expect(shouldShowExamples(answerForms)).toBe(true); + + // Act + const result = shouldShowExamples(answerForms); + + // Assert + expect(result).toBe(true); }); it("returns false when all forms are accepted", () => { + // Arrange const answerForms: readonly PerseusNumericInputAnswerForm[] = [ { name: "integer", @@ -74,11 +124,70 @@ describe("shouldShowExamples", () => { simplify: "required", }, ]; - expect(shouldShowExamples(answerForms)).toBe(false); + + // Act + const result = shouldShowExamples(answerForms); + + // Assert + expect(result).toBe(false); }); it("returns false when no forms are accepted", () => { + // Arrange const answerForms = []; - expect(shouldShowExamples(answerForms)).toBe(false); + + // Act + const result = shouldShowExamples(answerForms); + + // Assert + expect(result).toBe(false); + }); +}); + +describe("unionAnswerForms", () => { + it("returns an array of unique answer forms in the order provided", () => { + // Arrange + const answerForms: readonly PerseusNumericInputAnswerForm[][] = [ + [ + { + name: "integer", + simplify: "optional", + }, + { + name: "proper", + simplify: "required", + }, + ], + [ + { + name: "integer", + simplify: "optional", + }, + { + name: "improper", + simplify: "required", + }, + ], + ]; + const expected: readonly PerseusNumericInputAnswerForm[] = [ + { + name: "integer", + simplify: "optional", + }, + { + name: "proper", + simplify: "required", + }, + { + name: "improper", + simplify: "required", + }, + ]; + + // Act + const result = unionAnswerForms(answerForms); + + // Assert + expect(result).toEqual(expected); }); }); diff --git a/packages/perseus/src/widgets/numeric-input/utils.ts b/packages/perseus/src/widgets/numeric-input/utils.ts index dd4f13e580..7452282e82 100644 --- a/packages/perseus/src/widgets/numeric-input/utils.ts +++ b/packages/perseus/src/widgets/numeric-input/utils.ts @@ -1,5 +1,3 @@ -import _ from "underscore"; - import type {PerseusStrings} from "../../strings"; import type {PerseusNumericInputAnswerForm} from "@khanacademy/perseus-core"; @@ -35,37 +33,89 @@ export const generateExamples = ( answerForms: readonly PerseusNumericInputAnswerForm[], strings: PerseusStrings, ): ReadonlyArray => { - const forms = - answerForms?.length !== 0 - ? answerForms - : Object.keys(NumericExampleStrings).map((name) => { - return { - name: name, - simplify: "required", - } as PerseusNumericInputAnswerForm; - }); + // If the Content Creator has not specified any answer forms, + // we do not need to show any examples. + if (answerForms.length === 0) { + return []; + } + + // Generate a list of the unique answer forms. + const uniqueForms = getUniqueAnswerForms(answerForms); - let examples = _.map(forms, (form) => { + // Generate the example strings for each unique form. + const examples = uniqueForms.map((form) => { return NumericExampleStrings[form.name](form, strings); }); - examples = _.uniq(examples); + // Add the "Your answer" string to the beginning of the examples list. return [strings.yourAnswer].concat(examples); }; /** - * Determines whether to show examples of how to input - * the various supported answer forms. We do not show examples - * if all forms are accepted or if no forms are accepted. + * Determines whether to show examples of how to input the various supported answer forms. + * We do not show examples if all forms are accepted or if no forms are accepted. */ export const shouldShowExamples = ( answerForms: readonly PerseusNumericInputAnswerForm[], ): boolean => { - const noFormsAccepted = answerForms?.length === 0; - const answerFormNames: ReadonlyArray = _.uniq( - answerForms?.map((form) => form.name), - ); + // If the Content Creator has not specified any answer forms, + // we do not need to show any examples. + if (answerForms.length === 0) { + return false; + } + + // Generate a list of the unique names of the selected answer forms. + const answerFormNames: ReadonlyArray = getUniqueAnswerForms( + answerForms, + ).map((form) => form.name); + + // If all forms are accepted, we do not need to show any examples. const allFormsAccepted = answerFormNames.length >= Object.keys(NumericExampleStrings).length; - return !noFormsAccepted && !allFormsAccepted; + + return !allFormsAccepted; +}; + +/** + * uniqueAnswerForms takes a list of answer forms and returns a list of unique + * answer forms. This is useful for ensuring that we don't show duplicate examples + * to the user. + */ +const getUniqueAnswerForms = function ( + list: readonly PerseusNumericInputAnswerForm[], +): PerseusNumericInputAnswerForm[] { + // We use a Set to keep track of the forms we've already seen. + const foundForms = new Set(); + return list.filter((form) => { + // If we've already seen this form, skip it. + if (foundForms.has(form.name)) { + return false; + } + // Otherwise, add it to the set and return true. + foundForms.add(form.name); + return true; + }); +}; + +/** + * Takes a list of lists of answer forms, and returns a list of the forms + * in each of these lists in the same order that they're listed in the + * `formExamples` forms from above. + */ +export const unionAnswerForms: ( + answerFormsList: ReadonlyArray< + ReadonlyArray + >, +) => ReadonlyArray = function (answerFormsList) { + // Pull out all of the forms from the different lists. + const allForms = answerFormsList.flat(); + // Pull out the unique forms using getUniqueAnswerForms. + const uniqueForms = getUniqueAnswerForms(allForms); + // Sort them by the order they appear in the `formExamples` list. + const formExampleKeys = Object.keys(NumericExampleStrings); + return uniqueForms.sort((a, b) => { + return ( + formExampleKeys.indexOf(a.name) - formExampleKeys.indexOf(b.name) + ); + }); }; From 9bfbe9e346ee7218459a2bfb7ca70674adf1076f Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Tue, 28 Jan 2025 12:12:59 -0800 Subject: [PATCH 03/11] Modernization and Migration of InputWithExamples to NumericInput folder (#2121) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: This PR is part of the Numeric Input Project work. It is being landed onto the `feature/numeric-dx-refactor` branch. This PR contains the following changes: 1. Moves the InputWithExamples component to the NumericInput folder 2. Modernizes InputWithExamples to be a functional component 3. Addition of some comments Video Example of Snapshot Testing: https://github.com/user-attachments/assets/ca917778-50b0-46d2-89d8-dad95d1dadf2 Issue: LEMS-2785 ## Test plan: - Ensure all tests pass - Manual testing with PR Snapshot in upstream consumer - Landing onto feature branch that will see full QA regression pass before deployment Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (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), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/2121 --- .changeset/rich-flowers-prove.md | 6 + .../src/components/input-with-examples.tsx | 213 ------------------ .../src/widgets/input-number/input-number.tsx | 2 +- .../input-with-examples.stories.tsx | 2 +- .../numeric-input/input-with-examples.tsx | 193 ++++++++++++++++ .../numeric-input/numeric-input.class.tsx | 6 +- .../widgets/numeric-input/numeric-input.tsx | 11 +- 7 files changed, 210 insertions(+), 223 deletions(-) create mode 100644 .changeset/rich-flowers-prove.md delete mode 100644 packages/perseus/src/components/input-with-examples.tsx rename packages/perseus/src/{components/__stories__ => widgets/numeric-input}/input-with-examples.stories.tsx (93%) create mode 100644 packages/perseus/src/widgets/numeric-input/input-with-examples.tsx diff --git a/.changeset/rich-flowers-prove.md b/.changeset/rich-flowers-prove.md new file mode 100644 index 0000000000..1c1087443b --- /dev/null +++ b/.changeset/rich-flowers-prove.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/math-input": minor +"@khanacademy/perseus": minor +--- + +Modernization and Migration of InputWithExamples to NumericInput folder diff --git a/packages/perseus/src/components/input-with-examples.tsx b/packages/perseus/src/components/input-with-examples.tsx deleted file mode 100644 index 3010ddcfa6..0000000000 --- a/packages/perseus/src/components/input-with-examples.tsx +++ /dev/null @@ -1,213 +0,0 @@ -import * as PerseusLinter from "@khanacademy/perseus-linter"; -import * as React from "react"; -import _ from "underscore"; - -import {ClassNames as ApiClassNames} from "../perseus-api"; -import Renderer from "../renderer"; -import Util from "../util"; - -import {PerseusI18nContext} from "./i18n-context"; -import TextInput from "./text-input"; -import Tooltip, {HorizontalDirection, VerticalDirection} from "./tooltip"; - -import type {LinterContextProps} from "@khanacademy/perseus-linter"; -import type {StyleType} from "@khanacademy/wonder-blocks-core"; - -const {captureScratchpadTouchStart} = Util; - -type Props = { - value: string; - onChange: any; - className: string; - examples: ReadonlyArray; - shouldShowExamples: boolean; - convertDotToTimes?: boolean; - buttonSet?: string; - buttonsVisible?: "always" | "never" | "focused"; - labelText?: string; - onFocus: () => void; - onBlur: () => void; - disabled: boolean; - style?: StyleType; - id: string; - linterContext: LinterContextProps; -}; - -type DefaultProps = { - shouldShowExamples: Props["shouldShowExamples"]; - onFocus: Props["onFocus"]; - onBlur: Props["onBlur"]; - disabled: Props["disabled"]; - linterContext: Props["linterContext"]; - className: Props["className"]; -}; - -type State = { - focused: boolean; - showExamples: boolean; -}; - -class InputWithExamples extends React.Component { - static contextType = PerseusI18nContext; - declare context: React.ContextType; - - inputRef: React.RefObject; - - static defaultProps: DefaultProps = { - shouldShowExamples: true, - onFocus: function () {}, - onBlur: function () {}, - disabled: false, - linterContext: PerseusLinter.linterContextDefault, - className: "", - }; - - state: State = { - focused: false, - showExamples: false, - }; - - constructor(props: Props) { - super(props); - this.inputRef = React.createRef(); - } - - _getUniqueId: () => string = () => { - return `input-with-examples-${btoa(this.props.id).replace(/=/g, "")}`; - }; - - _getInputClassName: () => string = () => { - // Otherwise, we need to add these INPUT and FOCUSED tags here. - let className = ApiClassNames.INPUT + " " + ApiClassNames.INTERACTIVE; - if (this.state.focused) { - className += " " + ApiClassNames.FOCUSED; - } - if (this.props.className) { - className += " " + this.props.className; - } - return className; - }; - - _renderInput: () => any = () => { - const id = this._getUniqueId(); - const ariaId = `aria-for-${id}`; - // Generate text from a known set of format options that will read well in a screen reader - const examplesAria = - this.props.examples.length === 0 - ? "" - : `${this.props.examples[0]} - ${this.props.examples.slice(1).join(", or\n")}` - // @ts-expect-error TS2550: Property replaceAll does not exist on type string. - .replaceAll("*", "") - .replaceAll("$", "") - .replaceAll("\\ \\text{pi}", " pi") - .replaceAll("\\ ", " and "); - const inputProps = { - id: id, - "aria-describedby": ariaId, - ref: this.inputRef, - className: this._getInputClassName(), - labelText: this.props.labelText, - value: this.props.value, - onFocus: this._handleFocus, - onBlur: this._handleBlur, - disabled: this.props.disabled, - style: this.props.style, - onChange: this.props.onChange, - onTouchStart: captureScratchpadTouchStart, - autoCapitalize: "off", - autoComplete: "off", - autoCorrect: "off", - spellCheck: "false", - }; - return ( - <> - - - {examplesAria} - - - ); - }; - - _handleFocus: () => void = () => { - this.props.onFocus(); - this.setState({ - focused: true, - showExamples: true, - }); - }; - - show: () => void = () => { - this.setState({showExamples: true}); - }; - - hide: () => void = () => { - this.setState({showExamples: false}); - }; - - _handleBlur: () => void = () => { - this.props.onBlur(); - this.setState({ - focused: false, - showExamples: false, - }); - }; - - focus: () => void = () => { - this.inputRef.current?.focus(); - }; - - blur: () => void = () => { - this.inputRef.current?.blur(); - }; - - handleChange: (arg1: any) => void = (e) => { - this.props.onChange(e.target.value); - }; - render(): React.ReactNode { - const input = this._renderInput(); - - const examplesContent = - this.props.examples.length <= 2 - ? this.props.examples.join(" ") // A single item (with or without leading text) is not a "list" - : this.props.examples // 2 or more items should display as a list - .map((example, index) => { - // If the first example is bold, then it is most likely a heading/leading text. - // So, it shouldn't be part of the list. - return index === 0 && example.startsWith("**") - ? `${example}\n` - : `- ${example}`; - }) - .join("\n"); - - const showExamples = - this.props.shouldShowExamples && this.state.showExamples; - - return ( - - {input} -
- -
- - ); - } -} - -export default InputWithExamples; diff --git a/packages/perseus/src/widgets/input-number/input-number.tsx b/packages/perseus/src/widgets/input-number/input-number.tsx index 24f2af568d..2b44fd1feb 100644 --- a/packages/perseus/src/widgets/input-number/input-number.tsx +++ b/packages/perseus/src/widgets/input-number/input-number.tsx @@ -9,10 +9,10 @@ import * as React from "react"; import _ from "underscore"; import {PerseusI18nContext} from "../../components/i18n-context"; -import InputWithExamples from "../../components/input-with-examples"; import SimpleKeypadInput from "../../components/simple-keypad-input"; import {ApiOptions} from "../../perseus-api"; import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/input-number/input-number-ai-utils"; +import InputWithExamples from "../numeric-input/input-with-examples"; import type {PerseusStrings} from "../../strings"; import type {Path, Widget, WidgetExports, WidgetProps} from "../../types"; diff --git a/packages/perseus/src/components/__stories__/input-with-examples.stories.tsx b/packages/perseus/src/widgets/numeric-input/input-with-examples.stories.tsx similarity index 93% rename from packages/perseus/src/components/__stories__/input-with-examples.stories.tsx rename to packages/perseus/src/widgets/numeric-input/input-with-examples.stories.tsx index 95b31db55d..4263d4a7a9 100644 --- a/packages/perseus/src/components/__stories__/input-with-examples.stories.tsx +++ b/packages/perseus/src/widgets/numeric-input/input-with-examples.stories.tsx @@ -1,6 +1,6 @@ import {action} from "@storybook/addon-actions"; -import InputWithExamples from "../input-with-examples"; +import InputWithExamples from "./input-with-examples"; import type {Meta, StoryObj} from "@storybook/react"; diff --git a/packages/perseus/src/widgets/numeric-input/input-with-examples.tsx b/packages/perseus/src/widgets/numeric-input/input-with-examples.tsx new file mode 100644 index 0000000000..4ceb382ca3 --- /dev/null +++ b/packages/perseus/src/widgets/numeric-input/input-with-examples.tsx @@ -0,0 +1,193 @@ +import * as PerseusLinter from "@khanacademy/perseus-linter"; +import * as React from "react"; +import {forwardRef, useImperativeHandle} from "react"; +import _ from "underscore"; + +import {PerseusI18nContext} from "../../components/i18n-context"; +import TextInput from "../../components/text-input"; +import Tooltip, { + HorizontalDirection, + VerticalDirection, +} from "../../components/tooltip"; +import {ClassNames as ApiClassNames} from "../../perseus-api"; +import Renderer from "../../renderer"; +import Util from "../../util"; + +import type {LinterContextProps} from "@khanacademy/perseus-linter"; +import type {StyleType} from "@khanacademy/wonder-blocks-core"; + +const {captureScratchpadTouchStart} = Util; + +type Props = { + value: string; + onChange: any; + className: string; + examples: ReadonlyArray; + shouldShowExamples: boolean; + convertDotToTimes?: boolean; + buttonSet?: string; + buttonsVisible?: "always" | "never" | "focused"; + labelText?: string; + onFocus: () => void; + onBlur: () => void; + disabled: boolean; + style?: StyleType; + id: string; + linterContext: LinterContextProps; +}; + +// [LEMS-2411](Jan 2025) Third: This component has been moved to the NumericInput +// folder as we are actively working towards removing the InputNumber widget. +// This comment can be removed as part of LEMS-2411. + +/** + * The InputWithExamples component is a child component of the NumericInput + * and InputNumber components. It is responsible for rendering the UI elements + * for the desktop versions of these widgets, and displays a tooltip with + * examples of how to input the selected answer forms. + */ +const InputWithExamples = forwardRef< + React.RefObject, + Props +>((props, ref) => { + // Desctructure the props to set default values + const { + shouldShowExamples = true, + onFocus = () => {}, + onBlur = () => {}, + disabled = false, + linterContext = PerseusLinter.linterContextDefault, + className = "", + } = props; + + const context = React.useContext(PerseusI18nContext); + const inputRef = React.useRef(null); + const [inputFocused, setInputFocused] = React.useState(false); + + useImperativeHandle(ref, () => ({ + current: inputRef.current, + focus: () => { + if (inputRef.current) { + inputRef.current.focus(); + } + }, + blur: () => { + if (inputRef.current) { + inputRef.current.blur(); + } + }, + })); + + const _getUniqueId = () => { + return `input-with-examples-${btoa(props.id).replace(/=/g, "")}`; + }; + + const _getInputClassName = () => { + let inputClassName = + ApiClassNames.INPUT + " " + ApiClassNames.INTERACTIVE; + if (inputFocused) { + inputClassName += " " + ApiClassNames.FOCUSED; + } + if (className) { + inputClassName += " " + className; + } + return inputClassName; + }; + + const _renderInput = () => { + const id = _getUniqueId(); + const ariaId = `aria-for-${id}`; + + // Generate the provided examples in simple language for screen readers. + // If all examples are provided, do not provide them to the screen reader. + const examplesAria = + props.examples.length === 0 + ? "" + : `${props.examples[0]} + ${props.examples.slice(1).join(", or\n")}` + // @ts-expect-error TS2550: Property replaceAll does not exist on type string. + .replaceAll("*", "") + .replaceAll("$", "") + .replaceAll("\\ \\text{pi}", " pi") + .replaceAll("\\ ", " and "); + + const inputProps = { + id: id, + "aria-describedby": ariaId, + ref: inputRef, + className: _getInputClassName(), + labelText: props.labelText, + value: props.value, + onFocus: _handleFocus, + onBlur: _handleBlur, + disabled: disabled, + style: props.style, + onChange: props.onChange, + onTouchStart: captureScratchpadTouchStart, + autoCapitalize: "off", + autoComplete: "off", + autoCorrect: "off", + spellCheck: "false", + }; + return ( + <> + + + {examplesAria} + + + ); + }; + + const _handleFocus = () => { + onFocus(); + setInputFocused(true); + }; + + const _handleBlur = () => { + onBlur(); + setInputFocused(false); + }; + + // Display the examples as a string when there are less than or equal to 2 examples. + // Otherwise, display the examples as a list. + const examplesContent = + props.examples.length <= 2 + ? props.examples.join(" ") + : props.examples + .map((example, index) => { + return index === 0 && example.startsWith("**") + ? `${example}\n` + : `- ${example}`; + }) + .join("\n"); + + // Display the examples when they are enabled (shouldShowExamples) and the input is focused. + const showExamplesTooltip = shouldShowExamples && inputFocused; + + return ( + + {_renderInput()} +
+ +
+ + ); +}); + +export default InputWithExamples; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx index 44b8fede57..ea89da7a0e 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx @@ -8,7 +8,7 @@ import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/numeric-inp import {NumericInputComponent} from "./numeric-input"; import {unionAnswerForms} from "./utils"; -import type InputWithExamples from "../../components/input-with-examples"; +import type InputWithExamples from "./input-with-examples"; import type SimpleKeypadInput from "../../components/simple-keypad-input"; import type {FocusPath, Widget, WidgetExports, WidgetProps} from "../../types"; import type { @@ -74,7 +74,7 @@ export class NumericInput extends React.Component implements Widget { - inputRef: RefObject; + inputRef: RefObject; static defaultProps: DefaultProps = { currentValue: "", @@ -100,7 +100,7 @@ export class NumericInput // Create a ref that we can pass down to the input component so that we // can call focus on it when necessary. this.inputRef = React.createRef< - SimpleKeypadInput | InputWithExamples + SimpleKeypadInput | typeof InputWithExamples >(); } diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx index 5fd5c04099..81db1cbe0b 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.tsx @@ -9,13 +9,13 @@ import { } from "react"; import {PerseusI18nContext} from "../../components/i18n-context"; -import InputWithExamples from "../../components/input-with-examples"; import SimpleKeypadInput from "../../components/simple-keypad-input"; +import InputWithExamples from "./input-with-examples"; import {type NumericInputProps} from "./numeric-input.class"; import {generateExamples, shouldShowExamples} from "./utils"; -type InputRefType = SimpleKeypadInput | InputWithExamples | null; +type InputRefType = SimpleKeypadInput | typeof InputWithExamples | null; /** * The NumericInputComponent is a child component of the NumericInput class @@ -30,15 +30,16 @@ export const NumericInputComponent = forwardRef( // Pass the focus and blur methods to the Numeric Input Class component useImperativeHandle(ref, () => ({ + current: inputRef.current, focus: () => { if (inputRef.current) { - inputRef.current.focus(); + inputRef.current?.focus(); setIsFocused(true); } }, blur: () => { if (inputRef.current) { - inputRef.current.blur(); + inputRef.current?.blur(); setIsFocused(false); } }, @@ -105,7 +106,7 @@ export const NumericInputComponent = forwardRef( // (desktop-only) Otherwise, use the InputWithExamples component return ( } + ref={inputRef as React.RefObject} value={props.currentValue} onChange={handleChange} labelText={labelText} From 46036702af82b219a212cc75a77166baa99a1d20 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Tue, 28 Jan 2025 12:21:17 -0800 Subject: [PATCH 04/11] [its-a-numeric-story] Improving Numeric Input Storybook Stories (#2138) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: This PR is part of the Numeric Input Project. The purpose of this PR is to improve our storybook setup for our Numeric Input Widget. This includes the following work: - Modernizing story structure - Hooking up argTypes with descriptions - Updating RendererWithDebugUI to also set customKeypad to the same value as isMobile - This allows us to ensure that our Widgets that use MathInput are properly updating when toggled into Mobile view - Adjustments to SideBySide - Automatically collapse the Perseus JSON view. - Moved the PerseusJSON view below the Renderer View - Rename to SplitView to better encapsulate the new design - Updated variable names to match [Current (Live) Storybook Example](https://khan.github.io/perseus/?path=/docs/perseus-widgets-numericinput--docs) | [PR Storybook Example](https://650db21c3f5d1b2f13c02952-osexoxinde.chromatic.com/?path=/docs/perseus-widgets-numeric-input--docs) Issue: LEMS-2449 ## Video Example: https://github.com/user-attachments/assets/69f6dbfb-1fda-445b-a06f-90a178f9dbeb ## Test plan: - Ensure all tests pass + manual testing Author: SonicScrewdriver Reviewers: SonicScrewdriver, mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/2138 --- .changeset/six-cars-agree.md | 5 + .../src/__stories__/editor.stories.tsx | 10 +- .../numeric-input/numeric-input.stories.tsx | 355 ++++++++++++++---- .../numeric-input/numeric-input.testdata.ts | 257 ++++++++++++- testing/renderer-with-debug-ui.tsx | 16 +- .../server-item-renderer-with-debug-ui.tsx | 6 +- testing/{side-by-side.tsx => split-view.tsx} | 44 +-- 7 files changed, 587 insertions(+), 106 deletions(-) create mode 100644 .changeset/six-cars-agree.md rename testing/{side-by-side.tsx => split-view.tsx} (51%) diff --git a/.changeset/six-cars-agree.md b/.changeset/six-cars-agree.md new file mode 100644 index 0000000000..837b86f16b --- /dev/null +++ b/.changeset/six-cars-agree.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Cleanup of Numeric Input stories diff --git a/packages/perseus-editor/src/__stories__/editor.stories.tsx b/packages/perseus-editor/src/__stories__/editor.stories.tsx index e039eeada8..5b34458b52 100644 --- a/packages/perseus-editor/src/__stories__/editor.stories.tsx +++ b/packages/perseus-editor/src/__stories__/editor.stories.tsx @@ -4,7 +4,7 @@ import {action} from "@storybook/addon-actions"; import * as React from "react"; import {Editor} from ".."; -import SideBySide from "../../../../testing/side-by-side"; +import SplitView from "../../../../testing/split-view"; import {question1} from "../__testdata__/numeric-input.testdata"; import {registerAllWidgetsAndEditorsForTesting} from "../util/register-all-widgets-and-editors-for-testing"; @@ -85,9 +85,9 @@ export const DemoInteractiveGraph = (): React.ReactElement => { // class to be above it. // TODO: Refactor to aphrodite styles instead of scoped CSS in Less.
- { /> } - rightTitle="Serialized Widget Options" + JSONTitle="Serialized Widget Options" jsonObject={options} />
diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx index 0222b37690..ea619a7dfd 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.stories.tsx @@ -1,99 +1,320 @@ -import {action} from "@storybook/addon-actions"; import * as React from "react"; import {RendererWithDebugUI} from "../../../../../testing/renderer-with-debug-ui"; import {NumericInput} from "./numeric-input.class"; -import {question1} from "./numeric-input.testdata"; +import { + decimalProblem, + improperProblem, + integerProblem, + mixedProblem, + piProblem, + properProblem, + question1, + withCoefficient, +} from "./numeric-input.testdata"; -type StoryArgs = { - coefficient: boolean; - currentValue: string; - rightAlign: boolean; - size: "normal" | "small"; -}; +import type { + PerseusNumericInputWidgetOptions, + PerseusRenderer, +} from "@khanacademy/perseus-core"; +import type {Meta} from "@storybook/react"; + +// We're using this format as storybook was not able to infer the type of the options. +// It also gives us a lovely hover view of the JSON structure. +const answerFormsArray: string = `[ + { + simplify: string; + name: string; + } +]`; -function generateProps(overwrite) { - const base = { - alignment: "", - answers: [], - containerSizeClass: "medium", - isLastUsedWidget: true, +const answersArray: string = `[ + { + message: string; + value: number; + status: string; + answerForms: array; + strict: boolean; + maxError: number; + simplify: string; + } +]`; + +const meta: Meta = { + component: NumericInput, + title: "Perseus/Widgets/Numeric Input", + args: { coefficient: false, currentValue: "", - problemNum: 0, rightAlign: false, size: "normal", - static: false, - widgetId: "widgetId", - findWidgets: action("findWidgets"), - onBlur: action("onBlur"), - onChange: action("onChange"), - onFocus: action("onFocus"), - trackInteraction: action("trackInteraction"), - } as const; - - return {...base, ...overwrite}; -} - -export default { - title: "Perseus/Widgets/NumericInput", - args: { - coefficient: false, - currentValue: "8675309", - rightAlign: false, + answers: [ + { + status: "correct", + maxError: null, + strict: false, + value: 1252, + simplify: "required", + message: "", + }, + ], + // We're including all the answer forms to make it easier to edit in storybook. + answerForms: [ + {simplify: "required", name: "decimal"}, + {simplify: "required", name: "integer"}, + {simplify: "required", name: "mixed"}, + {simplify: "required", name: "percent"}, + {simplify: "required", name: "pi"}, + {simplify: "required", name: "proper"}, + {simplify: "required", name: "improper"}, + ], }, argTypes: { + answers: { + control: {type: "object"}, + description: + "A list of all the possible correct and incorrect answers", + table: { + type: { + summary: "array", + detail: answersArray, + }, + }, + }, + answerForms: { + control: {type: "object"}, + description: + "Used by examples, maybe not used and should be removed in the future", + table: { + type: { + summary: "array", + detail: answerFormsArray, + }, + }, + }, + currentValue: { + control: {type: "text"}, + description: "The current value of the input field", + table: { + type: {summary: "string"}, + }, + }, + coefficient: { + control: {type: "boolean"}, + description: + "A coefficient style number allows the student to use - for -1 and an empty string to mean 1.", + table: { + type: {summary: "boolean"}, + }, + }, + labelText: { + control: {type: "text"}, + description: + " Translatable Text; Text to describe this input. This will be shown to users using screenreaders.", + value: "What's the answer?", + table: { + type: {summary: "string"}, + }, + }, + rightAlign: { + control: {type: "boolean"}, + description: "Whether to right-align the text or not", + table: { + type: {summary: "boolean"}, + }, + }, size: { options: ["normal", "small"], control: {type: "radio"}, defaultValue: "normal", + description: + "Use size 'Normal' for all text boxes, unless there are multiple text boxes in one line and the answer area is too narrow to fit them.", + table: { + type: {summary: "string"}, + defaultValue: {summary: "normal"}, + }, + }, + static: { + control: {type: "boolean"}, + description: "Always false. Not used for this widget", + table: { + type: {summary: "boolean"}, + }, + }, + // ApiOptions and linterContext are large objects and not particularly applicable to this story, + // so we're hiding them from view to simplify the UI. + apiOptions: { + table: { + disable: true, + }, + }, + linterContext: { + table: { + disable: true, + }, }, }, }; -export const Question1 = (): React.ReactElement => { - return ; +export default meta; + +const updateWidgetOptions = ( + question: PerseusRenderer, + widgetId: string, + options: PerseusNumericInputWidgetOptions, +): PerseusRenderer => { + const widget = question.widgets[widgetId]; + return { + ...question, + widgets: { + [widgetId]: { + ...widget, + options: { + ...widget.options, + ...options, + }, + }, + }, + }; }; -export const Interactive = (args: StoryArgs): React.ReactElement => { - const props = generateProps(args); +export const Default = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions(question1, "numeric-input 1", args); + return ; +}; +Default.args = question1.widgets["numeric-input 1"].options; +Default.parameters = { + docs: { + description: { + story: "The default Numeric Input widget.", + }, + }, +}; - return ; +export const IntegerExample = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions( + integerProblem, + "numeric-input 1", + args, + ); + return ; +}; +IntegerExample.args = integerProblem.widgets["numeric-input 1"].options; +IntegerExample.parameters = { + docs: { + description: { + story: "Numeric Input set to strictly integer mode will only accept integer answers.", + }, + }, }; -export const Sizes = (args: StoryArgs): React.ReactElement => { - const smallProps = generateProps({...args, size: "small"}); - const normalProps = generateProps({...args, size: "normal"}); +export const DecimalExample = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions( + decimalProblem, + "numeric-input 1", + args, + ); + return ; +}; +DecimalExample.args = decimalProblem.widgets["numeric-input 1"].options; +DecimalExample.parameters = { + docs: { + description: { + story: "Numeric Inputs set to strictly decimal mode will only accept decimal answers.", + }, + }, +}; - return ( -
- - -
+export const ImproperExample = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions( + improperProblem, + "numeric-input 1", + args, ); + return ; +}; +ImproperExample.args = improperProblem.widgets["numeric-input 1"].options; +ImproperExample.parameters = { + docs: { + description: { + story: "Numeric Inputs set to strictly improper mode will only accept improper fractions.", + }, + }, +}; + +export const ProperExample = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions( + properProblem, + "numeric-input 1", + args, + ); + return ; +}; +ProperExample.args = properProblem.widgets["numeric-input 1"].options; +ProperExample.parameters = { + docs: { + description: { + story: "Numeric Inputs set to strictly proper mode will only accept proper fractions. This example does not require simplifying.", + }, + }, }; -export const TextAlignment = (args: StoryArgs): React.ReactElement => { - const leftProps = generateProps({...args, rightAlign: false}); - const rightProps = generateProps({...args, rightAlign: true}); - - return ( -
- - -
+export const MixedExample = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions(mixedProblem, "numeric-input 1", args); + return ; +}; +MixedExample.args = mixedProblem.widgets["numeric-input 1"].options; +MixedExample.parameters = { + docs: { + description: { + story: "Numeric Inputs set to strictly mixed mode will only accept mixed fractions.", + }, + }, +}; + +export const PiExample = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions(piProblem, "numeric-input 1", args); + return ; +}; +PiExample.args = piProblem.widgets["numeric-input 1"].options; +PiExample.parameters = { + docs: { + description: { + story: "Numeric Inputs set to strictly pi mode will only accept answers in terms of π. Approximating pi will result in an incorrect answer and a hint.", + }, + }, +}; + +export const CoefficientExample = ( + args: PerseusNumericInputWidgetOptions, +): React.ReactElement => { + const question = updateWidgetOptions( + withCoefficient, + "numeric-input 1", + args, ); + return ; +}; +CoefficientExample.args = withCoefficient.widgets["numeric-input 1"].options; +CoefficientExample.parameters = { + docs: { + description: { + story: "When Numeric Input is set to coefficient mode, it allows the student to use - for -1 and an empty string to mean 1.", + }, + }, }; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts b/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts index f23d28f667..6e6493382c 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts @@ -36,6 +36,253 @@ export const question1: PerseusRenderer = { }, }; +export const decimalProblem: PerseusRenderer = { + // Added a floating question mark to keep enough space to show the examples. + content: "$12 + 0.52 =$ [[\u2603 numeric-input 1]] \n\n‎", + images: {}, + widgets: { + "numeric-input 1": { + graded: true, + version: { + major: 0, + minor: 0, + }, + static: false, + type: "numeric-input", + options: { + coefficient: false, + static: false, + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 12.52, + simplify: "required", + message: "", + answerForms: ["decimal"], + }, + ], + labelText: "", + size: "normal", + answerForms: [ + { + simplify: "required", + name: "decimal", + }, + ], + }, + alignment: "default", + } as NumericInputWidget, + }, +}; + +export const integerProblem: PerseusRenderer = { + // Added a floating question mark to keep enough space to show the examples. + content: "$5/5 + 10/10 =$ [[\u2603 numeric-input 1]] \n\n‎", + images: {}, + widgets: { + "numeric-input 1": { + graded: true, + version: { + major: 0, + minor: 0, + }, + static: false, + type: "numeric-input", + options: { + coefficient: false, + static: false, + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 15, + simplify: "required", + message: "", + answerForms: ["integer"], + }, + ], + labelText: "", + size: "normal", + answerForms: [ + { + simplify: "required", + name: "integer", + }, + ], + }, + alignment: "default", + } as NumericInputWidget, + }, +}; + +export const improperProblem: PerseusRenderer = { + // Added a floating question mark to keep enough space to show the examples. + content: "$2/2 + 5/2 =$ [[\u2603 numeric-input 1]] \n\n‎\n\n‎", + images: {}, + widgets: { + "numeric-input 1": { + graded: true, + version: { + major: 0, + minor: 0, + }, + static: false, + type: "numeric-input", + options: { + coefficient: false, + static: false, + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 3.5, + simplify: "optional", + message: "", + answerForms: ["improper"], + }, + ], + labelText: "", + size: "normal", + answerForms: [ + { + simplify: "optional", + name: "improper", + }, + ], + }, + alignment: "default", + } as NumericInputWidget, + }, +}; + +export const piProblem: PerseusRenderer = { + // Added a floating question mark to keep enough space to show the examples. + content: + "$pi * 32 =$ [[\u2603 numeric-input 1]] \n\n‎\n\n Hint: Enter 100.53 to get an approx of pi error.", + images: {}, + widgets: { + "numeric-input 1": { + graded: true, + version: { + major: 0, + minor: 0, + }, + static: false, + type: "numeric-input", + options: { + coefficient: false, + static: false, + answers: [ + { + status: "correct", + maxError: null, + strict: false, + value: 100.53096491487338, + simplify: "required", + message: "", + answerForms: ["pi"], + }, + ], + labelText: "", + size: "normal", + answerForms: [ + { + simplify: "required", + name: "pi", + }, + ], + }, + alignment: "default", + } as NumericInputWidget, + }, +}; + +export const mixedProblem: PerseusRenderer = { + // Added a floating question mark to keep enough space to show the examples. + content: "$2 + 2/10 =$ [[\u2603 numeric-input 1]] \n\n‎", + images: {}, + widgets: { + "numeric-input 1": { + graded: true, + version: { + major: 0, + minor: 0, + }, + static: false, + type: "numeric-input", + options: { + coefficient: false, + static: false, + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 2.2, + simplify: "optional", + message: "", + answerForms: ["mixed"], + }, + ], + labelText: "", + size: "normal", + answerForms: [ + { + simplify: "optional", + name: "mixed", + }, + ], + }, + alignment: "default", + } as NumericInputWidget, + }, +}; + +export const properProblem: PerseusRenderer = { + // Added a floating question mark to keep enough space to show the examples. + content: "$3/10 + 2/10 =$ [[\u2603 numeric-input 1]] \n\n‎\n\n‎", + images: {}, + widgets: { + "numeric-input 1": { + graded: true, + version: { + major: 0, + minor: 0, + }, + static: false, + type: "numeric-input", + options: { + coefficient: false, + static: false, + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 0.5, + simplify: "optional", + message: "", + answerForms: ["proper"], + }, + ], + labelText: "", + size: "normal", + answerForms: [ + { + simplify: "optional", + name: "proper", + }, + ], + }, + alignment: "default", + } as NumericInputWidget, + }, +}; + export const percentageProblem: PerseusRenderer = { content: "$5008 \\div 4 =$ [[\u2603 numeric-input 1]] ", images: {}, @@ -134,6 +381,7 @@ export const multipleAnswersWithDecimals: PerseusRenderer = { value: 12.2, simplify: "required", message: "", + answerForms: ["decimal"], }, { status: "correct", @@ -142,10 +390,17 @@ export const multipleAnswersWithDecimals: PerseusRenderer = { value: 13.4, simplify: "required", message: "", + answerForms: ["decimal"], }, ], labelText: "What's the answer?", size: "normal", + answerforms: [ + { + simplify: "required", + name: "decimal", + }, + ], }, alignment: "default", } as NumericInputWidget, @@ -196,7 +451,7 @@ export const duplicatedAnswers: PerseusRenderer = { }; export const withCoefficient: PerseusRenderer = { - content: "$5008 \\div 4 =$ [[\u2603 numeric-input 1]] ", + content: "$1 =$ [[\u2603 numeric-input 1]] ", images: {}, widgets: { "numeric-input 1": { diff --git a/testing/renderer-with-debug-ui.tsx b/testing/renderer-with-debug-ui.tsx index 7db7fb17ef..daf4f89009 100644 --- a/testing/renderer-with-debug-ui.tsx +++ b/testing/renderer-with-debug-ui.tsx @@ -14,7 +14,7 @@ import {scorePerseusItem} from "../packages/perseus/src/renderer-util"; import {mockStrings} from "../packages/perseus/src/strings"; import {registerAllWidgetsForTesting} from "../packages/perseus/src/util/register-all-widgets-for-testing"; -import SideBySide from "./side-by-side"; +import SplitView from "./split-view"; import type {PerseusRenderer} from "@khanacademy/perseus-core"; import type {ComponentProps} from "react"; @@ -40,9 +40,15 @@ export const RendererWithDebugUI = ({ const [isMobile, setIsMobile] = React.useState(false); const {strings} = usePerseusI18n(); + const controlledAPIOptions = { + ...apiOptions, + isMobile, + customKeypad: isMobile, // Use the mobile keypad for mobile + }; + return ( - } - left={ + renderer={ { return ( - - {leftTitle} - {left} + + {rendererTitle} + {renderer} - - {rightTitle} + + {JSONTitle} @@ -43,18 +45,10 @@ const styles = { sideBySide: { display: "flex", flexWrap: "wrap", - flexDirection: "row", + flexDirection: "column", gap: spacing.large_24, padding: `0px ${spacing.large_24}px`, }, - leftPanel: { - flexBasis: `${interactiveSizes.defaultBoxSize}px`, - }, - rightPanel: { - flexGrow: 1, - flexBasis: `${interactiveSizes.defaultBoxSize}px`, - maxWidth: "50%", - }, } as const; -export default SideBySide; +export default SplitView; From 3eae8b6e030b0118f98e368af51c6a8d918aa648 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Tue, 28 Jan 2025 12:40:19 -0800 Subject: [PATCH 05/11] Ensure that Numeric Input Examples only show for correct answers. (#2159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: This PR is part of the Numeric Input Project. This is a bug fix that ensures that the Examples Tooltip for the Numeric Input widget only displays examples from the _correct_ answers. Issue: LEMS-2803 ## Test plan: - Run tests - Creation of new snapshot test that verifies wrong answerFormats are not being provided. Author: SonicScrewdriver Reviewers: SonicScrewdriver, handeyeco, mark-fitzgerald Required Reviewers: Approved By: handeyeco, mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x) Pull Request URL: https://github.com/Khan/perseus/pull/2159 --- .changeset/rotten-kangaroos-fly.md | 5 + .../__snapshots__/numeric-input.test.ts.snap | 120 ++++++++++++++++++ .../numeric-input/numeric-input.class.tsx | 20 +-- .../numeric-input/numeric-input.test.ts | 87 +++++++++++++ .../numeric-input/numeric-input.testdata.ts | 43 +++++++ 5 files changed, 267 insertions(+), 8 deletions(-) create mode 100644 .changeset/rotten-kangaroos-fly.md diff --git a/.changeset/rotten-kangaroos-fly.md b/.changeset/rotten-kangaroos-fly.md new file mode 100644 index 0000000000..c14b650f34 --- /dev/null +++ b/.changeset/rotten-kangaroos-fly.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Bug fix to ensure that Numeric Examples only show for correct answers. diff --git a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap index 87aa1d4461..f5e5533b5a 100644 --- a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap +++ b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap @@ -424,6 +424,126 @@ a mixed number, like 1 and 3/4 `; +exports[`numeric-input widget Should render tooltip using only correct answer formats: render tooltip only with correct answers 1`] = ` +
+
+
+
+ + + + 5008 \\div 4 = + + + + +
+ +
+ + +
+
+ + +
+
+
+
+`; + exports[`numeric-input widget Should render tooltip when format option is given: render with format tooltip 1`] = `
{ - return (answer.answerForms || []).map((form) => { - return { - simplify: answer.simplify, - name: form, - }; - }); - }), + // Filter out the correct answers and map them to the answer forms + // so that we can generate the examples for the widget. + widgetOptions.answers + .filter((answer) => answer.status === "correct") + .map((answer) => { + return (answer.answerForms || []).map((form) => { + return { + simplify: answer.simplify, + name: form, + }; + }); + }), ), }; diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts index ec3431ccbb..711435f5b6 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.test.ts @@ -15,8 +15,10 @@ import { question1, duplicatedAnswers, withCoefficient, + correctAndWrongAnswers, } from "./numeric-input.testdata"; +import type {PerseusNumericInputWidgetOptions} from "@khanacademy/perseus-core"; import type {PerseusNumericInputRubric} from "@khanacademy/perseus-score"; import type {UserEvent} from "@testing-library/user-event"; @@ -113,6 +115,16 @@ describe("numeric-input widget", () => { expect(container).toMatchSnapshot("render with format list tooltip"); }); + it("Should render tooltip using only correct answer formats", async () => { + // Arrange + const {container} = renderQuestion(correctAndWrongAnswers); + + // Assert + expect(container).toMatchSnapshot( + "render tooltip only with correct answers", + ); + }); + it("Should render an element with format options as text for use by assistive technologies", async () => { // Arrange - Fractions const questionWithFormatOptions = JSON.parse(JSON.stringify(question1)); @@ -374,3 +386,78 @@ describe("Numeric input widget", () => { ); }); }); + +describe("transform", () => { + it("removes the answers and extracts the answer forms", () => { + const transform = NumericInputWidgetExport.transform; + const widgetOptions: PerseusNumericInputWidgetOptions = { + coefficient: false, + static: false, + size: "normal", + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 0.5, + simplify: "required", + answerForms: ["proper"], + message: "", + }, + ], + }; + const renderProps = transform(widgetOptions); + expect(renderProps).toEqual({ + coefficient: false, + static: false, + size: "normal", + answerForms: [ + { + simplify: "required", + name: "proper", + }, + ], + }); + }); + + it("only uses answer forms from correct answers", () => { + const transform = NumericInputWidgetExport.transform; + const widgetOptions: PerseusNumericInputWidgetOptions = { + coefficient: false, + static: false, + size: "normal", + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 0.5, + simplify: "required", + answerForms: ["proper"], + message: "", + }, + { + status: "wrong", + maxError: null, + strict: true, + value: 0.5, + simplify: "required", + answerForms: ["decimal"], + message: "", + }, + ], + }; + const renderProps = transform(widgetOptions); + expect(renderProps).toEqual({ + coefficient: false, + static: false, + size: "normal", + answerForms: [ + { + simplify: "required", + name: "proper", + }, + ], + }); + }); +}); diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts b/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts index 6e6493382c..9e0742133b 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.testdata.ts @@ -358,6 +358,49 @@ export const multipleAnswers: PerseusRenderer = { }, }; +export const correctAndWrongAnswers: PerseusRenderer = { + content: "$5008 \\div 4 =$ [[\u2603 numeric-input 1]] ", + images: {}, + widgets: { + "numeric-input 1": { + graded: true, + version: { + major: 0, + minor: 0, + }, + static: false, + type: "numeric-input", + options: { + coefficient: false, + static: false, + answers: [ + { + status: "correct", + maxError: null, + strict: true, + value: 0.5, + simplify: "required", + answerForms: ["proper"], + message: "", + }, + { + status: "wrong", + maxError: null, + strict: true, + value: 0.5, + simplify: "required", + answerForms: ["decimal"], + message: "", + }, + ], + labelText: "What's the answer?", + size: "normal", + }, + alignment: "default", + } as NumericInputWidget, + }, +}; + export const multipleAnswersWithDecimals: PerseusRenderer = { content: "$5008 \\div 4 =$ [[\u2603 numeric-input 1]] ", images: {}, From 182cc5eea29bbba039f312a444b8b40efb252cf0 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 30 Jan 2025 10:32:42 -0800 Subject: [PATCH 06/11] [feature/numeric-dx-refactor] Fixing rebase with main --- .../numeric-input/__snapshots__/numeric-input.test.ts.snap | 1 + .../perseus/src/widgets/numeric-input/numeric-input.class.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap index f5e5533b5a..51d26397aa 100644 --- a/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap +++ b/packages/perseus/src/widgets/numeric-input/__snapshots__/numeric-input.test.ts.snap @@ -458,6 +458,7 @@ exports[`numeric-input widget Should render tooltip using only correct answer fo aria-disabled="false" aria-invalid="false" aria-label="What's the answer?" + aria-required="false" autocapitalize="off" autocomplete="off" autocorrect="off" diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx index eb31f287cf..167c4c6c65 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx @@ -7,7 +7,7 @@ import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/numeric-inp import {NumericInputComponent} from "./numeric-input"; import {unionAnswerForms} from "./utils"; - +import {scoreNumericInput} from "@khanacademy/perseus-score"; import type InputWithExamples from "./input-with-examples"; import type SimpleKeypadInput from "../../components/simple-keypad-input"; import type {FocusPath, Widget, WidgetExports, WidgetProps} from "../../types"; From d312277beafc74d415ab4695bdbeb44e3dcb7396 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 30 Jan 2025 10:40:38 -0800 Subject: [PATCH 07/11] [2-quell-the-fracus] [Bug Fix] - Ensure that users hand typing tex into Numeric Inputs on Desktop do not cause an infinite loop. I believe this solves the issue, but I'm putting this in a PR to perform some additional testing downstream. Issue: LEMS-198 Test Plan: - Run tests - New tex wrangler test --- .../src/util/tex-wrangler.test.ts | 19 +++++++++++++++++++ .../perseus-score/src/util/tex-wrangler.ts | 12 ++++++++++++ 2 files changed, 31 insertions(+) diff --git a/packages/perseus-score/src/util/tex-wrangler.test.ts b/packages/perseus-score/src/util/tex-wrangler.test.ts index ae426a1a36..4707e071d3 100644 --- a/packages/perseus-score/src/util/tex-wrangler.test.ts +++ b/packages/perseus-score/src/util/tex-wrangler.test.ts @@ -78,3 +78,22 @@ describe("parseTex", () => { assertParsed(undefined, ""); }); }); +describe("parseTex", () => { + it("should replace single fractions", () => { + assertParsed("\\dfrac{3}{4}", "3 / 4"); + }); + + it("should remove blackslash-escapes for percent signs", () => { + assertParsed("3\\%", "3%"); + assertParsed("3.5\\%", "3.5%"); + assertParsed("\\dfrac{3\\%}{4}", "3% / 4"); + }); + + it("should not throw error when input is undefined", () => { + assertParsed(undefined, ""); + }); + + it("should not cause an infinite loop if provided incomplete tex commands", () => { + assertParsed("\\frac", "/"); + }); +}); diff --git a/packages/perseus-score/src/util/tex-wrangler.ts b/packages/perseus-score/src/util/tex-wrangler.ts index f2329157f5..f58548987c 100644 --- a/packages/perseus-score/src/util/tex-wrangler.ts +++ b/packages/perseus-score/src/util/tex-wrangler.ts @@ -42,11 +42,23 @@ function parseNextExpression( // Find the first '{' and grab subsequent TeX // Ex) tex: '{3}{7}', and we want the '3' const openBracketIndex = tex.indexOf("{", currentIndex); + + // If there is no open bracket, set the endpoint to the end of the string + // and the expression to an empty string. This helps ensure we don't + // get stuck in an infinite loop when users handtype TeX. + if (openBracketIndex === -1) { + return { + endpoint: tex.length, + expression: "", + }; + } + const nextExpIndex = openBracketIndex + 1; // Truncate to only contain remaining TeX const endpoint = findEndpoint(tex, nextExpIndex); const expressionTeX = tex.substring(nextExpIndex, endpoint); + const parsedExp = walkTex(expressionTeX, handler); return { From 9fd21afbacb82f3c66992f17a67d98b49f72c661 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 30 Jan 2025 10:42:34 -0800 Subject: [PATCH 08/11] [2-quell-the-fracus] docs(changeset): Bugfix to ensure users cannot create infinite loop with incomplete tex in Numeric Input --- .changeset/green-wasps-try.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/green-wasps-try.md diff --git a/.changeset/green-wasps-try.md b/.changeset/green-wasps-try.md new file mode 100644 index 0000000000..a5a3329345 --- /dev/null +++ b/.changeset/green-wasps-try.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus-score": patch +--- + +Bugfix to ensure users cannot create infinite loop with incomplete tex in Numeric Input From ab0f191d8b5f7522a7ab81913acbe3ea6a545c86 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 30 Jan 2025 10:58:17 -0800 Subject: [PATCH 09/11] [2-quell-the-fracus] Have to fix import orders after feature branch rebase --- .../src/widgets/numeric-input/numeric-input.class.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx index 167c4c6c65..8803c1ae64 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx @@ -1,5 +1,6 @@ import {KhanMath} from "@khanacademy/kmath"; import {linterContextDefault} from "@khanacademy/perseus-linter"; +import {scoreNumericInput} from "@khanacademy/perseus-score"; import * as React from "react"; import {ApiOptions} from "../../perseus-api"; @@ -7,20 +8,20 @@ import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/numeric-inp import {NumericInputComponent} from "./numeric-input"; import {unionAnswerForms} from "./utils"; -import {scoreNumericInput} from "@khanacademy/perseus-score"; + import type InputWithExamples from "./input-with-examples"; import type SimpleKeypadInput from "../../components/simple-keypad-input"; import type {FocusPath, Widget, WidgetExports, WidgetProps} from "../../types"; -import type { - PerseusNumericInputRubric, - PerseusNumericInputUserInput, -} from "@khanacademy/perseus-score"; import type {NumericInputPromptJSON} from "../../widget-ai-utils/numeric-input/prompt-utils"; import type { PerseusNumericInputWidgetOptions, PerseusNumericInputAnswerForm, MathFormat, } from "@khanacademy/perseus-core"; +import type { + PerseusNumericInputRubric, + PerseusNumericInputUserInput, +} from "@khanacademy/perseus-score"; import type {PropsFor} from "@khanacademy/wonder-blocks-core"; import type {RefObject} from "react"; From 03c9590578b5831814d7f9098ffcd36281394685 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 30 Jan 2025 11:15:44 -0800 Subject: [PATCH 10/11] [2-quell-the-fracus] docs(changeset): Bugfix to ensure users cannot create infinite loop with incomplete tex in Numeric Input --- .changeset/fast-kids-pump.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/fast-kids-pump.md diff --git a/.changeset/fast-kids-pump.md b/.changeset/fast-kids-pump.md new file mode 100644 index 0000000000..c81d2c0cbe --- /dev/null +++ b/.changeset/fast-kids-pump.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": patch +"@khanacademy/perseus-score": patch +--- + +Bugfix to ensure users cannot create infinite loop with incomplete tex in Numeric Input From ff226d9832a8886acc2ab827eff86ae55f42cc25 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 30 Jan 2025 11:16:35 -0800 Subject: [PATCH 11/11] [2-quell-the-fracus] New changeset after fixing import order. --- .changeset/green-wasps-try.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/green-wasps-try.md diff --git a/.changeset/green-wasps-try.md b/.changeset/green-wasps-try.md deleted file mode 100644 index a5a3329345..0000000000 --- a/.changeset/green-wasps-try.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@khanacademy/perseus-score": patch ---- - -Bugfix to ensure users cannot create infinite loop with incomplete tex in Numeric Input