Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditionally switch decimal separator based on locale #800

Merged
merged 2 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/spotty-snails-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/math-input": patch
---

Bugfix: conditionally switch between period/comma for decimal separator
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import {DecimalSeparator} from "../../enums";
import {decimalSeparator} from "../../utils";
import {mathQuillInstance} from "../input/mathquill-instance";

Expand All @@ -19,8 +18,6 @@ enum ActionType {
MQ_END = 0,
}

const decimalSymbol = decimalSeparator === DecimalSeparator.COMMA ? "," : ".";

function buildGenericCallback(
str: string,
type: ActionType = ActionType.WRITE,
Expand Down Expand Up @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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}`, () => {
Expand Down Expand Up @@ -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();
});
});
30 changes: 27 additions & 3 deletions packages/math-input/src/components/keypad/button-assets.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
15 changes: 1 addition & 14 deletions packages/math-input/src/data/key-configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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({
Expand Down
5 changes: 0 additions & 5 deletions packages/math-input/src/enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 4 additions & 1 deletion packages/math-input/src/utils.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Loading