Conditionally switch decimal separator based on locale (#800)

## Summary: We were supposed to be switching between `,` and `.`, but we didn't see that through when updating how we're rendering icons for the v2 keypad. This updates that and adds some unit tests. <img width="386" alt="Screenshot 2023-11-14 at 10 21 29 AM" src="https://github.com/Khan/perseus/assets/16308368/0b70510d-7f5c-4735-8352-4ef1962f9774"> <img width="401" alt="Screenshot 2023-11-14 at 10 20 55 AM" src="https://github.com/Khan/perseus/assets/16308368/216d3787-24e9-49a5-9e4b-6f6ba8866803"> Issue: LC-1453 ## Test plan: - Use or simulate mobile web - Go to an exercise with a MathInput - Open the keypad by focusing the MathInput - Note a `.` as the decimal separator (if you're in the US) - Change your locale? - Use or simulate mobile web - Go to an exercise with a MathInput - Open the keypad by focusing the MathInput - Note a `,` as the decimal separator Author: handeyeco Reviewers: nedredmond, jeremywiebe, SonicScrewdriver Required Reviewers: Approved By: nedredmond Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Cypress (ubuntu-latest, 16.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald Pull Request URL: #800
Khan · Nov 15, 2023 · 1d58b88 · 1d58b88
1 parent 2adb82b
commit 1d58b88
Show file tree

Hide file tree

Showing 9 changed files with 76 additions and 27 deletions.
diff --git a/.changeset/spotty-snails-serve.md b/.changeset/spotty-snails-serve.md
@@ -0,0 +1,5 @@
+---
+"@khanacademy/math-input": patch
+---
+
+Bugfix: conditionally switch between period/comma for decimal separator
diff --git a/packages/math-input/src/components/key-handlers/key-translator.ts b/packages/math-input/src/components/key-handlers/key-translator.ts
@@ -1,4 +1,3 @@
-import {DecimalSeparator} from "../../enums";
 import {decimalSeparator} from "../../utils";
 import {mathQuillInstance} from "../input/mathquill-instance";
 
@@ -19,8 +18,6 @@ enum ActionType {
     MQ_END = 0,
 }
 
-const decimalSymbol = decimalSeparator === DecimalSeparator.COMMA ? "," : ".";
-
 function buildGenericCallback(
     str: string,
     type: ActionType = ActionType.WRITE,
@@ -72,7 +69,7 @@ const keyToMathquillMap: Record<Key, MathFieldUpdaterCallback> = {
     TAN: buildNormalFunctionCallback("tan"),
 
     CDOT: buildGenericCallback("\\cdot"),
-    DECIMAL: buildGenericCallback(decimalSymbol),
+    DECIMAL: buildGenericCallback(decimalSeparator),
     DIVIDE: buildGenericCallback("\\div"),
     EQUAL: buildGenericCallback("="),
     GEQ: buildGenericCallback("\\geq"),

diff --git a/packages/math-input/src/components/keypad/__tests__/__snapshots__/keypad.test.tsx.snap b/packages/math-input/src/components/keypad/__tests__/__snapshots__/keypad.test.tsx.snap
@@ -509,6 +509,7 @@ exports[`keypad should snapshot expanded: first render 1`] = `
                   class="default_xu2jcg-o_O-base_7gd6lb"
                 >
                   <svg
+                    data-test-id="period-decimal"
                     fill="none"
                     height="40"
                     viewBox="0 0 40 40"
@@ -1561,6 +1562,7 @@ exports[`keypad should snapshot unexpanded: first render 1`] = `
                   class="default_xu2jcg-o_O-base_7gd6lb"
                 >
                   <svg
+                    data-test-id="period-decimal"
                     fill="none"
                     height="40"
                     viewBox="0 0 40 40"

diff --git a/...ages/math-input/src/components/keypad/__tests__/__snapshots__/mobile-keypad.test.tsx.snap b/...ages/math-input/src/components/keypad/__tests__/__snapshots__/mobile-keypad.test.tsx.snap
@@ -407,6 +407,7 @@ exports[`mobile keypad should render keypad when active 1`] = `
                     class="default_xu2jcg-o_O-base_7gd6lb"
                   >
                     <svg
+                      data-test-id="period-decimal"
                       fill="none"
                       height="40"
                       viewBox="0 0 40 40"

diff --git a/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx b/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx
@@ -4,6 +4,7 @@ import * as React from "react";
 import "@testing-library/jest-dom";
 
 import keyConfigs from "../../../data/key-configs";
+import * as utils from "../../../utils";
 import {CursorContext} from "../../input/cursor-contexts";
 import Keypad from "../index";
 
@@ -19,6 +20,14 @@ const contextToKeyAria = {
 };
 
 describe("keypad", () => {
+    const originalDecimalSeparator = utils.decimalSeparator;
+
+    afterEach(() => {
+        // @ts-expect-error TS2540 - Cannot assign to 'decimalSeparator' because it is a read-only property.
+        // eslint-disable-next-line import/namespace
+        utils.decimalSeparator = originalDecimalSeparator;
+    });
+
     describe("shows navigation buttons", () => {
         Object.entries(contextToKeyAria).forEach(([context, ariaLabel]) => {
             it(`shows button for ${context}`, () => {
@@ -246,4 +255,30 @@ describe("keypad", () => {
             screen.getByRole("button", {name: "Left arrow"}),
         ).toBeInTheDocument();
     });
+
+    it(`can show the comma decimal separator`, () => {
+        // @ts-expect-error TS2540 - Cannot assign to 'decimalSeparator' because it is a read-only property.
+        // eslint-disable-next-line import/namespace
+        utils.decimalSeparator = utils.DecimalSeparator.COMMA;
+
+        // Arrange
+        // Act
+        render(
+            <Keypad onClickKey={() => {}} onAnalyticsEvent={async () => {}} />,
+        );
+
+        // Assert
+        expect(screen.getByTestId("comma-decimal")).toBeInTheDocument();
+    });
+
+    it(`can show the period decimal separator`, () => {
+        // Arrange
+        // Act
+        render(
+            <Keypad onClickKey={() => {}} onAnalyticsEvent={async () => {}} />,
+        );
+
+        // Assert
+        expect(screen.getByTestId("period-decimal")).toBeInTheDocument();
+    });
 });
diff --git a/packages/math-input/src/components/keypad/button-assets.tsx b/packages/math-input/src/components/keypad/button-assets.tsx
@@ -12,6 +12,8 @@ no copying and pasting is necessary.
 */
 import * as React from "react";
 
+import {DecimalSeparator, decimalSeparator} from "../../utils";
+
 import type Key from "../../data/keys";
 
 type Props = {id: Key};
@@ -168,25 +170,47 @@ export default function ButtonAsset({id}: Props): React.ReactElement {
                     />
                 </svg>
             );
-        // TODO(ned): Per the notes in `KeyConfigs`, shouldn't this be a comma
-        // that we replace with the period icon for i18n? Duplicating for now.
         case "DECIMAL":
         case "PERIOD":
+            // Different locales use different symbols for the decimal separator
+            // (, vs .)
+            if (
+                id === "DECIMAL" &&
+                decimalSeparator === DecimalSeparator.COMMA
+            ) {
+                // comma decimal separator
+                return (
+                    <svg
+                        width="40"
+                        height="40"
+                        viewBox="0 0 32 32"
+                        fill="none"
+                        xmlns="http://www.w3.org/2000/svg"
+                        data-test-id="comma-decimal"
+                    >
+                        <path
+                            d="M11.5559 25.3544C11.8679 24.661 12.1799 23.933 12.4919 23.1704C12.8039 22.425 13.0986 21.6884 13.3759 20.9604C13.6706 20.2324 13.9219 19.5737 14.1299 18.9844H16.6259L16.7299 19.2704C16.4526 19.877 16.1232 20.5357 15.7419 21.2464C15.3606 21.9397 14.9619 22.633 14.5459 23.3264C14.1299 24.037 13.7139 24.713 13.2979 25.3544H11.5559Z"
+                            fill="#21242C"
+                        />
+                    </svg>
+                );
+            }
+            // period / US decimal separator
             return (
                 <svg
                     width="40"
                     height="40"
                     viewBox="0 0 40 40"
                     fill="none"
                     xmlns="http://www.w3.org/2000/svg"
+                    data-test-id="period-decimal"
                 >
                     <path
                         d="M18.3401 27.512c0-.232.04-.448.12-.648.088-.208.204-.388.348-.54.152-.152.328-.272.528-.36.208-.088.428-.132.66-.132.232 0 .448.044.648.132.208.088.388.208.54.36.152.152.272.332.36.54.088.2.132.416.132.648 0 .24-.044.46-.132.66-.088.2-.208.376-.36.528-.152.152-.332.268-.54.348-.2.088-.416.132-.648.132-.232 0-.452-.044-.66-.132-.2-.08-.376-.196-.528-.348-.144-.152-.26-.328-.348-.528-.08-.2-.12-.42-.12-.66z"
                         fill="#21242C"
                     />
                 </svg>
             );
-
         case "PLUS":
             return (
                 <svg

diff --git a/packages/math-input/src/data/key-configs.ts b/packages/math-input/src/data/key-configs.ts
@@ -3,8 +3,7 @@
  */
 import * as i18n from "@khanacademy/wonder-blocks-i18n";
 
-import {DecimalSeparator, IconType} from "../enums";
-import {decimalSeparator} from "../utils";
+import {IconType} from "../enums";
 
 import type {KeyType} from "../enums";
 import type {KeyConfig} from "../types";
@@ -112,18 +111,6 @@ const KeyConfigs: {
             // I18N: A label for a 'decimal' sign (represented as '.' or ',').
             ariaLabel: i18n._("Decimal"),
         }),
-        icon:
-            decimalSeparator === DecimalSeparator.COMMA
-                ? {
-                      // TODO(charlie): Get an SVG icon for the comma, or verify with
-                      // design that the text-rendered version is acceptable.
-                      type: IconType.TEXT,
-                      data: ",",
-                  }
-                : {
-                      type: IconType.SVG,
-                      data: "PERIOD",
-                  },
     },
     PERIOD: {
         ...getDefaultOperatorFields({

diff --git a/packages/math-input/src/enums.ts b/packages/math-input/src/enums.ts
@@ -63,11 +63,6 @@ export enum IconType {
     TEXT = "TEXT",
 }
 
-export enum DecimalSeparator {
-    COMMA = "COMMA",
-    PERIOD = "PERIOD",
-}
-
 export enum EchoAnimationType {
     SLIDE_AND_FADE = "SLIDE_AND_FADE",
     FADE_ONLY = "FADE_ONLY",

diff --git a/packages/math-input/src/utils.ts b/packages/math-input/src/utils.ts
@@ -1,6 +1,9 @@
 import {getDecimalSeparator} from "@khanacademy/wonder-blocks-i18n";
 
-import {DecimalSeparator} from "./enums";
+export const DecimalSeparator = {
+    COMMA: ",",
+    PERIOD: ".",
+} as const;
 
 // NOTES(kevinb):
 // - In order to get the correct decimal separator for the current locale,