From 26f07035cbe97422e2296dadde1e4558d77f773e Mon Sep 17 00:00:00 2001 From: Ned Redmond Date: Thu, 17 Aug 2023 16:44:04 -0400 Subject: [PATCH 1/2] Revert "Roll v2 Keypad Onto Desktop Web (#645)" This reverts commit e036092e88f7b991fb3991b73abf53a1bf4a3a8f. --- .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, 318 insertions(+), 431 deletions(-) delete mode 100644 .changeset/great-pigs-deny.md diff --git a/.changeset/great-pigs-deny.md b/.changeset/great-pigs-deny.md deleted file mode 100644 index baeb2fbb51..0000000000 --- a/.changeset/great-pigs-deny.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -"@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 9ea339af3f..2ca0feeee8 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: {form: any; simplify: any; value: any; key: number}) => { + .map((obj, index) => { 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(obj.key, props), + onChange: (props) => this.updateForm(index, props), trackInteraction: () => {}, - widgetId: this.props.widgetId + "-" + obj.key, + widgetId: this.props.widgetId + "-" + index, } as const; return lens(obj) .merge([], { draggable: true, - onChange: (props) => this.updateForm(obj.key, props), - onDelete: () => this.handleRemoveForm(obj.key), + onChange: (props) => this.updateForm(index, props), + onDelete: () => this.handleRemoveForm(index), expressionProps: expressionProps, }) .freeze(); }) - .map((obj) => ); + .map((obj, index) => ); 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; @@ -29,14 +22,9 @@ export const DefaultWithBasicButtonSet = ( ): React.ReactElement => { return ; }; -export const DefaultWithAriaLabel = (args: StoryArgs): React.ReactElement => { - return ; -}; - -export const KeypadOpenByDefault = (args: StoryArgs): React.ReactElement => { +export const AlwaysVisibleButtonSet = (args: StoryArgs): React.ReactElement => { return ; }; - -export const KeypadNeverVisible = (args: StoryArgs): React.ReactElement => { - return ; +export const DefaultWithAriaLabel = (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 6886a34650..b94c143205 100644 --- a/packages/perseus/src/components/__tests__/math-input.test.tsx +++ b/packages/perseus/src/components/__tests__/math-input.test.tsx @@ -7,15 +7,6 @@ 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( @@ -28,7 +19,7 @@ describe("Perseus' MathInput", () => { render( {}} - keypadButtonSets={allButtonSets} + buttonSets={["basic"]} labelText="test" />, ); @@ -40,12 +31,7 @@ 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"); @@ -57,43 +43,18 @@ 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("switch").click(); - userEvent.click(screen.getByRole("button", {name: "1"})); + screen.getByRole("textbox").focus(); + userEvent.type(screen.getByRole("textbox"), "1"); userEvent.click(screen.getByRole("button", {name: "Plus"})); - userEvent.click(screen.getByRole("button", {name: "2"})); + userEvent.type(screen.getByRole("textbox"), "2"); userEvent.click(screen.getByRole("button", {name: "Minus"})); - userEvent.click(screen.getByRole("button", {name: "3"})); + userEvent.type(screen.getByRole("textbox"), "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 c5976415da..4002ec07b3 100644 --- a/packages/perseus/src/components/math-input.tsx +++ b/packages/perseus/src/components/math-input.tsx @@ -1,75 +1,41 @@ -/* 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 type {Keys, MathFieldInterface} from "@khanacademy/math-input"; +import TexButtons from "./tex-buttons"; -type ButtonsVisibleType = "always" | "never" | "focused"; +import type {ButtonSetsType} from "./tex-buttons"; +import type {MathFieldInterface} from "@khanacademy/math-input"; -type KeypadButtonSets = { - advancedRelations?: boolean; - basicRelations?: boolean; - divisionKey?: boolean; - logarithms?: boolean; - preAlgebra?: boolean; - trigonometry?: boolean; -}; +type ButtonsVisibleType = "always" | "never" | "focused"; type Props = { className?: string; value: string; onChange: any; convertDotToTimes: boolean; - /** - * @deprecated Use `keypadButtonSets` instead. Maps to `keypadButtonSets`. - * @see keypadButtonSets - */ - buttonSets?: LegacyButtonSets; - /** - * Overrides deprecated `buttonSets` prop. - */ - keypadButtonSets?: KeypadButtonSets; + buttonsVisible: ButtonsVisibleType; + buttonSets: ButtonSetsType; 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 = { @@ -96,28 +62,81 @@ class MathInput extends React.Component { static defaultProps: DefaultProps = { value: "", convertDotToTimes: false, + buttonsVisible: "focused", }; - state: State = { - focused: false, - keypadOpen: this.props.buttonsVisible === "always" ? true : false, - cursorContext: CursorContext.NONE, - }; + state: State = {focused: false}; 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); } - openKeypad: () => void = () => { - if (this.props.buttonsVisible === "never") { - return; + 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.setState({keypadOpen: true}); + this.mouseDown = false; }; - closeKeypad: () => void = () => this.setState({keypadOpen: 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; + } + if (this.props.buttonsVisible === "never") { + return false; + } + return this.state.focused; + }; insert: (value: any) => void = (value) => { const input = this.mathField(); @@ -187,9 +206,6 @@ 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 @@ -223,23 +239,9 @@ class MathInput extends React.Component { this.setState({focused: true}); }; - // 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 - } + blur: () => void = () => { + this.mathField()?.blur(); + this.setState({focused: false}); }; render(): React.ReactNode { @@ -256,211 +258,36 @@ 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.focus()} - onBlur={() => this.blur()} + onFocus={this.handleFocus} + onBlur={this.handleBlur} /> - 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 f7b99ce7fa..b1ba62cd73 100644 --- a/packages/perseus/src/styles/perseus-renderer.less +++ b/packages/perseus/src/styles/perseus-renderer.less @@ -599,7 +599,8 @@ } .perseus-math-input.mq-editable-field.mq-math-mode { - background-color: transparent; + // Mimic native text input + background: #fff; // If the font-size is too small, super/subscripts become hard to input font-size: 18px; @@ -607,11 +608,11 @@ // Make the click target easier to hit min-width: 100px; - border: unset; + // Mimic native text input w/o using -moz/-webkit-apperance: textfield + // border-color: rgb(78, 187, 212); + border-color: #a4a4a4; - &.mq-focused { - box-shadow: unset; - } + border-radius: 5px; & > .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 76088ddbcd..646cc320b2 100644 --- a/packages/perseus/src/styles/widgets/expression.less +++ b/packages/perseus/src/styles/widgets/expression.less @@ -26,6 +26,12 @@ 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 dd6047255e..e3ba676119 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} from "@testing-library/react"; +import {screen, fireEvent} from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import "@testing-library/jest-dom"; @@ -363,20 +363,93 @@ describe("error tooltip", () => { ); }); - it("shows error text in tooltip", async () => { + it("shows on error", () => { // 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 - expect(screen.getByText("Oops!")).toBeVisible(); + 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("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 0ee8994450..772bb6692d 100644 --- a/packages/perseus/src/widgets/expression.tsx +++ b/packages/perseus/src/widgets/expression.tsx @@ -1,19 +1,22 @@ 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 { @@ -36,7 +39,6 @@ const sendExpressionEvaluatedEvent = ( type InputPath = ReadonlyArray; -const ERROR_TITLE = i18n._("Oops!"); const ERROR_MESSAGE = i18n._("Sorry, I don't understand that!"); const insertBraces = (value) => { @@ -77,9 +79,8 @@ export type Props = ExternalProps & { }; export type ExpressionState = { - invalid: boolean; showErrorTooltip: boolean; - showErrorStyle: boolean; + showErrorText: boolean; }; type DefaultProps = { @@ -102,6 +103,7 @@ 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 @@ -307,14 +309,11 @@ export class Expression extends React.Component { displayName = "Expression"; state: ExpressionState = { - invalid: false, showErrorTooltip: false, - showErrorStyle: false, + showErrorText: 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; @@ -329,36 +328,40 @@ export class Expression extends React.Component { !_.isEqual(this.props.value, prevProps.value) || !_.isEqual(this.props.functions, prevProps.functions) ) { - 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, - }); - } + // 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); } } }; componentWillUnmount: () => void = () => { - this._isMounted = false; - }; + // 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); - _handleMouseDown = () => { - if (this._isMounted && this.state.showErrorTooltip) { - this.setState({ - showErrorTooltip: false, - }); - } + this._isMounted = false; }; simpleValidate: ( @@ -496,6 +499,41 @@ 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, @@ -503,49 +541,20 @@ export class Expression extends React.Component { }); return ( -
- 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} - - - - +
+ + {this.state.showErrorTooltip && errorTooltip}
); } From 8a52f64295ad654165b46d44c0babecdfbcbf5dd Mon Sep 17 00:00:00 2001 From: Ned Redmond Date: Thu, 17 Aug 2023 16:48:48 -0400 Subject: [PATCH 2/2] Create five-meals-wink.md --- .changeset/five-meals-wink.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changeset/five-meals-wink.md diff --git a/.changeset/five-meals-wink.md b/.changeset/five-meals-wink.md new file mode 100644 index 0000000000..a845151cc8 --- /dev/null +++ b/.changeset/five-meals-wink.md @@ -0,0 +1,2 @@ +--- +---