From 4ca8c00f6b2c97325fe7513c539c896ec80cc2ae Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Thu, 23 Jan 2025 12:21:37 -0800 Subject: [PATCH] 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) + ); + }); };