Skip to content

Commit

Permalink
Update Mathquill Mathfield with Portuguese trig functions (#1538)
Browse files Browse the repository at this point in the history
## Summary:
So basically:
- These two PRs ([1](#921) and [2](Khan/webapp#19111)) helped add support for Portuguese trig functions when typed in the input.
- This [PR](#1497) added support for the text of trig buttons to be translated, however the input would still update with "sin" if the "sen" button was pressed.
- The PR being reviewed updates the input correctly; so if the "sen" button is pressed, the input is updated with "sen"

How this works:
- When a button is pressed, the keypad yells into the void that "SIN" was pressed
- The keypad translator has a callback for updating the input when "SIN" is pressed
  - BEFORE: it would just updated the input with `sin()`
  - NOW: it looks for the translated version of "sin", checks it against known safe translations, and either updates it with a known safe translation or the default "sin"

Issue: LEMS-1621

## Test plan:
Once the strings are translated:
- Go to a Portuguese page with an expression widget
- Go to a question that requires the "sin" button
- Open the keypad
- Note the button is "sen" instead of "sin"
- Click the "sen" button
- Note the input is updated with "sen()"
- Fill out the correct answer and submit
- Note the validator accepts "sen" instead of "sin"

Author: handeyeco

Reviewers: handeyeco, benchristel, anakaren-rojas

Required Reviewers:

Approved By: benchristel

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1538
  • Loading branch information
handeyeco authored Aug 22, 2024
1 parent 811f914 commit 96f0337
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 12 deletions.
6 changes: 6 additions & 0 deletions .changeset/orange-rules-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/math-input": patch
"@khanacademy/perseus": patch
---

Use Portuguese sen and tg when updating Mathquill from the keypad
2 changes: 1 addition & 1 deletion packages/math-input/src/components/input/math-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class MathWrapper {
this.callbacks = callbacks;

this.mobileKeyTranslator = {
...getKeyTranslator(locale),
...getKeyTranslator(locale, strings),
// note(Matthew): our mobile backspace logic is really complicated
// and for some reason doesn't really work in the desktop experience.
// So we default to the basic backspace functionality in the
Expand Down
38 changes: 35 additions & 3 deletions packages/math-input/src/components/key-handlers/key-translator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,46 @@ function buildGenericCallback(
};
}

/**
* This lets us use translated functions
* (like tg->tan and sen->sin) when we know it's safe to.
* This lets us progressively support translations without needing
* to support every language all at once.
*
* @param {string} command - the translated command/function to check
* @param {string[]} supportedTranslations - list of translations we support
* @param {string} defaultCommand - what to fallback to if the command isn't supported
*/
function buildTranslatableFunctionCallback(
command: string,
supportedTranslations: string[],
defaultCommand: string,
) {
const cmd = supportedTranslations.includes(command)
? command
: defaultCommand;
return function (mathField: MathFieldInterface) {
mathField.write(`${cmd}\\left(\\right)`);
mathField.keystroke("Left");
};
}

function buildNormalFunctionCallback(command: string) {
return function (mathField: MathFieldInterface) {
mathField.write(`\\${command}\\left(\\right)`);
mathField.keystroke("Left");
};
}

type KeyTranslatorStrings = {
sin: string;
cos: string;
tan: string;
};

export const getKeyTranslator = (
locale: string,
strings: KeyTranslatorStrings,
): Record<Key, MathFieldUpdaterCallback> => ({
EXP: handleExponent,
EXP_2: handleExponent,
Expand All @@ -66,9 +97,10 @@ export const getKeyTranslator = (

LOG: buildNormalFunctionCallback("log"),
LN: buildNormalFunctionCallback("ln"),
SIN: buildNormalFunctionCallback("sin"),
COS: buildNormalFunctionCallback("cos"),
TAN: buildNormalFunctionCallback("tan"),

COS: buildNormalFunctionCallback(strings.cos),
SIN: buildTranslatableFunctionCallback(strings.sin, ["sin", "sen"], "sin"),
TAN: buildTranslatableFunctionCallback(strings.tan, ["tan", "tg"], "tan"),

CDOT: buildGenericCallback("\\cdot"),
DECIMAL: buildGenericCallback(getDecimalSeparator(locale)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Props = {
onChangeMathInput: (mathInputTex: string) => void;
keypadClosed?: boolean;
onAnalyticsEvent?: AnalyticsEventHandlerFn;
portuguese?: boolean;
};

function V2KeypadWithMathquill(props: Props) {
Expand All @@ -27,6 +28,14 @@ function V2KeypadWithMathquill(props: Props) {
const [keypadOpen, setKeypadOpen] = React.useState<boolean>(!keypadClosed);
const {strings} = useMathInputI18n();

if (props.portuguese) {
strings.sin = "sen";
strings.tan = "tg";
} else {
strings.sin = "sin";
strings.tan = "tan";
}

React.useEffect(() => {
if (!mathField && mathFieldWrapperRef.current) {
const mathFieldInstance = createMathField(
Expand All @@ -48,7 +57,7 @@ function V2KeypadWithMathquill(props: Props) {
}
}, [mathField, strings, onChangeMathInput]);

const keyTranslator = getKeyTranslator("en");
const keyTranslator = getKeyTranslator("en", strings);

function handleClickKey(key: Key) {
if (!mathField) {
Expand Down Expand Up @@ -325,4 +334,94 @@ describe("Keypad v2 with MathQuill", () => {
payload: {virtualKeypadVersion: "MATH_INPUT_KEYPAD_V2"},
});
});

it("handles english sin trig function", async () => {
// Arrange
const mockMathInputCallback = jest.fn();
render(
<V2KeypadWithMathquill onChangeMathInput={mockMathInputCallback} />,
);

// Act
await userEvent.click(screen.getByRole("tab", {name: "Geometry"}));
await userEvent.click(screen.getByRole("button", {name: "Sine"}));
await userEvent.click(screen.getByRole("tab", {name: "Numbers"}));
await userEvent.click(screen.getByRole("button", {name: "4"}));
await userEvent.click(screen.getByRole("button", {name: "2"}));

// Assert
expect(mockMathInputCallback).toHaveBeenLastCalledWith(
"\\sin\\left(42\\right)",
);
});

it("handles portuguese sen trig function", async () => {
// Arrange
const mockMathInputCallback = jest.fn();
render(
<V2KeypadWithMathquill
onChangeMathInput={mockMathInputCallback}
portuguese
/>,
);

// Act
await userEvent.click(screen.getByRole("tab", {name: "Geometry"}));
// This needs to stay as "getByText" because we're validating translations
// and aria-labels are in English
await userEvent.click(screen.getByText("sen"));
await userEvent.click(screen.getByRole("tab", {name: "Numbers"}));
await userEvent.click(screen.getByRole("button", {name: "4"}));
await userEvent.click(screen.getByRole("button", {name: "2"}));

// Assert
expect(mockMathInputCallback).toHaveBeenLastCalledWith(
"\\operatorname{sen}\\left(42\\right)",
);
});

it("handles english tan trig function", async () => {
// Arrange
const mockMathInputCallback = jest.fn();
render(
<V2KeypadWithMathquill onChangeMathInput={mockMathInputCallback} />,
);

// Act
await userEvent.click(screen.getByRole("tab", {name: "Geometry"}));
await userEvent.click(screen.getByRole("button", {name: "Tangent"}));
await userEvent.click(screen.getByRole("tab", {name: "Numbers"}));
await userEvent.click(screen.getByRole("button", {name: "4"}));
await userEvent.click(screen.getByRole("button", {name: "2"}));

// Assert
expect(mockMathInputCallback).toHaveBeenLastCalledWith(
"\\tan\\left(42\\right)",
);
});

it("handles portuguese tg trig function", async () => {
// Arrange
const mockMathInputCallback = jest.fn();
render(
<V2KeypadWithMathquill
onChangeMathInput={mockMathInputCallback}
portuguese
/>,
);

// Act
await userEvent.click(screen.getByRole("tab", {name: "Geometry"}));
// This needs to stay as "getByText" because we're validating translations
// and aria-labels are in English
await userEvent.click(screen.getByText("tg"));
await userEvent.click(screen.getByRole("tab", {name: "Numbers"}));
await userEvent.click(screen.getByRole("button", {name: "4"}));
await userEvent.click(screen.getByRole("button", {name: "2"}));

// Assert
expect(mockMathInputCallback).toHaveBeenLastCalledWith(
"\\operatorname{tg}\\left(42\\right)",
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ export function V2KeypadWithMathquill() {
}
}, [mathField]);

const keyTranslator = getKeyTranslator("en");
const keyTranslator = getKeyTranslator("en", {
sin: "sin",
cos: "cos",
tan: "tan",
});

function handleClickKey(key: Key) {
if (!mathField) {
Expand Down Expand Up @@ -86,10 +90,7 @@ export function V2KeypadWithMathquill() {
convertDotToTimes
preAlgebra
trigonometry
onAnalyticsEvent={async (event) => {
// eslint-disable-next-line no-console
console.log("Send Event:", event);
}}
onAnalyticsEvent={async () => {}}
showDismiss
/>
</PopoverContentCore>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default function GeometryPage(props: Props) {
const {onClickKey} = props;
const {strings} = useMathInputI18n();
const Keys = KeyConfigs(strings);

return (
<>
{/* Row 1 */}
Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/components/math-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class InnerMathInput extends React.Component<InnerProps, State> {
const input = this.mathField();
const {locale} = this.context;
const customKeyTranslator = {
...getKeyTranslator(locale),
...getKeyTranslator(locale, this.context.strings),
// If there's something in the input that can become part of a
// fraction, typing "/" puts it in the numerator. If not, typing
// "/" does nothing. In that case, enter a \frac.
Expand Down Expand Up @@ -259,7 +259,7 @@ class InnerMathInput extends React.Component<InnerProps, State> {

handleKeypadPress: (key: Keys, e: any) => void = (key, e) => {
const {locale} = this.context;
const translator = getKeyTranslator(locale)[key];
const translator = getKeyTranslator(locale, this.context.strings)[key];
const mathField = this.mathField();

if (mathField) {
Expand Down
9 changes: 9 additions & 0 deletions packages/perseus/src/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ export type PerseusStrings = {
videoWrapper: string;
mathInputTitle: string;
mathInputDescription: string;
sin: string;
cos: string;
tan: string;
};

/**
Expand Down Expand Up @@ -291,6 +294,9 @@ export const strings: {
mathInputTitle: "mathematics keyboard",
mathInputDescription:
"Use keyboard/mouse to interact with math-based input fields",
sin: "sin",
cos: "cos",
tan: "tan",
};

/**
Expand Down Expand Up @@ -441,4 +447,7 @@ export const mockStrings: PerseusStrings = {
mathInputTitle: "mathematics keyboard",
mathInputDescription:
"Use keyboard/mouse to interact with math-based input fields",
sin: "sin",
cos: "cos",
tan: "tan",
};
10 changes: 10 additions & 0 deletions packages/perseus/src/widgets/__tests__/expression.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,16 @@ describe("Expression Widget", function () {
const item = expressionItemWithAnswer("sin(x)");
await assertIncorrect(userEvent, item, "2");
});

it("allows portugese sen", async () => {
const item = expressionItemWithAnswer("sin(42)");
await assertCorrect(userEvent, item, "sen(42)");
});

it("allows portugese tg", async () => {
const item = expressionItemWithAnswer("tan(42)");
await assertCorrect(userEvent, item, "tg(42)");
});
});

describe("analytics", () => {
Expand Down

0 comments on commit 96f0337

Please sign in to comment.