From 3b65e652b731ed33075184263c483b9ef243398a Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Fri, 17 Jan 2025 15:08:17 -0800 Subject: [PATCH 1/7] [numeric-no-underscore] Refactoring Numeric Input helper functions to remove underscore, improve documentation, and add tests. Issue: LEMS-2446 --- packages/perseus-core/src/data-schema.ts | 18 ++- .../numeric-input/score-numeric-input.test.ts | 34 ++-- .../numeric-input/numeric-input.class.tsx | 66 ++------ .../numeric-input/numeric-input.test.ts | 38 +---- .../widgets/numeric-input/numeric-input.tsx | 1 - .../src/widgets/numeric-input/utils.test.ts | 150 ++++++++++++++++-- .../src/widgets/numeric-input/utils.ts | 105 +++++++++--- 7 files changed, 265 insertions(+), 147 deletions(-) diff --git a/packages/perseus-core/src/data-schema.ts b/packages/perseus-core/src/data-schema.ts index 32d5d3a03b..a60e17756e 100644 --- a/packages/perseus-core/src/data-schema.ts +++ b/packages/perseus-core/src/data-schema.ts @@ -1164,16 +1164,18 @@ export type MathFormat = | "pi"; export type PerseusNumericInputAnswerForm = { - simplify: - | "required" - | "correct" - | "enforced" - | "optional" - | null - | undefined; + simplify: PerseusNumericInputSimplify; name: MathFormat; }; +export type PerseusNumericInputSimplify = + | "required" + | "correct" + | "enforced" + | "optional" + | null + | undefined; + export type PerseusNumericInputWidgetOptions = { // A list of all the possible correct and incorrect answers answers: ReadonlyArray; @@ -1210,7 +1212,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; }; export type PerseusNumberLineWidgetOptions = { 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 c070aaf5c1..c9783287f1 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 @@ -10,7 +10,7 @@ describe("static function validate", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -34,7 +34,7 @@ describe("static function validate", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -123,7 +123,7 @@ describe("static function validate", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: true, message: "", }, @@ -147,7 +147,7 @@ describe("static function validate", () => { value: 1, status: "correct", maxError: 0.2, - simplify: "", + simplify: "optional", strict: true, message: "", }, @@ -171,7 +171,7 @@ describe("static function validate", () => { value: 1, status: "correct", maxError: 0.2, - simplify: "", + simplify: "optional", strict: true, message: "", }, @@ -197,7 +197,7 @@ describe("static function validate", () => { value: 4, status: "wrong", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -206,7 +206,7 @@ describe("static function validate", () => { value: 10, status: "correct", maxError: 10, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -241,7 +241,7 @@ describe("static function validate", () => { value: 1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -249,7 +249,7 @@ describe("static function validate", () => { value: -1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -287,7 +287,7 @@ describe("static function validate", () => { // This answer is missing its value field. status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -296,7 +296,7 @@ describe("static function validate", () => { value: 0.5, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -320,7 +320,7 @@ describe("static function validate", () => { value: null, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -329,7 +329,7 @@ describe("static function validate", () => { value: 0.5, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -349,7 +349,7 @@ describe("static function validate", () => { value: 0.2, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -372,7 +372,7 @@ describe("static function validate", () => { value: 1.2, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -395,7 +395,7 @@ describe("static function validate", () => { value: 1.1, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, @@ -415,7 +415,7 @@ describe("static function validate", () => { value: 0.9, status: "correct", maxError: 0, - simplify: "", + simplify: "optional", strict: false, message: "", }, 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 c73f346854..9af2924638 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx @@ -6,13 +6,12 @@ import { type PerseusNumericInputUserInput, } from "@khanacademy/perseus-score"; 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 {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 +156,24 @@ 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"), { + const rendererProps: RenderProps & {answers?: any} = { + ...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,8 +181,9 @@ const propsTransform = function ( }); }), ), - }); + }; + delete rendererProps.answers; 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 e3ecff2b19..e37754fb21 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..b13366a37d 100644 --- a/packages/perseus/src/widgets/numeric-input/utils.test.ts +++ b/packages/perseus/src/widgets/numeric-input/utils.test.ts @@ -1,10 +1,11 @@ -import {generateExamples, shouldShowExamples} from "./utils"; +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", @@ -15,7 +16,7 @@ describe("generateExamples", () => { simplify: "required", }, ]; - const strings: Partial = { + const fakePerseusStrings: Partial = { yourAnswer: "Your answer", integerExample: "Integer example", properExample: "Proper example", @@ -26,14 +27,80 @@ describe("generateExamples", () => { "Integer example", "Simplified proper example", ]; - expect( - generateExamples(answerForms, strings as PerseusStrings), - ).toEqual(expected); + + // Act + const result = generateExamples( + answerForms, + fakePerseusStrings as PerseusStrings, + ); + + // Assert + expect(result).toEqual(expected); + }); + + it("returns only unique entries in an array of examples", () => { + // Arrange + const answerForms: readonly PerseusNumericInputAnswerForm[] = [ + { + name: "integer", + simplify: "optional", + }, + { + name: "proper", + simplify: "required", + }, + { + name: "integer", + simplify: "optional", + }, + ]; + const fakePerseusStrings: Partial = { + yourAnswer: "Your answer", + integerExample: "Integer example", + properExample: "Proper example", + simplifiedProperExample: "Simplified proper example", + }; + const expected = [ + "Your answer", + "Integer example", + "Simplified proper example", + ]; + + // Act + const result = generateExamples( + answerForms, + fakePerseusStrings as PerseusStrings, + ); + + // Assert + expect(result).toEqual(expected); + }); + + it("returns an empty array if no answer forms are provided", () => { + // Arrange + const answerForms: readonly PerseusNumericInputAnswerForm[] = []; + const fakePerseusStrings: Partial = { + yourAnswer: "Your answer", + integerExample: "Integer example", + properExample: "Proper example", + simplifiedProperExample: "Simplified proper example", + }; + const expected = []; + + // Act + const result = generateExamples( + answerForms, + fakePerseusStrings as PerseusStrings, + ); + + // 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 +111,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 +147,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..7e1c643846 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,104 @@ 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 = uniqueBy(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 = uniqueBy(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; +}; + +/** + * uniqueBy takes a list of elements and a function which compares whether + * two elements are equal, and returns a list of unique elements. + */ +const uniqueBy = function ( + list: readonly PerseusNumericInputAnswerForm[], +): PerseusNumericInputAnswerForm[] { + return list.reduce( + (uniqueList, element) => { + // Check if the element is already in the list. + const inList = uniqueList.some((uniqueElement) => + compareAnswerForms(element, uniqueElement), + ); + // If it's in the list, return the list as is + if (inList) { + return uniqueList; + } + // If it's not in the list, add it. + return uniqueList.concat([element]); + }, + [], + ); +}; + +/** + * This is a helper function to compare two answer forms + * to see if they are identical. Given that the answer forms + * are simple objects, we can just compare the properties. + */ +const compareAnswerForms = function ( + a: PerseusNumericInputAnswerForm, + b: PerseusNumericInputAnswerForm, +): boolean { + return a.simplify === b.simplify && a.name === b.name; +}; + +/** + * 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 uniqueBy. + const uniqueForms = uniqueBy(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 e13293d432326b64488fdb294155292f17f3603f Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Fri, 17 Jan 2025 15:15:39 -0800 Subject: [PATCH 2/7] [numeric-no-underscore] Updating parser to handle stricter types --- .../perseus-parsers/numeric-input-widget.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/numeric-input-widget.ts b/packages/perseus/src/util/parse-perseus-json/perseus-parsers/numeric-input-widget.ts index 2cbadd8765..3d641573e7 100644 --- a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/numeric-input-widget.ts +++ b/packages/perseus/src/util/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, ), ), From eedb308a687f352e1b9fcab1874bfc5830651a9e Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Fri, 17 Jan 2025 15:16:14 -0800 Subject: [PATCH 3/7] [numeric-no-underscore] docs(changeset): Refactoring Numeric Input helper functions to remove underscore, improve documentation, and add tests. --- .changeset/sharp-peaches-love.md | 7 +++++++ 1 file changed, 7 insertions(+) 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. From a5907527ed0461b86cd38ce05a010a6525a35ee9 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Fri, 17 Jan 2025 15:23:46 -0800 Subject: [PATCH 4/7] [numeric-no-underscore] Updating function name and comment to indicate that it's no longer generic as it was not being used elsewhere. --- .../src/widgets/numeric-input/utils.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/perseus/src/widgets/numeric-input/utils.ts b/packages/perseus/src/widgets/numeric-input/utils.ts index 7e1c643846..e4985a8d86 100644 --- a/packages/perseus/src/widgets/numeric-input/utils.ts +++ b/packages/perseus/src/widgets/numeric-input/utils.ts @@ -40,7 +40,7 @@ export const generateExamples = ( } // Generate a list of the unique answer forms. - const uniqueForms = uniqueBy(answerForms); + const uniqueForms = getUniqueAnswerForms(answerForms); // Generate the example strings for each unique form. const examples = uniqueForms.map((form) => { @@ -65,9 +65,9 @@ export const shouldShowExamples = ( } // Generate a list of the unique names of the selected answer forms. - const answerFormNames: ReadonlyArray = uniqueBy(answerForms).map( - (form) => form.name, - ); + const answerFormNames: ReadonlyArray = getUniqueAnswerForms( + answerForms, + ).map((form) => form.name); // If all forms are accepted, we do not need to show any examples. const allFormsAccepted = @@ -77,10 +77,11 @@ export const shouldShowExamples = ( }; /** - * uniqueBy takes a list of elements and a function which compares whether - * two elements are equal, and returns a list of unique elements. + * 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 uniqueBy = function ( +const getUniqueAnswerForms = function ( list: readonly PerseusNumericInputAnswerForm[], ): PerseusNumericInputAnswerForm[] { return list.reduce( @@ -124,8 +125,8 @@ export const unionAnswerForms: ( ) => ReadonlyArray = function (answerFormsList) { // 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); + // 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) => { From 59cb86e82ae5a829beb3a1083f088ab124375c45 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Mon, 20 Jan 2025 13:09:51 -0800 Subject: [PATCH 5/7] [numeric-no-underscore] Updating snapshots after modernizing tests and no longer generating example strings if we aren't going to show them. --- .../src/components/input-with-examples.tsx | 2 +- .../parse-perseus-json-snapshot.test.ts.snap | 4 +- .../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 +----------- 6 files changed, 27 insertions(+), 995 deletions(-) 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/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap b/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap index 48cd27db6f..de4da55d33 100644 --- a/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap +++ b/packages/perseus/src/util/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/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 b066a3ddf5..845f630285 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 @@ -108,132 +108,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} - - - -
    • -
+ +
@@ -350,132 +229,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} - - - -
    • -
+ +
@@ -592,132 +350,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 a70f7f1de8..ab2c64597e 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 @@ -174,132 +174,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 0d0898367b..e55a437112 100644 --- a/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap +++ b/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap @@ -801,132 +801,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} - - - -
    • -
+ +
@@ -1021,132 +900,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 e662ff87d9..462442b50a 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 @@ -141,132 +141,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} - - - -
    • -
+ +
@@ -358,132 +237,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} - - - -
    • -
+ +
From c2165ff950281d636ebd0d887caa6ccd0fe30a4f Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Mon, 20 Jan 2025 13:46:20 -0800 Subject: [PATCH 6/7] [numeric-no-underscore] Simplifying logic for getUniqueAnswerForms, as we don't need the full scope of what isEqual was providing previously. --- .../src/widgets/numeric-input/utils.test.ts | 2 +- .../src/widgets/numeric-input/utils.ts | 38 ++++++------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/packages/perseus/src/widgets/numeric-input/utils.test.ts b/packages/perseus/src/widgets/numeric-input/utils.test.ts index b13366a37d..e501e362a2 100644 --- a/packages/perseus/src/widgets/numeric-input/utils.test.ts +++ b/packages/perseus/src/widgets/numeric-input/utils.test.ts @@ -51,7 +51,7 @@ describe("generateExamples", () => { }, { name: "integer", - simplify: "optional", + simplify: "required", }, ]; const fakePerseusStrings: Partial = { diff --git a/packages/perseus/src/widgets/numeric-input/utils.ts b/packages/perseus/src/widgets/numeric-input/utils.ts index e4985a8d86..7452282e82 100644 --- a/packages/perseus/src/widgets/numeric-input/utils.ts +++ b/packages/perseus/src/widgets/numeric-input/utils.ts @@ -84,33 +84,17 @@ export const shouldShowExamples = ( const getUniqueAnswerForms = function ( list: readonly PerseusNumericInputAnswerForm[], ): PerseusNumericInputAnswerForm[] { - return list.reduce( - (uniqueList, element) => { - // Check if the element is already in the list. - const inList = uniqueList.some((uniqueElement) => - compareAnswerForms(element, uniqueElement), - ); - // If it's in the list, return the list as is - if (inList) { - return uniqueList; - } - // If it's not in the list, add it. - return uniqueList.concat([element]); - }, - [], - ); -}; - -/** - * This is a helper function to compare two answer forms - * to see if they are identical. Given that the answer forms - * are simple objects, we can just compare the properties. - */ -const compareAnswerForms = function ( - a: PerseusNumericInputAnswerForm, - b: PerseusNumericInputAnswerForm, -): boolean { - return a.simplify === b.simplify && a.name === b.name; + // 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; + }); }; /** From 148214628cacfcd69555b1c26406883f0ebd6676 Mon Sep 17 00:00:00 2001 From: Sarah Third Date: Tue, 21 Jan 2025 14:37:36 -0800 Subject: [PATCH 7/7] [numeric-no-underscore] PR Feedback --- packages/perseus-core/src/data-schema.ts | 8 ++- .../numeric-input/numeric-input.class.tsx | 5 +- .../src/widgets/numeric-input/utils.test.ts | 51 +++++-------------- 3 files changed, 20 insertions(+), 44 deletions(-) diff --git a/packages/perseus-core/src/data-schema.ts b/packages/perseus-core/src/data-schema.ts index a60e17756e..a8c7aa80e6 100644 --- a/packages/perseus-core/src/data-schema.ts +++ b/packages/perseus-core/src/data-schema.ts @@ -1164,7 +1164,7 @@ export type MathFormat = | "pi"; export type PerseusNumericInputAnswerForm = { - simplify: PerseusNumericInputSimplify; + simplify: PerseusNumericInputSimplify | null | undefined; name: MathFormat; }; @@ -1172,9 +1172,7 @@ export type PerseusNumericInputSimplify = | "required" | "correct" | "enforced" - | "optional" - | null - | undefined; + | "optional"; export type PerseusNumericInputWidgetOptions = { // A list of all the possible correct and incorrect answers @@ -1212,7 +1210,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: PerseusNumericInputSimplify; + simplify: PerseusNumericInputSimplify | null | undefined; }; export type PerseusNumberLineWidgetOptions = { 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 9af2924638..d5df5c6f37 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx +++ b/packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx @@ -169,7 +169,9 @@ type RenderProps = { const propsTransform = function ( widgetOptions: PerseusNumericInputWidgetOptions, ): RenderProps { - const rendererProps: RenderProps & {answers?: any} = { + // Omit the answers from the widget options since they are + // not needed for rendering the widget. + const {answers: _, ...rendererProps} = { ...widgetOptions, answerForms: unionAnswerForms( widgetOptions.answers.map((answer) => { @@ -183,7 +185,6 @@ const propsTransform = function ( ), }; - delete rendererProps.answers; return rendererProps; }; diff --git a/packages/perseus/src/widgets/numeric-input/utils.test.ts b/packages/perseus/src/widgets/numeric-input/utils.test.ts index e501e362a2..e4139bf17f 100644 --- a/packages/perseus/src/widgets/numeric-input/utils.test.ts +++ b/packages/perseus/src/widgets/numeric-input/utils.test.ts @@ -1,6 +1,7 @@ +import {mockStrings} from "../../strings"; + import {generateExamples, shouldShowExamples, unionAnswerForms} from "./utils"; -import type {PerseusStrings} from "../../strings"; import type {PerseusNumericInputAnswerForm} from "@khanacademy/perseus-core"; describe("generateExamples", () => { @@ -16,23 +17,15 @@ describe("generateExamples", () => { simplify: "required", }, ]; - const fakePerseusStrings: 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$", ]; // Act - const result = generateExamples( - answerForms, - fakePerseusStrings as PerseusStrings, - ); + const result = generateExamples(answerForms, mockStrings); // Assert expect(result).toEqual(expected); @@ -54,23 +47,15 @@ describe("generateExamples", () => { simplify: "required", }, ]; - const fakePerseusStrings: 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$", ]; // Act - const result = generateExamples( - answerForms, - fakePerseusStrings as PerseusStrings, - ); + const result = generateExamples(answerForms, mockStrings); // Assert expect(result).toEqual(expected); @@ -79,19 +64,11 @@ describe("generateExamples", () => { it("returns an empty array if no answer forms are provided", () => { // Arrange const answerForms: readonly PerseusNumericInputAnswerForm[] = []; - const fakePerseusStrings: Partial = { - yourAnswer: "Your answer", - integerExample: "Integer example", - properExample: "Proper example", - simplifiedProperExample: "Simplified proper example", - }; + const expected = []; // Act - const result = generateExamples( - answerForms, - fakePerseusStrings as PerseusStrings, - ); + const result = generateExamples(answerForms, mockStrings); // Assert expect(result).toEqual(expected);