Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2-quell-the-fracus] [Bug Fix] - Ensure that users hand typing tex into Numeric Inputs on Desktop do not cause an infinite loop. #2175

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/fast-kids-pump.md
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions .changeset/rich-flowers-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/math-input": minor
"@khanacademy/perseus": minor
---

Modernization and Migration of InputWithExamples to NumericInput folder
5 changes: 5 additions & 0 deletions .changeset/rotten-kangaroos-fly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Bug fix to ensure that Numeric Examples only show for correct answers.
7 changes: 7 additions & 0 deletions .changeset/sharp-peaches-love.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/six-cars-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Cleanup of Numeric Input stories
6 changes: 6 additions & 0 deletions .changeset/smart-countries-hunt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/math-input": minor
"@khanacademy/perseus": minor
---

Refactoring Numeric Input to move UI-logic to functional component.
3 changes: 3 additions & 0 deletions packages/math-input/src/components/input/math-input.tsx
Original file line number Diff line number Diff line change
@@ -353,6 +353,9 @@ class MathInput extends React.Component<Props, State> {
});
};

// [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,
) => {
16 changes: 8 additions & 8 deletions packages/perseus-core/src/data-schema.ts
Original file line number Diff line number Diff line change
@@ -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<PerseusNumericInputAnswer>;
@@ -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 = {
Original file line number Diff line number Diff line change
@@ -29,6 +29,13 @@ const parseMathFormat = enumeration(
"pi",
);

const parseSimplify = enumeration(
"required",
"correct",
"enforced",
"optional",
);

export const parseNumericInputWidget: Parser<NumericInputWidget> = parseWidget(
constant("numeric-input"),
object({
@@ -48,8 +55,15 @@ export const parseNumericInputWidget: Parser<NumericInputWidget> = 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,
),
),
Original file line number Diff line number Diff line change
@@ -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,
10 changes: 5 additions & 5 deletions packages/perseus-editor/src/__stories__/editor.stories.tsx
Original file line number Diff line number Diff line change
@@ -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.
<div className="framework-perseus">
<SideBySide
leftTitle="Editor"
left={
<SplitView
rendererTitle="Editor"
renderer={
<View style={{width: "360px", margin: "20px"}}>
<Editor
ref={editorRef}
@@ -128,7 +128,7 @@ export const DemoInteractiveGraph = (): React.ReactElement => {
/>
</View>
}
rightTitle="Serialized Widget Options"
JSONTitle="Serialized Widget Options"
jsonObject={options}
/>
</div>
19 changes: 19 additions & 0 deletions packages/perseus-score/src/util/tex-wrangler.test.ts
Original file line number Diff line number Diff line change
@@ -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", "/");
});
});
12 changes: 12 additions & 0 deletions packages/perseus-score/src/util/tex-wrangler.ts
Original file line number Diff line number Diff line change
@@ -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 {
Original file line number Diff line number Diff line change
@@ -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,15 +267,15 @@ describe("scoreNumericInput", () => {
value: 1,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
{
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: "",
},
Loading