From 1d58b887179b129b5027e20484fde5169170f052 Mon Sep 17 00:00:00 2001 From: Matthew Date: Wed, 15 Nov 2023 09:28:32 -0600 Subject: [PATCH] Conditionally switch decimal separator based on locale (#800) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. Screenshot 2023-11-14 at 10 21 29 AM Screenshot 2023-11-14 at 10 20 55 AM 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: https://github.com/Khan/perseus/pull/800 --- .changeset/spotty-snails-serve.md | 5 +++ .../components/key-handlers/key-translator.ts | 5 +-- .../__snapshots__/keypad.test.tsx.snap | 2 ++ .../__snapshots__/mobile-keypad.test.tsx.snap | 1 + .../keypad/__tests__/keypad.test.tsx | 35 +++++++++++++++++++ .../src/components/keypad/button-assets.tsx | 30 ++++++++++++++-- packages/math-input/src/data/key-configs.ts | 15 +------- packages/math-input/src/enums.ts | 5 --- packages/math-input/src/utils.ts | 5 ++- 9 files changed, 76 insertions(+), 27 deletions(-) create mode 100644 .changeset/spotty-snails-serve.md diff --git a/.changeset/spotty-snails-serve.md b/.changeset/spotty-snails-serve.md new file mode 100644 index 0000000000..f682581293 --- /dev/null +++ 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 index ca4c170e74..3a0e36a168 100644 --- 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 = { 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 index 4149389e8e..2970bbe512 100644 --- 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" > { + 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( + {}} onAnalyticsEvent={async () => {}} />, + ); + + // Assert + expect(screen.getByTestId("comma-decimal")).toBeInTheDocument(); + }); + + it(`can show the period decimal separator`, () => { + // Arrange + // Act + render( + {}} 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 index cb5887ed11..dd6d1f498d 100644 --- 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,10 +170,32 @@ export default function ButtonAsset({id}: Props): React.ReactElement { /> ); - // 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 ( + + + + ); + } + // period / US decimal separator return ( ); - case "PLUS": return (