Skip to content

Commit

Permalink
Refactoring Numeric Input helper functions to remove underscore (#2128)
Browse files Browse the repository at this point in the history
## Summary:
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

## Test plan:
- 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: #2128
  • Loading branch information
SonicScrewdriver authored Jan 23, 2025
1 parent 660c0ee commit 3568ca6
Show file tree
Hide file tree
Showing 15 changed files with 286 additions and 1,154 deletions.
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.
16 changes: 8 additions & 8 deletions packages/perseus-core/src/data-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1164,16 +1164,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>;
Expand Down Expand Up @@ -1210,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: string | null | undefined;
simplify: PerseusNumericInputSimplify | null | undefined;
};

export type PerseusNumberLineWidgetOptions = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe("static function validate", () => {
value: 1,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -34,7 +34,7 @@ describe("static function validate", () => {
value: 1,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand Down Expand Up @@ -123,7 +123,7 @@ describe("static function validate", () => {
value: 1,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: true,
message: "",
},
Expand All @@ -147,7 +147,7 @@ describe("static function validate", () => {
value: 1,
status: "correct",
maxError: 0.2,
simplify: "",
simplify: "optional",
strict: true,
message: "",
},
Expand All @@ -171,7 +171,7 @@ describe("static function validate", () => {
value: 1,
status: "correct",
maxError: 0.2,
simplify: "",
simplify: "optional",
strict: true,
message: "",
},
Expand All @@ -197,7 +197,7 @@ describe("static function validate", () => {
value: 4,
status: "wrong",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -206,7 +206,7 @@ describe("static function validate", () => {
value: 10,
status: "correct",
maxError: 10,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand Down Expand Up @@ -241,15 +241,15 @@ describe("static function validate", () => {
value: 1,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
{
value: -1,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand Down Expand Up @@ -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: "",
},
Expand All @@ -296,7 +296,7 @@ describe("static function validate", () => {
value: 0.5,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -320,7 +320,7 @@ describe("static function validate", () => {
value: null,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -329,7 +329,7 @@ describe("static function validate", () => {
value: 0.5,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -349,7 +349,7 @@ describe("static function validate", () => {
value: 0.2,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -372,7 +372,7 @@ describe("static function validate", () => {
value: 1.2,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -395,7 +395,7 @@ describe("static function validate", () => {
value: 1.1,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand All @@ -415,7 +415,7 @@ describe("static function validate", () => {
value: 0.9,
status: "correct",
maxError: 0,
simplify: "",
simplify: "optional",
strict: false,
message: "",
},
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/components/input-with-examples.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class InputWithExamples extends React.Component<Props, State> {
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")}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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,
),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 3568ca6

Please sign in to comment.