[numeric-refactor-feedback-cleanup] Numeric Input Refector Feedback (#…

…2268) ## Summary: This is a little PR that addresses some additional feedback points from the Numeric Input Refactor. This includes: - Using useId() for generating ids in InputWithExamples - Removing underscores from InputWithExamples after it was converted to a functional component as part of the Numeric Input Refactor work. - Removing unnecessary constructors - Rename of `getTooltipContent` to `renderTooltipContent` - Moving RenderProps to top of file for numeric-input.class.tsx - Removing unnecessary data from test data - Updating test name in utils.test.ts - Simplifying types from `NumericExampleStrings` after they were moved into the util file. - Commenting the reason for the `ariaLabel` setting to `undefined`. - New tests in answer-types ## Test plan: - Tests Pass - Manual testing in Storybook (Link to ZND here) Author: SonicScrewdriver Reviewers: mark-fitzgerald Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x) Pull Request URL: #2268
Khan · Feb 26, 2025 · 9d01457 · 9d01457
1 parent 4dde998
commit 9d01457
Show file tree

Hide file tree

Showing 15 changed files with 134 additions and 235 deletions.
diff --git a/.changeset/flat-carrots-reply.md b/.changeset/flat-carrots-reply.md
@@ -0,0 +1,6 @@
+---
+"@khanacademy/perseus": patch
+"@khanacademy/perseus-score": patch
+---
+
+Minor dev improvements for Numeric Input after Refactor changes.
diff --git a/packages/perseus-score/src/util/answer-types.test.ts b/packages/perseus-score/src/util/answer-types.test.ts
@@ -8,6 +8,14 @@ const validateFraction = (correctAnswer: string, guess: string) => {
     return validator(guess);
 };
 
+const validateImproperFraction = (correctAnswer: string, guess: string) => {
+    const validator = khanAnswerTypes.number.createValidatorFunctional(
+        correctAnswer,
+        {forms: ["improper"], strict: true, simplify: "optional"},
+    );
+    return validator(guess);
+};
+
 /**
  * Mobile keypad inputs submits fractions as for e.g. "\frac{1}{4}" to represent
  * "1/4". Whereas desktop browsers submits them as "1/4".
@@ -75,6 +83,14 @@ describe("For mobile keypad input", () => {
             expect(validateFraction("0.25", ".08").correct).toBe(false);
         });
     });
+
+    describe("when validator function is invoked with improper form", () => {
+        it("accepts a strict improper answer with tex answer", () => {
+            expect(validateImproperFraction("3", "\\frac{9}{3}").correct).toBe(
+                true,
+            );
+        });
+    });
 });
 
 describe("For desktop browser based inputs", () => {
@@ -111,4 +127,14 @@ describe("For desktop browser based inputs", () => {
             expect(validateFraction("0.25", ".08").correct).toBe(false);
         });
     });
+
+    describe("when validator function is invoked with improper form", () => {
+        it("rejects a strict improper with whole number answer", () => {
+            expect(validateImproperFraction("3", "3").correct).toBe(false);
+        });
+
+        it("accepts a strict improper answer with simple text answer", () => {
+            expect(validateImproperFraction("3", "9/3").correct).toBe(true);
+        });
+    });
 });
diff --git a/packages/perseus/src/components/simple-keypad-input.tsx b/packages/perseus/src/components/simple-keypad-input.tsx
@@ -21,12 +21,7 @@ export default class SimpleKeypadInput extends React.Component<any> {
     static contextType = KeypadContext;
     declare context: React.ContextType<typeof KeypadContext>;
     _isMounted = false;
-    inputRef: React.RefObject<KeypadInput>;
-
-    constructor(props: any) {
-        super(props);
-        this.inputRef = React.createRef<KeypadInput>();
-    }
+    inputRef = React.createRef<KeypadInput>();
 
     componentDidMount() {
         // TODO(scottgrant): This is a hack to remove the deprecated call to

diff --git a/packages/perseus/src/components/text-input.tsx b/packages/perseus/src/components/text-input.tsx
@@ -105,6 +105,10 @@ class TextInput extends React.Component<Props> {
 
         const formattedValue = value === null ? "" : value.toString();
 
+        // Some of our content was saved with empty strings as the label text,
+        // and we don't want to render an empty aria-label attribute.
+        const ariaLabel = labelText || undefined;
+
         return (
             <TextField
                 ref={this.inputRef}
@@ -113,7 +117,7 @@ class TextInput extends React.Component<Props> {
                 id={this.id}
                 value={formattedValue}
                 type="text"
-                aria-label={labelText || undefined}
+                aria-label={ariaLabel}
                 aria-describedby={this.props["aria-describedby"]}
                 onChange={(value) => this.props.onChange(value)}
                 placeholder={placeholder}

diff --git a/...ges/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set-jipt.test.ts.snap b/...ges/perseus/src/widgets/graded-group-set/__snapshots__/graded-group-set-jipt.test.ts.snap
@@ -67,13 +67,13 @@ exports[`graded-group-set should render all graded groups 1`] = `
                         autocomplete="off"
                         autocorrect="off"
                         class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-                        id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id=":r0:"
                         tabindex="0"
                         type="text"
                         value=""
                       />
                       <span
-                        id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id="aria-for-:r0:"
                         style="display: none;"
                       />
                     </div>
@@ -144,13 +144,13 @@ exports[`graded-group-set should render all graded groups 1`] = `
                         autocomplete="off"
                         autocorrect="off"
                         class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-                        id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id=":r3:"
                         tabindex="0"
                         type="text"
                         value=""
                       />
                       <span
-                        id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id="aria-for-:r3:"
                         style="display: none;"
                       />
                     </div>
@@ -221,13 +221,13 @@ exports[`graded-group-set should render all graded groups 1`] = `
                         autocomplete="off"
                         autocorrect="off"
                         class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-                        id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id=":r6:"
                         tabindex="0"
                         type="text"
                         value=""
                       />
                       <span
-                        id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id="aria-for-:r6:"
                         style="display: none;"
                       />
                     </div>

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
@@ -133,13 +133,13 @@ exports[`graded group widget should snapshot 1`] = `
                         autocomplete="off"
                         autocorrect="off"
                         class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-                        id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id=":r0:"
                         tabindex="0"
                         type="text"
                         value=""
                       />
                       <span
-                        id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                        id="aria-for-:r0:"
                         style="display: none;"
                       />
                     </div>

diff --git a/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap b/packages/perseus/src/widgets/group/__snapshots__/group.test.tsx.snap
@@ -756,13 +756,13 @@ exports[`group widget should snapshot: initial render 1`] = `
                             autocomplete="off"
                             autocorrect="off"
                             class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-                            id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                            id=":r0:"
                             tabindex="0"
                             type="text"
                             value=""
                           />
                           <span
-                            id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+                            id="aria-for-:r0:"
                             style="display: none;"
                           />
                         </div>
@@ -812,13 +812,13 @@ exports[`group widget should snapshot: initial render 1`] = `
                             autocomplete="off"
                             autocorrect="off"
                             class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-                            id="input-with-examples-bnVtZXJpYy1pbnB1dCAy"
+                            id=":r3:"
                             tabindex="0"
                             type="text"
                             value=""
                           />
                           <span
-                            id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAy"
+                            id="aria-for-:r3:"
                             style="display: none;"
                           />
                         </div>

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
@@ -100,13 +100,13 @@ exports[`numeric-input widget Should render predictably: after interaction 1`] =
             autocomplete="off"
             autocorrect="off"
             class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_ylhnsi"
-            id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id=":r6:"
             tabindex="0"
             type="text"
             value="1252"
           />
           <span
-            id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id="aria-for-:r6:"
             style="display: none;"
           />
         </div>
@@ -152,13 +152,13 @@ exports[`numeric-input widget Should render predictably: first render 1`] = `
             autocomplete="off"
             autocorrect="off"
             class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-            id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id=":r6:"
             tabindex="0"
             type="text"
             value=""
           />
           <span
-            id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id="aria-for-:r6:"
             style="display: none;"
           />
         </div>
@@ -197,21 +197,21 @@ exports[`numeric-input widget Should render tooltip as list when multiple format
           class="perseus-widget-container widget-nohighlight widget-inline-block"
         >
           <input
-            aria-describedby="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            aria-describedby="aria-for-:rf:"
             aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             autocapitalize="off"
             autocomplete="off"
             autocorrect="off"
             class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-            id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id=":rf:"
             tabindex="0"
             type="text"
             value=""
           />
           <span
-            id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id="aria-for-:rf:"
             style="display: none;"
           >
             Your answer should be 
@@ -255,7 +255,7 @@ exports[`numeric-input widget Should render tooltip using only correct answer fo
           class="perseus-widget-container widget-nohighlight widget-inline-block"
         >
           <input
-            aria-describedby="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            aria-describedby="aria-for-:ri:"
             aria-disabled="false"
             aria-invalid="false"
             aria-label="What's the answer?"
@@ -264,13 +264,13 @@ exports[`numeric-input widget Should render tooltip using only correct answer fo
             autocomplete="off"
             autocorrect="off"
             class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-            id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id=":ri:"
             tabindex="0"
             type="text"
             value=""
           />
           <span
-            id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id="aria-for-:ri:"
             style="display: none;"
           >
             Your answer should be 
@@ -312,21 +312,21 @@ exports[`numeric-input widget Should render tooltip when format option is given:
           class="perseus-widget-container widget-nohighlight widget-inline-block"
         >
           <input
-            aria-describedby="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            aria-describedby="aria-for-:r9:"
             aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             autocapitalize="off"
             autocomplete="off"
             autocorrect="off"
             class="input_1ck1z8k-o_O-LabelMedium_1rew30o-o_O-default_1tc7wjm-o_O-defaultFocus_lrnqlz-o_O-inputWithExamples_1y6ajxo"
-            id="input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id=":r9:"
             tabindex="0"
             type="text"
             value=""
           />
           <span
-            id="aria-for-input-with-examples-bnVtZXJpYy1pbnB1dCAx"
+            id="aria-for-:r9:"
             style="display: none;"
           >
             Your answer should be 

diff --git a/packages/perseus/src/widgets/numeric-input/input-with-examples.tsx b/packages/perseus/src/widgets/numeric-input/input-with-examples.tsx
@@ -1,7 +1,7 @@
 import * as PerseusLinter from "@khanacademy/perseus-linter";
 import Tooltip, {TooltipContent} from "@khanacademy/wonder-blocks-tooltip";
 import * as React from "react";
-import {forwardRef, useImperativeHandle} from "react";
+import {forwardRef, useId, useImperativeHandle} from "react";
 import _ from "underscore";
 
 import {PerseusI18nContext} from "../../components/i18n-context";
@@ -60,6 +60,8 @@ const InputWithExamples = forwardRef<
     const context = React.useContext(PerseusI18nContext);
     const inputRef = React.useRef<TextInput>(null);
     const [inputFocused, setInputFocused] = React.useState<boolean>(false);
+    const id = useId();
+    const ariaId = `aria-for-${id}`;
 
     useImperativeHandle(ref, () => ({
         current: inputRef.current,
@@ -75,11 +77,7 @@ const InputWithExamples = forwardRef<
         },
     }));
 
-    const _getUniqueId = () => {
-        return `input-with-examples-${btoa(props.id).replace(/=/g, "")}`;
-    };
-
-    const _getInputClassName = () => {
+    const getInputClassName = () => {
         let inputClassName = ApiClassNames.INPUT;
         if (inputFocused) {
             inputClassName += " " + ApiClassNames.FOCUSED;
@@ -90,10 +88,17 @@ const InputWithExamples = forwardRef<
         return inputClassName;
     };
 
-    const _renderInput = () => {
-        const id = _getUniqueId();
-        const ariaId = `aria-for-${id}`;
+    const handleFocus = () => {
+        onFocus();
+        setInputFocused(true);
+    };
+
+    const handleBlur = () => {
+        onBlur();
+        setInputFocused(false);
+    };
 
+    const renderInput = () => {
         // Generate the provided examples in simple language for screen readers.
         const examplesAria = props.shouldShowExamples
             ? `${props.examples[0]}
@@ -110,11 +115,11 @@ const InputWithExamples = forwardRef<
             // If we have examples, we want to provide the aria-describedby attribute
             "aria-describedby": props.shouldShowExamples ? ariaId : undefined,
             ref: inputRef,
-            className: _getInputClassName(),
+            className: getInputClassName(),
             labelText: props.labelText,
             value: props.value,
-            onFocus: _handleFocus,
-            onBlur: _handleBlur,
+            onFocus: handleFocus,
+            onBlur: handleBlur,
             disabled: disabled,
             style: props.style,
             onChange: props.onChange,
@@ -134,23 +139,10 @@ const InputWithExamples = forwardRef<
         );
     };
 
-    const _handleFocus = () => {
-        onFocus();
-        setInputFocused(true);
-    };
-
-    const _handleBlur = () => {
-        onBlur();
-        setInputFocused(false);
-    };
-
-    const getTooltipContent = () => {
+    const renderTooltipContent = () => {
         return (
             <TooltipContent>
-                <div
-                    id={_getUniqueId()}
-                    className="input-with-examples-tooltip"
-                >
+                <div id={id} className="input-with-examples-tooltip">
                     <Renderer
                         content={examplesContent}
                         linterContext={PerseusLinter.pushContextStack(
@@ -181,11 +173,11 @@ const InputWithExamples = forwardRef<
 
     return (
         <Tooltip
-            content={getTooltipContent()}
+            content={renderTooltipContent()}
             opened={showExamplesTooltip}
             placement="bottom"
         >
-            {_renderInput()}
+            {renderInput()}
         </Tooltip>
     );
 });