From 5bcf118c3190d92c2d5112b5f559e67cad5a906b Mon Sep 17 00:00:00 2001 From: Ned Redmond Date: Wed, 11 Oct 2023 15:06:52 -0400 Subject: [PATCH] Roll v2 Keypad Onto Desktop Web (#711) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/Khan/perseus/assets/23404711/b199ff5b-9db6-4394-8ef3-120531564ab3 Updates Perseus / Math Input to use keypad. Also changes how it's used: - Instead of a set of keys appearing on focus, the button on the right acts as a toggle. - Instead of a warning icon appearing when there is an parsing, the input shows a red "invalid" state and a tooltip appears with a friendly message. - `buttonsVisible` prop states changed: - `focused` default behavior, toggle off to start - `always` default behavior, toggle on to start - `never` toggle button disabled - `buttonSets` type was marked as deprecated - `keypadButtonSets` prop added that takes new type - `buttonSets` maps to `keypadButtonSets` - Also adds optional `extraKeys` prop that takes an array of `Keys`. Author: nedredmond Reviewers: jeremywiebe, nedredmond, handeyeco Required Reviewers: Approved By: jeremywiebe Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 16.x), ✅ Cypress (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 16.x), ✅ gerald, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x) Pull Request URL: https://github.com/Khan/perseus/pull/711 --- .changeset/great-pigs-deny.md | 9 + .changeset/stale-otters-fix.md | 7 + packages/math-input/src/data/keys.ts | 233 +++++------ packages/math-input/src/index.ts | 1 + .../src/widgets/expression-editor.tsx | 63 ++- .../src/widgets/interaction-editor.tsx | 58 +-- .../widgets/interaction/constraint-editor.tsx | 14 +- .../__tests__/mock-asset-loading-widget.tsx | 3 +- .../__stories__/math-input.stories.tsx | 21 +- .../components/__tests__/math-input.test.tsx | 103 ++++- .../perseus/src/components/math-input.tsx | 370 +++++++++++++----- .../perseus/src/components/tex-buttons.tsx | 4 +- packages/perseus/src/perseus-types.ts | 12 +- .../perseus/src/styles/perseus-renderer.less | 11 +- .../src/styles/widgets/expression.less | 6 - .../__stories__/expression.stories.tsx | 6 +- .../src/widgets/__tests__/expression.test.tsx | 81 +--- packages/perseus/src/widgets/expression.tsx | 193 ++++----- 18 files changed, 730 insertions(+), 465 deletions(-) create mode 100644 .changeset/great-pigs-deny.md create mode 100644 .changeset/stale-otters-fix.md diff --git a/.changeset/great-pigs-deny.md b/.changeset/great-pigs-deny.md new file mode 100644 index 0000000000..baeb2fbb51 --- /dev/null +++ b/.changeset/great-pigs-deny.md @@ -0,0 +1,9 @@ +--- +"@khanacademy/perseus": major +"@khanacademy/perseus-editor": patch +--- + +# Update MathInput + +- `buttonSets` is now deprecated in favor of `keypadButtonSets`, but currently maps to the new prop for backwards compatability. +- `buttonsVisible` is now a bit misleading: "focused" is the default state with a toggle-able keypad and "always" shows the keypad by default. diff --git a/.changeset/stale-otters-fix.md b/.changeset/stale-otters-fix.md new file mode 100644 index 0000000000..ba79916e4e --- /dev/null +++ b/.changeset/stale-otters-fix.md @@ -0,0 +1,7 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/math-input": minor +"@khanacademy/perseus-editor": minor +--- + +Desktop Expression Widget now uses v2 keypad diff --git a/packages/math-input/src/data/keys.ts b/packages/math-input/src/data/keys.ts index e5b873da78..0ed8a2f524 100644 --- a/packages/math-input/src/data/keys.ts +++ b/packages/math-input/src/data/keys.ts @@ -1,120 +1,123 @@ -type Key = - | "PLUS" - | "MINUS" - | "NEGATIVE" - | "TIMES" - | "DIVIDE" - | "DECIMAL" - | "PERIOD" - | "PERCENT" - | "CDOT" - | "EQUAL" - | "NEQ" - | "GT" - | "LT" - | "GEQ" - | "LEQ" // mobile native only - | "FRAC_INCLUSIVE" // mobile native only - | "FRAC_EXCLUSIVE" // mobile native only - | "FRAC" - | "EXP" - | "EXP_2" - | "EXP_3" - | "SQRT" - | "CUBE_ROOT" - | "RADICAL" - | "LEFT_PAREN" - | "RIGHT_PAREN" - | "LN" - | "LOG" - | "LOG_N" - | "SIN" - | "COS" // TODO(charlie): Add in additional Greek letters. - | "TAN" - | "PI" - | "THETA" - | "UP" - | "RIGHT" - | "DOWN" - | "LEFT" - | "BACKSPACE" - | "DISMISS" - | "JUMP_OUT_PARENTHESES" - | "JUMP_OUT_EXPONENT" - | "JUMP_OUT_BASE" - | "JUMP_INTO_NUMERATOR" - | "JUMP_OUT_NUMERATOR" - | "JUMP_OUT_DENOMINATOR" // Multi-functional keys. - | "NOOP" // mobile native only - | "MANY" // A custom key that captures an arbitrary number of symbols but has no 'default' symbol or action. - | "NUM_0" - | "NUM_1" - | "NUM_2" - | "NUM_3" - | "NUM_4" - | "NUM_5" - | "NUM_6" - | "NUM_7" - | "NUM_8" - | "NUM_9" - | "a" - | "b" - | "c" - | "d" - | "e" - | "f" - | "g" - | "h" - | "i" - | "j" - | "k" - | "l" - | "m" - | "n" - | "o" - | "p" - | "q" - | "r" - | "s" - | "t" - | "u" - | "v" - | "w" - | "x" - | "y" - | "z" - | "A" - | "B" - | "C" - | "D" - | "E" - | "F" - | "G" - | "H" - | "I" - | "J" - | "K" - | "L" - | "M" - | "N" - | "O" - | "P" - | "Q" - | "R" - | "S" - | "T" - | "U" - | "V" - | "W" - | "X" - | "Y" - | "Z" +export const KeyArray = [ + "PLUS", + "MINUS", + "NEGATIVE", + "TIMES", + "DIVIDE", + "DECIMAL", + "PERIOD", + "PERCENT", + "CDOT", + "EQUAL", + "NEQ", + "GT", + "LT", + "GEQ", + "LEQ", // mobile native only + "FRAC_INCLUSIVE", // mobile native only + "FRAC_EXCLUSIVE", // mobile native only + "FRAC", + "EXP", + "EXP_2", + "EXP_3", + "SQRT", + "CUBE_ROOT", + "RADICAL", + "LEFT_PAREN", + "RIGHT_PAREN", + "LN", + "LOG", + "LOG_N", + "SIN", + "COS", // TODO(charlie): Add in additional Greek letters., + "TAN", + "PI", + "THETA", + "UP", + "RIGHT", + "DOWN", + "LEFT", + "BACKSPACE", + "DISMISS", + "JUMP_OUT_PARENTHESES", + "JUMP_OUT_EXPONENT", + "JUMP_OUT_BASE", + "JUMP_INTO_NUMERATOR", + "JUMP_OUT_NUMERATOR", + "JUMP_OUT_DENOMINATOR", // Multi-functional keys. + "NOOP", // mobile native only + "MANY", // A custom key that captures an arbitrary number of symbols but has no 'default' symbol or action. + "NUM_0", + "NUM_1", + "NUM_2", + "NUM_3", + "NUM_4", + "NUM_5", + "NUM_6", + "NUM_7", + "NUM_8", + "NUM_9", + "a", + "b", + "c", + "d", + "e", + "f", + "g", + "h", + "i", + "j", + "k", + "l", + "m", + "n", + "o", + "p", + "q", + "r", + "s", + "t", + "u", + "v", + "w", + "x", + "y", + "z", + "A", + "B", + "C", + "D", + "E", + "F", + "G", + "H", + "I", + "J", + "K", + "L", + "M", + "N", + "O", + "P", + "Q", + "R", + "S", + "T", + "U", + "V", + "W", + "X", + "Y", + "Z", // Currently only used by // Perseus' Expression MathInput - | "PHI" - | "NTHROOT3" - | "POW" - | "LOG_B"; + "PHI", + "NTHROOT3", + "POW", + "LOG_B", +] as const; + +type Key = typeof KeyArray[number]; export default Key; diff --git a/packages/math-input/src/index.ts b/packages/math-input/src/index.ts index 858b839892..d566b07a0a 100644 --- a/packages/math-input/src/index.ts +++ b/packages/math-input/src/index.ts @@ -43,6 +43,7 @@ export type {KeypadAPI, KeypadConfiguration} from "./types"; // Key list, configuration map, and types export type {default as Keys} from "./data/keys"; +export {KeyArray} from "./data/keys"; export {default as KeyConfigs} from "./data/key-configs"; export {type KeyType, KeypadType} from "./enums"; diff --git a/packages/perseus-editor/src/widgets/expression-editor.tsx b/packages/perseus-editor/src/widgets/expression-editor.tsx index 063cccd101..bb3a721385 100644 --- a/packages/perseus-editor/src/widgets/expression-editor.tsx +++ b/packages/perseus-editor/src/widgets/expression-editor.tsx @@ -26,6 +26,10 @@ type Props = { } & Omit & Changeable.ChangeableProps; +// types for iterables +type AnswerForm = PerseusExpressionWidgetOptions["answerForms"][number]; +type LegacyButtonSet = PerseusExpressionWidgetOptions["buttonSets"][number]; + type DefaultProps = { answerForms: Props["answerForms"]; times: Props["times"]; @@ -33,14 +37,22 @@ type DefaultProps = { functions: Props["functions"]; }; +const parseAnswerKey = ({key}: AnswerForm): number => { + const parsedKey = Number.parseInt(key ?? ""); + if (Number.isNaN(parsedKey)) { + throw new Error(`Invalid answer key: ${key}`); + } + return parsedKey; +}; + // Pick a key that isn't currently used by an answer in answerForms -const _makeNewKey = (answerForms: any) => { +const _makeNewKey = (answerForms: ReadonlyArray) => { // first note all the currently used keys in an array, used like a map :3 // note that this automatically updates the array's length property to // be one past the largest key. const usedKeys: Array = []; answerForms.forEach((ans) => { - usedKeys[ans.key] = true; + usedKeys[parseAnswerKey(ans)] = true; }); // then scan through the array to find the first unused (undefined) key @@ -98,7 +110,9 @@ class ExpressionEditor extends React.Component { render(): React.ReactNode { const answerOptions = this.props.answerForms - .map((obj, index) => { + .map((ans: AnswerForm) => { + const key = parseAnswerKey(ans); + const expressionProps = { // note we're using // *this.props*.{times,functions,buttonSets} since each @@ -108,26 +122,30 @@ class ExpressionEditor extends React.Component { buttonSets: this.props.buttonSets, buttonsVisible: "focused", - form: obj.form, - simplify: obj.simplify, - value: obj.value, + form: ans.form, + simplify: ans.simplify, + value: ans.value, - onChange: (props) => this.updateForm(index, props), + onChange: (props) => this.updateForm(key, props), trackInteraction: () => {}, - widgetId: this.props.widgetId + "-" + index, + widgetId: this.props.widgetId + "-" + ans.key, } as const; - return lens(obj) + return lens(ans) .merge([], { draggable: true, - onChange: (props) => this.updateForm(index, props), - onDelete: () => this.handleRemoveForm(index), + onChange: (props) => + this.updateForm( + Number.parseInt(ans.key ?? ""), + props, + ), + onDelete: () => this.handleRemoveForm(key), expressionProps: expressionProps, }) .freeze(); }) - .map((obj, index) => ); + .map((obj) => ); const sortable = ( { // The first one gets special cased to always be checked, disabled, // and float left. const isFirst = name === "basic"; - const checked = this.props.buttonSets.includes(name) || isFirst; + const checked = + this.props.buttonSets.includes( + name as PerseusExpressionWidgetOptions["buttonSets"][number], + ) || isFirst; const className = isFirst ? "button-set-label-float" : "button-set-label"; @@ -342,7 +363,7 @@ class ExpressionEditor extends React.Component { this.change({answerForms}); }; - handleRemoveForm: (arg1: number) => void = (i) => { + handleRemoveForm: (answerKey: number) => void = (i) => { const answerForms = this.props.answerForms.slice(); answerForms.splice(i, 1); this.change({answerForms}); @@ -350,7 +371,7 @@ class ExpressionEditor extends React.Component { // called when the options (including the expression itself) to an answer // form change - updateForm: (arg1: number, arg2: any) => void = (i, props) => { + updateForm: (i: number, props: any) => void = (i, props) => { const answerForms = lens(this.props.answerForms) .merge([i], props) .freeze(); @@ -358,7 +379,7 @@ class ExpressionEditor extends React.Component { this.change({answerForms}); }; - handleReorder: (arg1: any) => void = (components) => { + handleReorder: (components: any) => void = (components) => { const answerForms = components.map((component) => { const form = _(component.props).pick( "considered", @@ -375,8 +396,10 @@ class ExpressionEditor extends React.Component { }; // called when the selected buttonset changes - handleButtonSet: (arg1: string) => void = (changingName) => { - const buttonSetNames = Object.keys(TexButtons.buttonSets); + handleButtonSet: (changingName: string) => void = (changingName) => { + const buttonSetNames = Object.keys( + TexButtons.buttonSets, + ) as LegacyButtonSet[]; // Filter to preserve order - using .union and .difference would always // move the last added button set to the end. @@ -394,8 +417,8 @@ class ExpressionEditor extends React.Component { // "basic+div". Toggle between the two of them. // If someone can think of a more elegant formulation of this (there // must be one!) feel free to change it. - let keep: string | undefined; - let remove: string | undefined; + let keep: LegacyButtonSet | undefined; + let remove: LegacyButtonSet | undefined; if (this.props.buttonSets.includes("basic+div")) { keep = "basic"; remove = "basic+div"; diff --git a/packages/perseus-editor/src/widgets/interaction-editor.tsx b/packages/perseus-editor/src/widgets/interaction-editor.tsx index 1ee2c05ce5..7c0b553c1b 100644 --- a/packages/perseus-editor/src/widgets/interaction-editor.tsx +++ b/packages/perseus-editor/src/widgets/interaction-editor.tsx @@ -62,25 +62,26 @@ class PointEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
Coordinate: \Large( ,{" "} \Large)
@@ -137,44 +138,45 @@ class LineEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
Start: \Large( ,{" "} \Large)
End: \Large( ,{" "} \Large)
@@ -254,23 +256,24 @@ class MovablePointEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
Start: \Large( ,{" "} \Large)
@@ -333,6 +336,7 @@ class MovableLineEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
@@ -340,34 +344,34 @@ class MovableLineEditor extends React.Component {
Start: \Large( ,{" "} \Large)
End: \Large( ,{" "} \Large)
@@ -434,35 +438,36 @@ class FunctionEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
{this.props.funcName + "(x)="}{" "}
Range: \Large( ,{" "} \Large)
@@ -535,45 +540,46 @@ class ParametricEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
X(t) ={" "}
Y(t) ={" "}
Range: \Large( ,{" "} \Large)
@@ -641,6 +647,7 @@ class LabelEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
@@ -657,19 +664,19 @@ class LabelEditor extends React.Component {
Location: \Large( ,{" "} \Large)
@@ -720,46 +727,47 @@ class RectangleEditor extends React.Component { render(): React.ReactNode { const {TeX} = getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; return (
Bottom left: \Large( ,{" "} \Large)
Width:{" "}
Height:{" "}
diff --git a/packages/perseus-editor/src/widgets/interaction/constraint-editor.tsx b/packages/perseus-editor/src/widgets/interaction/constraint-editor.tsx index e93c9c23ca..8841e388df 100644 --- a/packages/perseus-editor/src/widgets/interaction/constraint-editor.tsx +++ b/packages/perseus-editor/src/widgets/interaction/constraint-editor.tsx @@ -45,6 +45,8 @@ class ConstraintEditor extends React.Component { render(): React.ReactNode { const {TeX} = Dependencies.getDependencies(); + const analyticsStub = {onAnalyticsEvent: () => Promise.resolve()}; + return (
@@ -76,10 +78,10 @@ class ConstraintEditor extends React.Component {
x={" "}
@@ -89,10 +91,10 @@ class ConstraintEditor extends React.Component {
y={" "}
@@ -102,17 +104,17 @@ class ConstraintEditor extends React.Component {
x \in \Large[{" "} , {" "} {" "} \Large]
@@ -121,17 +123,17 @@ class ConstraintEditor extends React.Component {
y \in \Large[{" "} , {" "} {" "} \Large]
diff --git a/packages/perseus/src/__tests__/mock-asset-loading-widget.tsx b/packages/perseus/src/__tests__/mock-asset-loading-widget.tsx index b06f2ed830..fe67a1ebb8 100644 --- a/packages/perseus/src/__tests__/mock-asset-loading-widget.tsx +++ b/packages/perseus/src/__tests__/mock-asset-loading-widget.tsx @@ -5,14 +5,13 @@ import AssetContext from "../asset-context"; import type {PerseusItem} from "../perseus-types"; import type {WidgetExports} from "../types"; -// @ts-expect-error - TS2322 - Type '"mocked-asset-widget"' is not assignable to type '"categorizer" | "cs-program" | "definition" | "dropdown" | "example-graphie-widget" | "example-widget" | "explanation" | "expression" | "graded-group-set" | "graded-group" | ... 29 more ... | "video"'. export const mockedAssetItem: PerseusItem = { question: { content: "[[\u2603 mocked-asset-widget 1]]", images: Object.freeze({}), widgets: { "mocked-asset-widget 1": { - type: "mocked-asset-widget", + type: "mocked-asset-widget" as any, alignment: "default", static: false, graded: true, diff --git a/packages/perseus/src/components/__stories__/math-input.stories.tsx b/packages/perseus/src/components/__stories__/math-input.stories.tsx index 57fbe7fb5a..07fc8f27ae 100644 --- a/packages/perseus/src/components/__stories__/math-input.stories.tsx +++ b/packages/perseus/src/components/__stories__/math-input.stories.tsx @@ -13,8 +13,16 @@ export default { } as Story; const defaultObject = { - buttonSets: ["basic"], + keypadButtonSets: { + advancedRelations: true, + basicRelations: true, + divisionKey: true, + logarithms: true, + preAlgebra: true, + trigonometry: true, + }, onChange: () => {}, + analytics: {onAnalyticsEvent: () => Promise.resolve()}, } as const; export const DefaultWithBasicButtonSet = ( @@ -22,9 +30,14 @@ export const DefaultWithBasicButtonSet = ( ): React.ReactElement => { return ; }; -export const AlwaysVisibleButtonSet = (args: StoryArgs): React.ReactElement => { - return ; -}; export const DefaultWithAriaLabel = (args: StoryArgs): React.ReactElement => { return ; }; + +export const KeypadOpenByDefault = (args: StoryArgs): React.ReactElement => { + return ; +}; + +export const KeypadNeverVisible = (args: StoryArgs): React.ReactElement => { + return ; +}; diff --git a/packages/perseus/src/components/__tests__/math-input.test.tsx b/packages/perseus/src/components/__tests__/math-input.test.tsx index b94c143205..f7caa261be 100644 --- a/packages/perseus/src/components/__tests__/math-input.test.tsx +++ b/packages/perseus/src/components/__tests__/math-input.test.tsx @@ -7,6 +7,15 @@ import {testDependencies} from "../../../../../testing/test-dependencies"; import * as Dependencies from "../../dependencies"; import MathInput from "../math-input"; +const allButtonSets = { + advancedRelations: true, + basicRelations: true, + divisionKey: true, + logarithms: true, + preAlgebra: true, + trigonometry: true, +}; + describe("Perseus' MathInput", () => { beforeEach(() => { jest.spyOn(Dependencies, "getDependencies").mockReturnValue( @@ -19,8 +28,9 @@ describe("Perseus' MathInput", () => { render( {}} - buttonSets={["basic"]} + keypadButtonSets={allButtonSets} labelText="test" + analytics={{onAnalyticsEvent: () => Promise.resolve()}} />, ); @@ -31,7 +41,13 @@ describe("Perseus' MathInput", () => { it("is possible to type in the input", () => { // Assemble const mockOnChange = jest.fn(); - render(); + render( + Promise.resolve()}} + />, + ); // Act userEvent.type(screen.getByRole("textbox"), "12345"); @@ -43,18 +59,89 @@ describe("Perseus' MathInput", () => { it("is possible to use buttons", () => { // Assemble const mockOnChange = jest.fn(); - render(); + render( + Promise.resolve()}} + />, + ); // Act - // focusing the input triggers the popover - screen.getByRole("textbox").focus(); - userEvent.type(screen.getByRole("textbox"), "1"); + screen.getByRole("switch").click(); + userEvent.click(screen.getByRole("button", {name: "1"})); userEvent.click(screen.getByRole("button", {name: "Plus"})); - userEvent.type(screen.getByRole("textbox"), "2"); + userEvent.click(screen.getByRole("button", {name: "2"})); userEvent.click(screen.getByRole("button", {name: "Minus"})); - userEvent.type(screen.getByRole("textbox"), "3"); + userEvent.click(screen.getByRole("button", {name: "3"})); // Assert expect(mockOnChange).toHaveBeenLastCalledWith("1+2-3"); }); + + it("is possible to use buttons with legacy props", () => { + // Assemble + const mockOnChange = jest.fn(); + render( + Promise.resolve()}} + />, + ); + + // Act + // focusing the input triggers the popover + screen.getByRole("switch").click(); + userEvent.click(screen.getByRole("button", {name: "1"})); + userEvent.click(screen.getByRole("button", {name: "Plus"})); + userEvent.click(screen.getByRole("button", {name: "2"})); + userEvent.click(screen.getByRole("button", {name: "Divide"})); + userEvent.click(screen.getByRole("button", {name: "3"})); + + // Assert + expect(mockOnChange).toHaveBeenLastCalledWith("1+2\\div3"); + }); + + it("returns focus to input after button click", () => { + // Assemble + render( + {}} + keypadButtonSets={allButtonSets} + analytics={{onAnalyticsEvent: () => Promise.resolve()}} + />, + ); + + // Act + // focusing the input triggers the popover + screen.getByRole("switch").click(); + userEvent.click(screen.getByRole("button", {name: "1"})); + + // Assert + expect(screen.getByRole("textbox")).toHaveFocus(); + }); + + it("does not return focus to input after button press via keyboard", () => { + // Assemble + render( + {}} + keypadButtonSets={allButtonSets} + analytics={{onAnalyticsEvent: () => Promise.resolve()}} + />, + ); + + // Act + // focusing the input triggers the popover + screen.getByRole("switch").click(); + userEvent.tab(); // to "123" tab + userEvent.tab(); // to extra keys tab + userEvent.tab(); // to whole keypad + userEvent.tab(); // to "1" button + userEvent.keyboard("{enter}"); + + // Assert + expect(screen.getByRole("textbox")).not.toHaveFocus(); + }); }); diff --git a/packages/perseus/src/components/math-input.tsx b/packages/perseus/src/components/math-input.tsx index e4feade5be..658522a75c 100644 --- a/packages/perseus/src/components/math-input.tsx +++ b/packages/perseus/src/components/math-input.tsx @@ -1,41 +1,79 @@ +/* eslint-disable @khanacademy/ts-no-error-suppressions */ import { + DesktopKeypad, keyTranslator, createMathField, mathQuillInstance, + CursorContext, + getCursorContext, } from "@khanacademy/math-input"; +import Clickable from "@khanacademy/wonder-blocks-clickable"; +import Color, {fade} from "@khanacademy/wonder-blocks-color"; +import {View} from "@khanacademy/wonder-blocks-core"; +import * as i18n from "@khanacademy/wonder-blocks-i18n"; +import {Popover, PopoverContentCore} from "@khanacademy/wonder-blocks-popover"; +import Spacing from "@khanacademy/wonder-blocks-spacing"; +import {StyleSheet} from "aphrodite"; import classNames from "classnames"; import $ from "jquery"; import * as React from "react"; -import ReactDOM from "react-dom"; import _ from "underscore"; -import TexButtons from "./tex-buttons"; - -import type {ButtonSetsType} from "./tex-buttons"; -import type {MathFieldInterface} from "@khanacademy/math-input"; +import type {LegacyButtonSets} from "../perseus-types"; +import type {PerseusDependenciesV2} from "../types"; +import type {Keys, MathFieldInterface} from "@khanacademy/math-input"; type ButtonsVisibleType = "always" | "never" | "focused"; +type KeypadButtonSets = { + advancedRelations?: boolean; + basicRelations?: boolean; + divisionKey?: boolean; + logarithms?: boolean; + preAlgebra?: boolean; + trigonometry?: boolean; +}; + type Props = { className?: string; value: string; onChange: any; convertDotToTimes: boolean; - buttonsVisible: ButtonsVisibleType; - buttonSets: ButtonSetsType; + /** + * @deprecated Use `keypadButtonSets` instead. Maps to `keypadButtonSets`. + * @see keypadButtonSets + */ + buttonSets?: LegacyButtonSets; + /** + * Overrides deprecated `buttonSets` prop. + */ + keypadButtonSets?: KeypadButtonSets; labelText?: string; onFocus?: () => void; onBlur?: () => void; + hasError?: boolean; + extraKeys?: ReadonlyArray; + /** + * Whether to show the keypad buttons. + * The strings now misleading, but we keep them for backwards compatibility. + * - `focused` means that the keypad **appears on toggle, *off* by default**. + * - `always` means that the keypad **appears on toggle, *on* by default.** + * - `never` means that the keypad is **never shown**. + */ + buttonsVisible?: ButtonsVisibleType; + analytics: PerseusDependenciesV2["analytics"]; }; type DefaultProps = { value: Props["value"]; convertDotToTimes: Props["convertDotToTimes"]; - buttonsVisible: Props["buttonsVisible"]; }; type State = { focused: boolean; + keypadOpen: boolean; + cursorContext: typeof CursorContext[keyof typeof CursorContext]; + openedWithEventType?: string; }; const customKeyTranslator = { @@ -62,81 +100,30 @@ class MathInput extends React.Component { static defaultProps: DefaultProps = { value: "", convertDotToTimes: false, - buttonsVisible: "focused", }; - state: State = {focused: false}; + state: State = { + focused: false, + keypadOpen: this.props.buttonsVisible === "always" ? true : false, + cursorContext: CursorContext.NONE, + }; componentDidMount() { - window.addEventListener("mousedown", this.handleMouseDown); - window.addEventListener("mouseup", this.handleMouseUp); - // Ideally, we would be able to pass an initial value directly into // the constructor this.mathField()?.latex(this.props.value); } - componentWillUnmount() { - window.removeEventListener("mousedown", this.handleMouseDown); - window.removeEventListener("mouseup", this.handleMouseUp); - } - - // handlers: - // keep track of two related bits of state: - // * this.state.focused - whether the buttons are currently shown - // * this.mouseDown - whether a mouse click is active that started in the - // buttons div - - handleFocus: () => void = () => { - this.setState({focused: true}); - // TODO(joel) fix properly - we should probably allow onFocus handlers - // to this property, but we need to work correctly with them. - // if (this.props.onFocus) { - // this.props.onFocus(); - // } - }; - - handleMouseDown: (arg1: MouseEvent) => void = (event) => { - // @ts-expect-error - TS2531 - Object is possibly 'null'. | TS2345 - Argument of type 'EventTarget | null' is not assignable to parameter of type 'Node | null'. - const focused = ReactDOM.findDOMNode(this).contains(event.target); - this.mouseDown = focused; - if (!focused) { - this.setState({focused: false}); - } - }; - - handleMouseUp: (arg1: MouseEvent) => void = () => { - // this mouse click started in the buttons div so we should focus the - // input - if (this.mouseDown) { - this.focus(); - } - this.mouseDown = false; - }; - - handleBlur: (arg1: React.FocusEvent) => void = (e) => { - // TODO(michaelpolyak): Consider trapping focus within the button group. - // Focusing back on the input when TAB out of the last button in the - // group. This will probably require ESCAPE key handling to enable to - // close (blur) the button group in order to focus on next page element. - if ( - !this.mouseDown && - // @ts-expect-error - TS2531 - Object is possibly 'null'. - !ReactDOM.findDOMNode(this).contains(e.relatedTarget) - ) { - this.setState({focused: false}); - } - }; - - _shouldShowButtons: () => boolean = () => { - if (this.props.buttonsVisible === "always") { - return true; - } + openKeypad() { if (this.props.buttonsVisible === "never") { - return false; + return; } - return this.state.focused; - }; + this.setState({keypadOpen: true}); + } + + closeKeypad() { + this.setState({keypadOpen: false}); + } insert: (value: any) => void = (value) => { const input = this.mathField(); @@ -206,6 +193,9 @@ class MathInput extends React.Component { if (this.props.value !== value) { this.props.onChange(value); } + this.setState({ + cursorContext: getCursorContext(mathField), + }); }, enter: () => { // NOTE(kevinb): This isn't how answers to exercises are @@ -239,9 +229,31 @@ class MathInput extends React.Component { this.setState({focused: true}); }; - blur: () => void = () => { - this.mathField()?.blur(); - this.setState({focused: false}); + // removing mathfield focus here makes the cursor vanished when the + // input is still focused + blur: () => void = () => this.setState({focused: false}); + + handleKeypadPress: (key: Keys, e: any) => void = (key, e) => { + const translator = keyTranslator[key]; + const mathField = this.mathField(); + + if (mathField) { + if (translator) { + translator(mathField, key); + } + this.setState({ + cursorContext: getCursorContext(mathField), + }); + } + + // We want to prevent taking focus from input when clicking on keypad + // Clickable handles "onClick" differently than react; + // a keyboard event is "keydown" type. + // In react without WonderBlocks, "enter" or "space" keydown events + // are also "click" events, differentiated by "detail". + if (e.type === "click") { + this.focus(); + } }; render(): React.ReactNode { @@ -258,36 +270,202 @@ class MathInput extends React.Component { className = className + " " + this.props.className; } - let buttons = null; - if (this._shouldShowButtons()) { - // @ts-expect-error - TS2322 - Type 'Element' is not assignable to type 'null'. - buttons = ( - - ); - } - return ( -
-
+ +
{ + const mathField = this.mathField(); + if (!mathField) { + return; + } + this.setState({ + cursorContext: getCursorContext(mathField), + }); + }} + > (this.__mathFieldWrapperRef = ref)} aria-label={this.props.labelText} - onFocus={this.handleFocus} - onBlur={this.handleBlur} + onFocus={() => this.focus()} + onBlur={() => this.blur()} /> + this.closeKeypad()} + dismissEnabled + content={() => ( + + + + )} + > + {this.props.buttonsVisible === "never" ? ( + + ) : ( + + this.state.keypadOpen + ? this.closeKeypad() + : this.openKeypad() + } + > + {(props) => ( + + )} + + )} +
-
- {buttons} -
-
+ ); } } +const MathInputIcon = ({hovered, focused, active}) => { + let color: string | undefined; + switch (true) { + case focused || active: + color = Color.white; + break; + case hovered: + color = Color.blue; + break; + default: + color = Color.offBlack; + break; + } + const dynamicClass = + active || focused ? styles.iconActive : styles.iconInactive; + return ( + + + + + + ); +}; + +const mapButtonSets = (buttonSets?: LegacyButtonSets) => { + const keypadButtonSets: KeypadButtonSets = {}; + if (!buttonSets) { + return keypadButtonSets; + } + buttonSets.forEach((buttonSet) => { + switch (buttonSet) { + case "advanced relations": + keypadButtonSets.advancedRelations = true; + break; + case "basic relations": + keypadButtonSets.basicRelations = true; + break; + case "basic+div": + keypadButtonSets.divisionKey = true; + break; + case "logarithms": + keypadButtonSets.logarithms = true; + break; + case "prealgebra": + keypadButtonSets.preAlgebra = true; + break; + case "trig": + keypadButtonSets.trigonometry = true; + break; + case "basic": + default: + break; + } + }); + return keypadButtonSets; +}; + +const inputFocused = { + borderWidth: 2, + borderColor: Color.blue, + margin: -1, +}; + +const styles = StyleSheet.create({ + iconContainer: { + display: "flex", + justifyContent: "center", + height: "100%", + padding: Spacing.xxxSmall_4, + borderRadius: 1, + }, + iconInactive: { + backgroundColor: Color.offBlack8, + }, + iconActive: { + backgroundColor: Color.offBlack64, + }, + outerWrapper: { + display: "inline-block", + borderStyle: "solid", + borderWidth: 1, + borderColor: Color.offBlack50, + borderRadius: 3, + background: Color.white, + ":hover": inputFocused, + }, + wrapperFocused: inputFocused, + wrapperError: { + borderColor: Color.red, + background: fade(Color.red, 0.08), + ":hover": { + borderColor: Color.red, + }, + }, + popoverContent: { + padding: 0, + paddingBottom: Spacing.xxSmall_6, + maxWidth: "initial", + }, +}); + export default MathInput; diff --git a/packages/perseus/src/components/tex-buttons.tsx b/packages/perseus/src/components/tex-buttons.tsx index 2b16457246..0e0e0f65f7 100644 --- a/packages/perseus/src/components/tex-buttons.tsx +++ b/packages/perseus/src/components/tex-buttons.tsx @@ -6,6 +6,7 @@ import _ from "underscore"; import {getDependencies} from "../dependencies"; +import type {LegacyButtonSets} from "../perseus-types"; import type {Keys} from "@khanacademy/math-input"; const prettyBig = {fontSize: "150%"} as const; @@ -22,7 +23,7 @@ type ButtonSet = (props: any) => [ ]; type ButtonSets = { - readonly [key: string]: ReadonlyArray; + readonly [key in LegacyButtonSets[number]]: ReadonlyArray; }; // These are functions because we want to generate a new component for each use @@ -285,7 +286,6 @@ class TexButtons extends React.Component { // for _.keys() to return the keys in an arbitrary order, but in // practice, they will be ordered as listed above. const sortedButtonSets = _.sortBy(this.props.sets, (setName) => - // @ts-expect-error - TS2345 - Argument of type 'string | number' is not assignable to parameter of type 'string'. _.keys(buttonSets).indexOf(setName), ); diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 8465a7e04d..80262a985c 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -311,11 +311,21 @@ export type PerseusExplanationWidgetOptions = { static: boolean; }; +export type LegacyButtonSets = ReadonlyArray< + | "basic" + | "basic+div" + | "trig" + | "prealgebra" + | "logarithms" + | "basic relations" + | "advanced relations" +>; + export type PerseusExpressionWidgetOptions = { // The expression forms the answer may come in answerForms: ReadonlyArray; // Different buttons sets that can show in the expression. Options are "basic", "basic+div", "trig", "prealgebra", "logarithms", "basic relations", "advanced relations" - buttonSets: ReadonlyArray; + buttonSets: LegacyButtonSets; // Variables that can be used as functions. Default: ["f", "g", "h"] functions: ReadonlyArray; // Use x for rendering multiplication instead of a center dot. diff --git a/packages/perseus/src/styles/perseus-renderer.less b/packages/perseus/src/styles/perseus-renderer.less index d32a5e1731..53c41bf0b0 100644 --- a/packages/perseus/src/styles/perseus-renderer.less +++ b/packages/perseus/src/styles/perseus-renderer.less @@ -598,8 +598,7 @@ } .perseus-math-input.mq-editable-field.mq-math-mode { - // Mimic native text input - background: #fff; + background-color: transparent; // If the font-size is too small, super/subscripts become hard to input font-size: 18px; @@ -607,11 +606,11 @@ // Make the click target easier to hit min-width: 100px; - // Mimic native text input w/o using -moz/-webkit-apperance: textfield - // border-color: rgb(78, 187, 212); - border-color: #a4a4a4; + border: unset; - border-radius: 5px; + &.mq-focused { + box-shadow: unset; + } & > .mq-root-block { // A bit of extra whitespace here greatly improves legibility diff --git a/packages/perseus/src/styles/widgets/expression.less b/packages/perseus/src/styles/widgets/expression.less index 646cc320b2..76088ddbcd 100644 --- a/packages/perseus/src/styles/widgets/expression.less +++ b/packages/perseus/src/styles/widgets/expression.less @@ -26,12 +26,6 @@ width: 210px; } - &.show-error-tooltip - .perseus-math-input.mq-editable-field.mq-math-mode - > .mq-root-block { - padding-right: 25px; - } - .perseus-formats-tooltip { width: 190px; } diff --git a/packages/perseus/src/widgets/__stories__/expression.stories.tsx b/packages/perseus/src/widgets/__stories__/expression.stories.tsx index 6e02492a3e..1c71ec5ae6 100644 --- a/packages/perseus/src/widgets/__stories__/expression.stories.tsx +++ b/packages/perseus/src/widgets/__stories__/expression.stories.tsx @@ -10,7 +10,7 @@ import expressionExport from "../expression"; import TestKeypadContextWrapper from "./test-keypad-context-wrapper"; -import type {PerseusItem} from "../../perseus-types"; +import type {LegacyButtonSets, PerseusItem} from "../../perseus-types"; import type {Keys as Key} from "@khanacademy/math-input"; type StoryArgs = { @@ -69,12 +69,12 @@ export const DesktopKitchenSink = (args: StoryArgs): React.ReactElement => { "logarithms", "basic relations", "advanced relations", - ], + ] as LegacyButtonSets, }; const keypadConfiguration = { keypadType: KeypadType.EXPRESSION, - extraKeys: ["x", "y", "z"] as Key[], + extraKeys: ["x", "y", "z"] as Array, }; return ( diff --git a/packages/perseus/src/widgets/__tests__/expression.test.tsx b/packages/perseus/src/widgets/__tests__/expression.test.tsx index ab230ee478..d8d1dfb554 100644 --- a/packages/perseus/src/widgets/__tests__/expression.test.tsx +++ b/packages/perseus/src/widgets/__tests__/expression.test.tsx @@ -1,5 +1,5 @@ import {it, describe, beforeEach} from "@jest/globals"; -import {screen, fireEvent} from "@testing-library/react"; +import {screen} from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; @@ -428,93 +428,20 @@ describe("error tooltip", () => { ); }); - it("shows on error", () => { + it("shows error text in tooltip", async () => { // Arrange const {renderer} = renderQuestion(expressionItem2.question); const expression = renderer.findWidgets("expression 1")[0]; // Act expression.insert("x&&&&&^1"); + screen.getByRole("textbox").blur(); renderer.guessAndScore(); - jest.runOnlyPendingTimers(); // Assert - const errorMessage = screen.getByText( - "Sorry, I don't understand that!", - ); - expect(errorMessage).not.toBeUndefined(); - }); - - it("shows error text on mouse over", async () => { - // Arrange - const {renderer} = renderQuestion(expressionItem2.question); - const expression = renderer.findWidgets("expression 1")[0]; - - // Act - expression.insert("x&&&&&^1"); - renderer.guessAndScore(); - jest.runOnlyPendingTimers(); - - const tooltip = await screen.findByTestId("test-error-icon"); - - // eslint-disable-next-line testing-library/prefer-user-event - fireEvent.mouseEnter(tooltip); - - // Assert - expect( - screen.getByText("Sorry, I don't understand that!"), - ).toBeVisible(); - }); - it("hides error text on mouse leave", async () => { - // Arrange - const {renderer} = renderQuestion(expressionItem2.question); - const expression = renderer.findWidgets("expression 1")[0]; - - expression.insert("x&&&&&^1"); - renderer.guessAndScore(); - jest.runOnlyPendingTimers(); - - const tooltip = await screen.findByTestId("test-error-icon"); - - // Act - // NOTE(Nicole): The existing perseus code requires explicit events - // eslint-disable-next-line testing-library/prefer-user-event - fireEvent.mouseEnter(tooltip); - // NOTE(Nicole): The existing perseus code requires explicit events - // eslint-disable-next-line testing-library/prefer-user-event - fireEvent.mouseLeave(tooltip); - - // Assert - expect( - screen.getByText("Sorry, I don't understand that!"), - ).not.toBeVisible(); - }); - - it("toggles error text on click", async () => { - // Arrange - const {renderer} = renderQuestion(expressionItem2.question); - const expression = renderer.findWidgets("expression 1")[0]; - - expression.insert("x&&&&&^1"); - renderer.guessAndScore(); - jest.runOnlyPendingTimers(); - - const tooltip = await screen.findByTestId("test-error-icon"); - - // Act & Assert - // NOTE(Nicole): The existing perseus code requires explicit events - // eslint-disable-next-line testing-library/prefer-user-event - fireEvent.click(tooltip); + expect(screen.getByText("Oops!")).toBeVisible(); expect( screen.getByText("Sorry, I don't understand that!"), ).toBeVisible(); - - // Act & Assert - // NOTE(Nicole): The existing perseus code requires explicit events - // eslint-disable-next-line testing-library/prefer-user-event - fireEvent.click(tooltip); - expect( - screen.getByText("Sorry, I don't understand that!"), - ).not.toBeVisible(); }); }); diff --git a/packages/perseus/src/widgets/expression.tsx b/packages/perseus/src/widgets/expression.tsx index acc8162425..3066070208 100644 --- a/packages/perseus/src/widgets/expression.tsx +++ b/packages/perseus/src/widgets/expression.tsx @@ -1,24 +1,22 @@ import * as KAS from "@khanacademy/kas"; -import {KeypadInput, KeypadType} from "@khanacademy/math-input"; +import {KeyArray, KeypadInput, KeypadType} from "@khanacademy/math-input"; import {linterContextDefault} from "@khanacademy/perseus-linter"; +import {View} from "@khanacademy/wonder-blocks-core"; import * as i18n from "@khanacademy/wonder-blocks-i18n"; +import Tooltip from "@khanacademy/wonder-blocks-tooltip"; import classNames from "classnames"; import * as React from "react"; import _ from "underscore"; -import InlineIcon from "../components/inline-icon"; import MathInput from "../components/math-input"; -import Tooltip, { - HorizontalDirection, - VerticalDirection, -} from "../components/tooltip"; import {useDependencies} from "../dependencies"; -import {iconExclamationSign} from "../icon-paths"; import {Errors as PerseusErrors, Log} from "../logging/log"; import * as Changeable from "../mixins/changeable"; import {ApiOptions, ClassNames as ApiClassNames} from "../perseus-api"; +import a11y from "../util/a11y"; import KhanAnswerTypes from "../util/answer-types"; +import type {DependenciesContext} from "../dependencies"; import type { PerseusExpressionWidgetOptions, PerseusExpressionAnswerForm, @@ -34,6 +32,7 @@ import type {PropsFor} from "@khanacademy/wonder-blocks-core"; type InputPath = ReadonlyArray; +const ERROR_TITLE = i18n._("Oops!"); const ERROR_MESSAGE = i18n._("Sorry, I don't understand that!"); const insertBraces = (value) => { @@ -85,7 +84,7 @@ type RenderProps = { type ExternalProps = WidgetProps; export type Props = ExternalProps & - ReturnType & { + Partial> & { apiOptions: NonNullable; buttonSets: NonNullable; functions: NonNullable; @@ -97,8 +96,9 @@ export type Props = ExternalProps & }; export type ExpressionState = { + invalid: boolean; showErrorTooltip: boolean; - showErrorText: boolean; + showErrorStyle: boolean; }; type DefaultProps = { @@ -121,7 +121,6 @@ type OnInputErrorFunctionType = ( // The new, MathQuill input expression widget export class Expression extends React.Component { _isMounted = false; - errorTimeout: null | number = null; //#region Previously a class extension /* Content creators input a list of answers which are matched from top to @@ -315,11 +314,14 @@ export class Expression extends React.Component { displayName = "Expression"; state: ExpressionState = { + invalid: false, showErrorTooltip: false, - showErrorText: false, + showErrorStyle: false, }; componentDidMount: () => void = () => { + document.addEventListener("mousedown", this._handleMouseDown); + // TODO(scottgrant): This is a hack to remove the deprecated call to // this.isMounted() but is still considered an anti-pattern. this._isMounted = true; @@ -334,42 +336,38 @@ export class Expression extends React.Component { !_.isEqual(this.props.value, prevProps.value) || !_.isEqual(this.props.functions, prevProps.functions) ) { - // TODO(jeff, CP-3128): Use Wonder Blocks Timing API. - // eslint-disable-next-line no-restricted-syntax - // @ts-expect-error - TS2769 - No overload matches this call. - clearTimeout(this.errorTimeout); - - if (this.parse(this.props.value, this.props).parsed) { - // eslint-disable-next-line react/no-did-update-set-state - this.setState({showErrorTooltip: false}); - } else { - // Store timeout ID so that we can clear it above - // TODO(jeff, CP-3128): Use Wonder Blocks Timing API. - // eslint-disable-next-line no-restricted-syntax - // @ts-expect-error - TS2322 - Type 'Timeout' is not assignable to type 'number'. - this.errorTimeout = setTimeout(() => { - const apiResult = this.props.apiOptions.onInputError( - null, // reserved for some widget identifier - this.props.value, - ERROR_MESSAGE, - ); - if (apiResult !== false) { - this.setState({showErrorTooltip: true}); - } - }, 2000); + this.setState({ + invalid: false, + showErrorTooltip: false, + showErrorStyle: false, + }); + if (!this.parse(this.props.value, this.props).parsed) { + const apiResult = this.props.apiOptions.onInputError( + null, // reserved for some widget identifier + this.props.value, + ERROR_MESSAGE, + ); + if (apiResult !== false) { + this.setState({ + invalid: true, + }); + } } } }; componentWillUnmount: () => void = () => { - // TODO(jeff, CP-3128): Use Wonder Blocks Timing API. - // eslint-disable-next-line no-restricted-syntax - // @ts-expect-error - TS2769 - No overload matches this call. - clearTimeout(this.errorTimeout); - this._isMounted = false; }; + _handleMouseDown = () => { + if (this._isMounted && this.state.showErrorTooltip) { + this.setState({ + showErrorTooltip: false, + }); + } + }; + simpleValidate: ( rubric: Rubric, onInputError: OnInputErrorFunctionType, @@ -422,7 +420,7 @@ export class Expression extends React.Component { }; _handleFocus: () => void = () => { - this.props.analytics.onAnalyticsEvent({ + this.props.analytics?.onAnalyticsEvent({ type: "perseus:expression-focused", payload: null, }); @@ -497,7 +495,7 @@ export class Expression extends React.Component { }; sendExpressionEvaluatedEvent(result: "correct" | "incorrect" | "invalid") { - this.props.analytics.onAnalyticsEvent({ + this.props.analytics?.onAnalyticsEvent({ type: "perseus:expression-evaluated", payload: { result, @@ -536,41 +534,6 @@ export class Expression extends React.Component { /> ); } - // TODO(alex): Style this tooltip to be more consistent with other - // tooltips on the site; align to left middle (once possible) - const errorTooltip = ( - - - { - this.setState({showErrorText: true}); - }} - onMouseLeave={() => { - this.setState({showErrorText: false}); - }} - onClick={() => { - // TODO(alex): Better error feedback for mobile - this.setState({ - showErrorText: !this.state.showErrorText, - }); - }} - > - - -
{ERROR_MESSAGE}
- - - ); const className = classNames({ "perseus-widget-expression": true, @@ -578,20 +541,54 @@ export class Expression extends React.Component { }); return ( -
- - {this.state.showErrorTooltip && errorTooltip} +
+ this.state.invalid && + this.setState({ + showErrorTooltip: true, + showErrorStyle: true, + }) + } + onFocus={() => + this.setState({ + showErrorTooltip: false, + }) + } + > + {/** + * This is a visually hidden container for the error tooltip. + https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/alert_role#example_3_visually_hidden_alert_container_for_screen_reader_notifications + */} + + {this.state.showErrorTooltip && + ERROR_TITLE + " " + ERROR_MESSAGE} + + + {}, + } + } + /> +
); } @@ -616,8 +613,8 @@ const keypadConfigurationForProps = ( const keypadType = KeypadType.EXPRESSION; // Extract any and all variables and constants from the answer forms. - const uniqueExtraVariables: Record = {}; - const uniqueExtraConstants: Record = {}; + const uniqueExtraVariables: Partial> = {}; + const uniqueExtraConstants: Partial> = {}; for (const answerForm of widgetOptions.answerForms) { const maybeExpr = KAS.parse(answerForm.value, widgetOptions); if (maybeExpr.parsed) { @@ -630,12 +627,20 @@ const keypadConfigurationForProps = ( symbol === "pi" || symbol === "theta"; const toKey = (symbol: any) => isGreek(symbol) ? symbol.toUpperCase() : symbol; + const isKey = (key: string): key is Key => + KeyArray.includes(key as Key); for (const variable of expr.getVars()) { - uniqueExtraVariables[toKey(variable)] = true; + const maybeKey = toKey(variable); + if (isKey(maybeKey)) { + uniqueExtraVariables[maybeKey] = true; + } } for (const constant of expr.getConsts()) { - uniqueExtraConstants[toKey(constant)] = true; + const maybeKey = toKey(constant); + if (isKey(maybeKey)) { + uniqueExtraConstants[maybeKey] = true; + } } } }