Skip to content

Commit 1b40412

Browse files
SonicScrewdriverSonicScrewdriver
committed
Refactoring Numeric Input helper functions to remove underscore (#2128)
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: #2128
1 parent e4d8294 commit 1b40412

File tree

15 files changed

+286
-1154
lines changed

15 files changed

+286
-1154
lines changed

‎.changeset/sharp-peaches-love.md

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@khanacademy/math-input": minor
3+
"@khanacademy/perseus": minor
4+
"@khanacademy/perseus-core": minor
5+
---
6+
7+
Refactoring Numeric Input helper functions to remove underscore, improve documentation, and add tests.

‎packages/perseus-core/src/data-schema.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -1178,16 +1178,16 @@ export type MathFormat =
11781178
| "pi";
11791179

11801180
export type PerseusNumericInputAnswerForm = {
1181-
simplify:
1182-
| "required"
1183-
| "correct"
1184-
| "enforced"
1185-
| "optional"
1186-
| null
1187-
| undefined;
1181+
simplify: PerseusNumericInputSimplify | null | undefined;
11881182
name: MathFormat;
11891183
};
11901184

1185+
export type PerseusNumericInputSimplify =
1186+
| "required"
1187+
| "correct"
1188+
| "enforced"
1189+
| "optional";
1190+
11911191
export type PerseusNumericInputWidgetOptions = {
11921192
// A list of all the possible correct and incorrect answers
11931193
answers: ReadonlyArray<PerseusNumericInputAnswer>;
@@ -1224,7 +1224,7 @@ export type PerseusNumericInputAnswer = {
12241224
// NOTE: perseus_data.go says this is non-nullable even though we handle null values.
12251225
maxError: number | null | undefined;
12261226
// Unsimplified answers are Ungraded, Accepted, or Wrong. Options: "required", "correct", or "enforced"
1227-
simplify: string | null | undefined;
1227+
simplify: PerseusNumericInputSimplify | null | undefined;
12281228
};
12291229

12301230
export type PerseusNumberLineWidgetOptions = {

‎packages/perseus-core/src/parse-perseus-json/perseus-parsers/numeric-input-widget.ts

+16-2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ const parseMathFormat = enumeration(
2929
"pi",
3030
);
3131

32+
const parseSimplify = enumeration(
33+
"required",
34+
"correct",
35+
"enforced",
36+
"optional",
37+
);
38+
3239
export const parseNumericInputWidget: Parser<NumericInputWidget> = parseWidget(
3340
constant("numeric-input"),
3441
object({
@@ -48,8 +55,15 @@ export const parseNumericInputWidget: Parser<NumericInputWidget> = parseWidget(
4855
// the data, we should simplify `simplify`.
4956
simplify: optional(
5057
nullable(
51-
union(string).or(
52-
pipeParsers(boolean).then(convert(String)).parser,
58+
union(parseSimplify).or(
59+
pipeParsers(boolean).then(
60+
convert((value) => {
61+
if (typeof value === "boolean") {
62+
return value ? "required" : "optional";
63+
}
64+
return value;
65+
}),
66+
).parser,
5367
).parser,
5468
),
5569
),

‎packages/perseus-core/src/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap

+2-2
Original file line numberDiff line numberDiff line change
@@ -5579,7 +5579,7 @@ exports[`parseAndTypecheckPerseusItem correctly parses data/numeric-input-answer
55795579
{
55805580
"maxError": 0,
55815581
"message": "",
5582-
"simplify": "true",
5582+
"simplify": "required",
55835583
"status": "correct",
55845584
"strict": false,
55855585
"value": 1.125,
@@ -6082,7 +6082,7 @@ exports[`parseAndTypecheckPerseusItem correctly parses data/numeric-input-with-s
60826082
{
60836083
"maxError": 0,
60846084
"message": "",
6085-
"simplify": "false",
6085+
"simplify": "optional",
60866086
"status": "correct",
60876087
"strict": false,
60886088
"value": 2.6,

‎packages/perseus-score/src/widgets/numeric-input/score-numeric-input.test.ts

+17-17
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ describe("scoreNumericInput", () => {
3636
value: 1,
3737
status: "correct",
3838
maxError: 0,
39-
simplify: "",
39+
simplify: "optional",
4040
strict: false,
4141
message: "",
4242
},
@@ -60,7 +60,7 @@ describe("scoreNumericInput", () => {
6060
value: 1,
6161
status: "correct",
6262
maxError: 0,
63-
simplify: "",
63+
simplify: "optional",
6464
strict: false,
6565
message: "",
6666
},
@@ -149,7 +149,7 @@ describe("scoreNumericInput", () => {
149149
value: 1,
150150
status: "correct",
151151
maxError: 0,
152-
simplify: "",
152+
simplify: "optional",
153153
strict: true,
154154
message: "",
155155
},
@@ -173,7 +173,7 @@ describe("scoreNumericInput", () => {
173173
value: 1,
174174
status: "correct",
175175
maxError: 0.2,
176-
simplify: "",
176+
simplify: "optional",
177177
strict: true,
178178
message: "",
179179
},
@@ -197,7 +197,7 @@ describe("scoreNumericInput", () => {
197197
value: 1,
198198
status: "correct",
199199
maxError: 0.2,
200-
simplify: "",
200+
simplify: "optional",
201201
strict: true,
202202
message: "",
203203
},
@@ -223,7 +223,7 @@ describe("scoreNumericInput", () => {
223223
value: 4,
224224
status: "wrong",
225225
maxError: 0,
226-
simplify: "",
226+
simplify: "optional",
227227
strict: false,
228228
message: "",
229229
},
@@ -232,7 +232,7 @@ describe("scoreNumericInput", () => {
232232
value: 10,
233233
status: "correct",
234234
maxError: 10,
235-
simplify: "",
235+
simplify: "optional",
236236
strict: false,
237237
message: "",
238238
},
@@ -267,15 +267,15 @@ describe("scoreNumericInput", () => {
267267
value: 1,
268268
status: "correct",
269269
maxError: 0,
270-
simplify: "",
270+
simplify: "optional",
271271
strict: false,
272272
message: "",
273273
},
274274
{
275275
value: -1,
276276
status: "correct",
277277
maxError: 0,
278-
simplify: "",
278+
simplify: "optional",
279279
strict: false,
280280
message: "",
281281
},
@@ -313,7 +313,7 @@ describe("scoreNumericInput", () => {
313313
// This answer is missing its value field.
314314
status: "correct",
315315
maxError: 0,
316-
simplify: "",
316+
simplify: "optional",
317317
strict: false,
318318
message: "",
319319
},
@@ -322,7 +322,7 @@ describe("scoreNumericInput", () => {
322322
value: 0.5,
323323
status: "correct",
324324
maxError: 0,
325-
simplify: "",
325+
simplify: "optional",
326326
strict: false,
327327
message: "",
328328
},
@@ -346,7 +346,7 @@ describe("scoreNumericInput", () => {
346346
value: null,
347347
status: "correct",
348348
maxError: 0,
349-
simplify: "",
349+
simplify: "optional",
350350
strict: false,
351351
message: "",
352352
},
@@ -355,7 +355,7 @@ describe("scoreNumericInput", () => {
355355
value: 0.5,
356356
status: "correct",
357357
maxError: 0,
358-
simplify: "",
358+
simplify: "optional",
359359
strict: false,
360360
message: "",
361361
},
@@ -375,7 +375,7 @@ describe("scoreNumericInput", () => {
375375
value: 0.2,
376376
status: "correct",
377377
maxError: 0,
378-
simplify: "",
378+
simplify: "optional",
379379
strict: false,
380380
message: "",
381381
},
@@ -398,7 +398,7 @@ describe("scoreNumericInput", () => {
398398
value: 1.2,
399399
status: "correct",
400400
maxError: 0,
401-
simplify: "",
401+
simplify: "optional",
402402
strict: false,
403403
message: "",
404404
},
@@ -421,7 +421,7 @@ describe("scoreNumericInput", () => {
421421
value: 1.1,
422422
status: "correct",
423423
maxError: 0,
424-
simplify: "",
424+
simplify: "optional",
425425
strict: false,
426426
message: "",
427427
},
@@ -441,7 +441,7 @@ describe("scoreNumericInput", () => {
441441
value: 0.9,
442442
status: "correct",
443443
maxError: 0,
444-
simplify: "",
444+
simplify: "optional",
445445
strict: false,
446446
message: "",
447447
},

‎packages/perseus/src/components/input-with-examples.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class InputWithExamples extends React.Component<Props, State> {
9393
const ariaId = `aria-for-${id}`;
9494
// Generate text from a known set of format options that will read well in a screen reader
9595
const examplesAria =
96-
this.props.examples.length === 7
96+
this.props.examples.length === 0
9797
? ""
9898
: `${this.props.examples[0]}
9999
${this.props.examples.slice(1).join(", or\n")}`

0 commit comments

Comments
 (0)