From e036092e88f7b991fb3991b73abf53a1bf4a3a8f Mon Sep 17 00:00:00 2001 From: Ned Redmond Date: Tue, 15 Aug 2023 19:50:28 -0400 Subject: [PATCH] Roll v2 Keypad Onto Desktop Web (#645) 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. - ~~There is no longer a `buttonsVisible` prop, since visibility is managed by the toggle. I believe this disabled the buttons in some cases. @jeremywiebe do we still need that ability?~~ - This has been restored. The three states are: - `focused` default behavior, toggle off to start - `always` default behavior, toggle on to start - `never` toggle button disabled - ~~`buttonSets` went from "any" that seemed to take an array to an object with fields that reflect the props on Keypad that toggle the presence of tabs.~~ - `buttonSets` type was restored but marked deprecated in jsDocs - `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, jeremywiebe Checks: ✅ finish_coverage, ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald, ✅ Check for .changeset file (ubuntu-latest, 16.x) Pull Request URL: https://github.com/Khan/perseus/pull/645 --- .changeset/great-pigs-deny.md | 9 + .../src/widgets/expression-editor.tsx | 12 +- .../src/widgets/interaction-editor.tsx | 25 -- .../widgets/interaction/constraint-editor.tsx | 6 - .../__stories__/math-input.stories.tsx | 20 +- .../components/__tests__/math-input.test.tsx | 53 ++- .../perseus/src/components/math-input.tsx | 363 +++++++++++++----- .../perseus/src/styles/perseus-renderer.less | 11 +- .../src/styles/widgets/expression.less | 6 - .../src/widgets/__tests__/expression.test.ts | 81 +--- packages/perseus/src/widgets/expression.tsx | 163 ++++---- 11 files changed, 431 insertions(+), 318 deletions(-) create mode 100644 .changeset/great-pigs-deny.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/packages/perseus-editor/src/widgets/expression-editor.tsx b/packages/perseus-editor/src/widgets/expression-editor.tsx index 2ca0feeee8..9ea339af3f 100644 --- a/packages/perseus-editor/src/widgets/expression-editor.tsx +++ b/packages/perseus-editor/src/widgets/expression-editor.tsx @@ -98,7 +98,7 @@ class ExpressionEditor extends React.Component { render(): React.ReactNode { const answerOptions = this.props.answerForms - .map((obj, index) => { + .map((obj: {form: any; simplify: any; value: any; key: number}) => { const expressionProps = { // note we're using // *this.props*.{times,functions,buttonSets} since each @@ -112,22 +112,22 @@ class ExpressionEditor extends React.Component { simplify: obj.simplify, value: obj.value, - onChange: (props) => this.updateForm(index, props), + onChange: (props) => this.updateForm(obj.key, props), trackInteraction: () => {}, - widgetId: this.props.widgetId + "-" + index, + widgetId: this.props.widgetId + "-" + obj.key, } as const; return lens(obj) .merge([], { draggable: true, - onChange: (props) => this.updateForm(index, props), - onDelete: () => this.handleRemoveForm(index), + onChange: (props) => this.updateForm(obj.key, props), + onDelete: () => this.handleRemoveForm(obj.key), expressionProps: expressionProps, }) .freeze(); }) - .map((obj, index) => ); + .map((obj) => ); const sortable = ( {
Coordinate: \Large( { /> ,{" "} {
Start: \Large( { /> ,{" "} {
End: \Large( { /> ,{" "} {
Start: \Large( ,{" "} {
Start: \Large( ,{" "} {
End: \Large( ,{" "} {
{this.props.funcName + "(x)="}{" "} {
Range: \Large( { /> ,{" "} {
X(t) ={" "} {
Y(t) ={" "} {
Range: \Large( { /> ,{" "} {
Location: \Large( { /> ,{" "} {
Bottom left: \Large( { /> ,{" "} {
Width:{" "} {
Height:{" "} {
x={" "} {
y={" "} {
x \in \Large[{" "} , {" "} {
y \in \Large[{" "} , {" "} {}, } as const; @@ -22,9 +29,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..6886a34650 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,7 +28,7 @@ describe("Perseus' MathInput", () => { render( {}} - buttonSets={["basic"]} + keypadButtonSets={allButtonSets} labelText="test" />, ); @@ -31,7 +40,12 @@ describe("Perseus' MathInput", () => { it("is possible to type in the input", () => { // Assemble const mockOnChange = jest.fn(); - render(); + render( + , + ); // Act userEvent.type(screen.getByRole("textbox"), "12345"); @@ -43,18 +57,43 @@ describe("Perseus' MathInput", () => { it("is possible to use buttons", () => { // Assemble const mockOnChange = jest.fn(); - render(); + render( + , + ); // 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( + , + ); + + // 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"); + }); }); diff --git a/packages/perseus/src/components/math-input.tsx b/packages/perseus/src/components/math-input.tsx index 4002ec07b3..c5976415da 100644 --- a/packages/perseus/src/components/math-input.tsx +++ b/packages/perseus/src/components/math-input.tsx @@ -1,41 +1,75 @@ +/* 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 {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; }; type DefaultProps = { value: Props["value"]; convertDotToTimes: Props["convertDotToTimes"]; - buttonsVisible: Props["buttonsVisible"]; }; type State = { focused: boolean; + keypadOpen: boolean; + cursorContext: typeof CursorContext[keyof typeof CursorContext]; }; const customKeyTranslator = { @@ -62,82 +96,29 @@ 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 [FEI-5003] - 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 [FEI-5003] - 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: () => void = () => { if (this.props.buttonsVisible === "never") { - return false; + return; } - return this.state.focused; + this.setState({keypadOpen: true}); }; + closeKeypad: () => void = () => this.setState({keypadOpen: false}); + insert: (value: any) => void = (value) => { const input = this.mathField(); const inputModifier = customKeyTranslator[value]; @@ -206,6 +187,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 +223,23 @@ 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) => void = (key) => { + const translator = keyTranslator[key]; + const mathField = this.mathField(); + + if (mathField) { + if (translator) { + translator(mathField, key); + } + this.setState({ + cursorContext: getCursorContext(mathField), + }); + mathField.focus(); // to see cursor position after button press + } }; render(): React.ReactNode { @@ -258,36 +256,211 @@ class MathInput extends React.Component { className = className + " " + this.props.className; } - let buttons = null; - if (this._shouldShowButtons()) { - // @ts-expect-error [FEI-5003] - 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()} + content={() => ( + + {}} + extraKeys={this.props.extraKeys} + onClickKey={this.handleKeypadPress} + cursorContext={this.state.cursorContext} + multiplicationDot={ + !this.props.convertDotToTimes + } + {...(this.props.keypadButtonSets ?? + mapButtonSets(this.props?.buttonSets))} + /> + + )} + > + {this.props.buttonsVisible === "never" ? ( + + ) : ( + + this.state.keypadOpen + ? this.closeKeypad() + : this.openKeypad() + } + > + {(props) => ( + + )} + + )} +
-
- {buttons} -
-
+ ); } } +const MathInputIcon = ({hovered, focused, active}) => { + // const color = hovered ? Color.blue : Color.offBlack; + 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 ( + + + + + + ); +}; + +export type LegacyButtonSets = ReadonlyArray< + | "basic" + | "basic+div" + | "trig" + | "prealgebra" + | "logarithms" + | "basic relations" + | "advanced relations" +>; + +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/styles/perseus-renderer.less b/packages/perseus/src/styles/perseus-renderer.less index b1ba62cd73..f7b99ce7fa 100644 --- a/packages/perseus/src/styles/perseus-renderer.less +++ b/packages/perseus/src/styles/perseus-renderer.less @@ -599,8 +599,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; @@ -608,11 +607,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/__tests__/expression.test.ts b/packages/perseus/src/widgets/__tests__/expression.test.ts index e3ba676119..dd6047255e 100644 --- a/packages/perseus/src/widgets/__tests__/expression.test.ts +++ b/packages/perseus/src/widgets/__tests__/expression.test.ts @@ -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"; @@ -363,93 +363,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 772bb6692d..0ee8994450 100644 --- a/packages/perseus/src/widgets/expression.tsx +++ b/packages/perseus/src/widgets/expression.tsx @@ -1,22 +1,19 @@ import * as KAS from "@khanacademy/kas"; import {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 {getDependencies} 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 { @@ -39,6 +36,7 @@ const sendExpressionEvaluatedEvent = ( type InputPath = ReadonlyArray; +const ERROR_TITLE = i18n._("Oops!"); const ERROR_MESSAGE = i18n._("Sorry, I don't understand that!"); const insertBraces = (value) => { @@ -79,8 +77,9 @@ export type Props = ExternalProps & { }; export type ExpressionState = { + invalid: boolean; showErrorTooltip: boolean; - showErrorText: boolean; + showErrorStyle: boolean; }; type DefaultProps = { @@ -103,7 +102,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 @@ -309,11 +307,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; @@ -328,42 +329,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 [FEI-5003] - 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 [FEI-5003] - 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 [FEI-5003] - 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, @@ -499,41 +496,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, @@ -541,20 +503,49 @@ 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} + + + +
); }