From 284e068b8e3bfb1f9ab49d84d209c5f9ef2d93c1 Mon Sep 17 00:00:00 2001 From: Matthew Date: Wed, 31 Jul 2024 08:50:41 -0500 Subject: [PATCH] Support visible/aria label for the Expression widget (#1404) * add inputs for labels to the expression editor * add aria label to mobile input * fix tests * changeset * rough out idea for CSS * flow elements better * add comment to hack * change name of prop and tweak tests * remove findDOMNode from render * consolidate mobile vs desktop logic * add more tests * make labels serializable * ignore errors in story * try again with vertical-align * use WB labels * move styles to aphrodite * add comments * i18n aria label default * add WCAG link to tooltip thing --- .changeset/stupid-coats-dance.md | 7 + .../components/__tests__/integration.test.tsx | 1 + .../input/__tests__/context-tracking.test.ts | 2 +- .../input/__tests__/mathquill.test.ts | 2 +- .../src/components/input/math-input.tsx | 2 + .../src/components/input/math-wrapper.ts | 28 +- .../src/full-mobile-input.stories.tsx | 1 + .../__stories__/article-editor.stories.tsx | 41 ++ .../src/widgets/expression-editor.tsx | 46 +- .../__stories__/math-input.stories.tsx | 3 +- .../components/__tests__/math-input.test.tsx | 55 ++- .../perseus/src/components/math-input.tsx | 8 +- packages/perseus/src/perseus-types.ts | 4 + packages/perseus/src/strings.ts | 3 + .../perseus/src/styles/perseus-renderer.less | 3 + .../__stories__/expression.stories.tsx | 2 + .../__testdata__/expression.testdata.ts | 15 + .../__snapshots__/input-number.test.ts.snap | 2 +- .../__snapshots__/numeric-input.test.ts.snap | 2 +- .../src/widgets/__tests__/expression.test.tsx | 436 ++++++++++-------- packages/perseus/src/widgets/expression.tsx | 201 +++++--- 21 files changed, 571 insertions(+), 293 deletions(-) create mode 100644 .changeset/stupid-coats-dance.md create mode 100644 packages/perseus-editor/src/__stories__/article-editor.stories.tsx diff --git a/.changeset/stupid-coats-dance.md b/.changeset/stupid-coats-dance.md new file mode 100644 index 0000000000..5d066e9ebd --- /dev/null +++ b/.changeset/stupid-coats-dance.md @@ -0,0 +1,7 @@ +--- +"@khanacademy/math-input": minor +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +Add label options for Expression diff --git a/packages/math-input/src/components/__tests__/integration.test.tsx b/packages/math-input/src/components/__tests__/integration.test.tsx index 1cf02e81e2..76cabd7140 100644 --- a/packages/math-input/src/components/__tests__/integration.test.tsx +++ b/packages/math-input/src/components/__tests__/integration.test.tsx @@ -48,6 +48,7 @@ function InputWithContext({keypadConfiguration}) { onBlur={() => { keypadElement?.dismiss(); }} + ariaLabel="Math input" /> ); }} diff --git a/packages/math-input/src/components/input/__tests__/context-tracking.test.ts b/packages/math-input/src/components/input/__tests__/context-tracking.test.ts index 085794384e..25c793c5dd 100644 --- a/packages/math-input/src/components/input/__tests__/context-tracking.test.ts +++ b/packages/math-input/src/components/input/__tests__/context-tracking.test.ts @@ -11,7 +11,7 @@ describe("Cursor context", () => { span = document.createElement("span"); document.body.appendChild(span); - mathField = new TestMathWrapper(span, mockStrings, "en"); + mathField = new TestMathWrapper(span, "Math field", mockStrings, "en"); }); afterEach(() => { diff --git a/packages/math-input/src/components/input/__tests__/mathquill.test.ts b/packages/math-input/src/components/input/__tests__/mathquill.test.ts index e078779a28..2624ea4a9e 100644 --- a/packages/math-input/src/components/input/__tests__/mathquill.test.ts +++ b/packages/math-input/src/components/input/__tests__/mathquill.test.ts @@ -21,7 +21,7 @@ describe("MathQuill", () => { span = document.createElement("span"); document.body.appendChild(span); - mathField = new TestMathWrapper(span, mockStrings, "en"); + mathField = new TestMathWrapper(span, "Math field", mockStrings, "en"); }); afterEach(() => { diff --git a/packages/math-input/src/components/input/math-input.tsx b/packages/math-input/src/components/input/math-input.tsx index 2f136f32f8..230975cdd8 100644 --- a/packages/math-input/src/components/input/math-input.tsx +++ b/packages/math-input/src/components/input/math-input.tsx @@ -24,6 +24,7 @@ const constrainingFrictionFactor = 0.8; type Props = { keypadElement?: KeypadAPI; + ariaLabel: string; onBlur: () => void; onChange: (value: string, callback: any) => void; onFocus: () => void; @@ -97,6 +98,7 @@ class MathInput extends React.Component { this.mathField = new MathWrapper( this._mathContainer, + this.props.ariaLabel, this.context.strings, this.context.locale, { diff --git a/packages/math-input/src/components/input/math-wrapper.ts b/packages/math-input/src/components/input/math-wrapper.ts index 2582e96289..e42b54e35a 100644 --- a/packages/math-input/src/components/input/math-wrapper.ts +++ b/packages/math-input/src/components/input/math-wrapper.ts @@ -38,20 +38,28 @@ class MathWrapper { mobileKeyTranslator: Record; constructor( - element, + mathFieldMount, + ariaLabel: string, strings: MathInputStrings, locale: string, callbacks = {}, ) { - this.mathField = createMathField(element, locale, strings, () => { - return { - // use a span instead of a textarea so that we don't bring up the - // native keyboard on mobile when selecting the input - substituteTextarea: function () { - return document.createElement("span"); - }, - }; - }); + this.mathField = createMathField( + mathFieldMount, + locale, + strings, + () => { + return { + // use a span instead of a textarea so that we don't bring up the + // native keyboard on mobile when selecting the input + substituteTextarea: function () { + return document.createElement("span"); + }, + }; + }, + ); + this.mathField?.setAriaLabel(ariaLabel); + this.callbacks = callbacks; this.mobileKeyTranslator = { diff --git a/packages/math-input/src/full-mobile-input.stories.tsx b/packages/math-input/src/full-mobile-input.stories.tsx index a4b8588272..22f32e407b 100644 --- a/packages/math-input/src/full-mobile-input.stories.tsx +++ b/packages/math-input/src/full-mobile-input.stories.tsx @@ -84,6 +84,7 @@ const Basic = ({keypadElement, setKeypadElement}) => { onBlur={() => { keypadElement?.dismiss(); }} + ariaLabel="Mobile input" /> { + const [state, setState] = useState(); + const articleEditorRef = useRef(); + + function handleChange(value) { + setState(value.json); + } + + function serialize() { + // eslint-disable-next-line no-console + console.log((articleEditorRef.current as any).serialize()); + } + + return ( + <> + +
+ {}} + json={state} + onChange={handleChange} + previewURL="/perseus/frame" + ref={articleEditorRef as any} + /> + + ); +}; diff --git a/packages/perseus-editor/src/widgets/expression-editor.tsx b/packages/perseus-editor/src/widgets/expression-editor.tsx index 89f68a35fe..66e074ab70 100644 --- a/packages/perseus-editor/src/widgets/expression-editor.tsx +++ b/packages/perseus-editor/src/widgets/expression-editor.tsx @@ -17,7 +17,7 @@ import SortableArea from "../components/sortable"; import type {PerseusExpressionWidgetOptions} from "@khanacademy/perseus"; -const {InfoTip, PropCheckBox} = components; +const {InfoTip, PropCheckBox, TextInput} = components; type Props = { widgetId?: any; @@ -175,6 +175,48 @@ class ExpressionEditor extends React.Component {

Global Options

+
+ + +

+ Optional visible text; strongly encouraged to help + learners using dictation software, but can be + omitted if the surrounding content provides enough + context. +

+ +
+ +
+ + +

+ Label text that's read by screen readers. Highly + recommend adding a label here to ensure your + exercise is accessible. For more information on + writting accessible labels, please see{" "} + + this article. + +

+ +
+
{ "buttonSets", "functions", "times", + "visibleLabel", + "ariaLabel", ]; const answerForms = this.props.answerForms.map((form) => { diff --git a/packages/perseus/src/components/__stories__/math-input.stories.tsx b/packages/perseus/src/components/__stories__/math-input.stories.tsx index 5598cc4c39..e59b4a4726 100644 --- a/packages/perseus/src/components/__stories__/math-input.stories.tsx +++ b/packages/perseus/src/components/__stories__/math-input.stories.tsx @@ -25,6 +25,7 @@ const defaultObject = { value: "", onChange: () => {}, analytics: {onAnalyticsEvent: () => Promise.resolve()}, + labelText: "Math input", } as const; export const DefaultWithBasicButtonSet = ( @@ -33,7 +34,7 @@ export const DefaultWithBasicButtonSet = ( return ; }; export const DefaultWithAriaLabel = (args: StoryArgs): React.ReactElement => { - return ; + return ; }; export const KeypadOpenByDefault = (args: StoryArgs): React.ReactElement => { diff --git a/packages/perseus/src/components/__tests__/math-input.test.tsx b/packages/perseus/src/components/__tests__/math-input.test.tsx index 8d0e7b4625..7ec7dd9aa4 100644 --- a/packages/perseus/src/components/__tests__/math-input.test.tsx +++ b/packages/perseus/src/components/__tests__/math-input.test.tsx @@ -36,7 +36,6 @@ describe("Perseus' MathInput", () => { {}} keypadButtonSets={allButtonSets} - labelText="test" analytics={{onAnalyticsEvent: () => Promise.resolve()}} convertDotToTimes={false} value="" @@ -45,7 +44,48 @@ describe("Perseus' MathInput", () => { act(() => jest.runOnlyPendingTimers()); // Assert - expect(screen.getByLabelText("test")).toBeInTheDocument(); + expect( + screen.getByRole("textbox", {name: "Math input:"}), + ).toBeInTheDocument(); + }); + + it("provides a default aria label", () => { + // Assemble + render( + {}} + keypadButtonSets={allButtonSets} + analytics={{onAnalyticsEvent: () => Promise.resolve()}} + convertDotToTimes={false} + value="" + />, + ); + act(() => jest.runOnlyPendingTimers()); + + // Assert + expect( + screen.getByRole("textbox", {name: "Math input:"}), + ).toBeInTheDocument(); + }); + + it("is possible to overwrite the aria label", () => { + // Assemble + render( + {}} + keypadButtonSets={allButtonSets} + analytics={{onAnalyticsEvent: () => Promise.resolve()}} + convertDotToTimes={false} + value="" + ariaLabel="Hello world" + />, + ); + act(() => jest.runOnlyPendingTimers()); + + // Assert + expect( + screen.getByRole("textbox", {name: "Hello world:"}), + ).toBeInTheDocument(); }); it("is possible to type in the input", async () => { @@ -63,7 +103,10 @@ describe("Perseus' MathInput", () => { act(() => jest.runOnlyPendingTimers()); // Act - await userEvent.type(screen.getByRole("textbox"), "12345"); + await userEvent.type( + screen.getByRole("textbox", {name: "Math input:"}), + "12345", + ); act(() => jest.runOnlyPendingTimers()); // Assert @@ -150,7 +193,7 @@ describe("Perseus' MathInput", () => { await userEvent.click(screen.getByRole("button", {name: "1"})); // Assert - expect(screen.getByRole("textbox")).toHaveFocus(); + expect(screen.getByRole("textbox", {name: /Math input/})).toHaveFocus(); }); it("does not return focus to input after button press via keyboard", async () => { @@ -177,7 +220,9 @@ describe("Perseus' MathInput", () => { act(() => jest.runOnlyPendingTimers()); // Assert - expect(screen.getByRole("textbox")).not.toHaveFocus(); + expect( + screen.getByRole("textbox", {name: "Math input:"}), + ).not.toHaveFocus(); }); it("does not focus on the keypad button when it is clicked with the mouse", async () => { diff --git a/packages/perseus/src/components/math-input.tsx b/packages/perseus/src/components/math-input.tsx index f5a0f79b44..f606ef092f 100644 --- a/packages/perseus/src/components/math-input.tsx +++ b/packages/perseus/src/components/math-input.tsx @@ -52,7 +52,7 @@ type Props = { * Overrides deprecated `buttonSets` prop. */ keypadButtonSets?: KeypadButtonSets; - labelText?: string; + ariaLabel: string; onFocus?: () => void; onBlur?: () => void; hasError?: boolean; @@ -241,6 +241,7 @@ class InnerMathInput extends React.Component { ); } + this.__mathField?.setAriaLabel(this.props.ariaLabel); return this.__mathField; }; @@ -322,7 +323,6 @@ class InnerMathInput extends React.Component { (this.__mathFieldWrapperRef = ref)} - aria-label={this.props.labelText} onFocus={() => this.focus()} onBlur={() => this.blur()} /> @@ -394,6 +394,10 @@ class MathInput extends React.Component { declare context: React.ContextType; inputRef = React.createRef(); + static defaultProps: Pick = { + ariaLabel: "Math input", + }; + blur() { this.inputRef.current?.blur(); } diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 94be90aedc..daa04041ec 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -418,6 +418,10 @@ export type PerseusExpressionWidgetOptions = { functions: ReadonlyArray; // Use x for rendering multiplication instead of a center dot. times: boolean; + // visible label associated with the MathQuill field + visibleLabel?: string; + // aria label for screen readers attached to MathQuill field + ariaLabel?: string; // Controls when buttons for special characters are visible when using a // desktop browser. Defaults to "focused". // NOTE: This isn't listed in perseus-format.js or perseus_data.go, but diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts index 9716e69297..e2027b91cf 100644 --- a/packages/perseus/src/strings.ts +++ b/packages/perseus/src/strings.ts @@ -4,6 +4,7 @@ export type PerseusStrings = { closeKeypad: string; openKeypad: string; + mathInputBox: string; removeHighlight: string; addHighlight: string; hintPos: ({pos}: {pos: number}) => string; @@ -136,6 +137,7 @@ export const strings: { } = { closeKeypad: "close math keypad", openKeypad: "open math keypad", + mathInputBox: "Math input box", removeHighlight: "Remove highlight", addHighlight: "Add highlight", hintPos: "Hint #%(pos)s", @@ -292,6 +294,7 @@ export const strings: { export const mockStrings: PerseusStrings = { closeKeypad: "close math keypad", openKeypad: "open math keypad", + mathInputBox: "Math input box", removeHighlight: "Remove highlight", addHighlight: "Add highlight", hintPos: ({pos}) => `Hint #${pos}`, diff --git a/packages/perseus/src/styles/perseus-renderer.less b/packages/perseus/src/styles/perseus-renderer.less index 6c7c15a315..65c34b6d90 100644 --- a/packages/perseus/src/styles/perseus-renderer.less +++ b/packages/perseus/src/styles/perseus-renderer.less @@ -455,6 +455,9 @@ &.widget-inline-block { display: inline-block; + // we added this to help center inline Expression widgets + // in the context of text/MathJax + vertical-align: bottom; } } diff --git a/packages/perseus/src/widgets/__stories__/expression.stories.tsx b/packages/perseus/src/widgets/__stories__/expression.stories.tsx index eb4a412d67..826672b656 100644 --- a/packages/perseus/src/widgets/__stories__/expression.stories.tsx +++ b/packages/perseus/src/widgets/__stories__/expression.stories.tsx @@ -83,6 +83,8 @@ export const DesktopKitchenSink = (args: StoryArgs): React.ReactElement => { []} isLastUsedWidget={false} diff --git a/packages/perseus/src/widgets/__testdata__/expression.testdata.ts b/packages/perseus/src/widgets/__testdata__/expression.testdata.ts index a3eb60bdd8..9b28e70a75 100644 --- a/packages/perseus/src/widgets/__testdata__/expression.testdata.ts +++ b/packages/perseus/src/widgets/__testdata__/expression.testdata.ts @@ -68,6 +68,19 @@ export const expressionItemWithAnswer = (answer: string): PerseusItem => { ); }; +export const expressionItemWithLabels = createItemJson( + { + answerForms: [], + times: false, + buttonSets: ["basic"], + functions: [], + buttonsVisible: "always", + ariaLabel: "Test aria label", + visibleLabel: "Test visible label", + }, + {major: 1, minor: 0}, +); + export const expressionItem2: PerseusItem = createItemJson( { answerForms: [ @@ -117,6 +130,8 @@ export const expressionItem3Options: PerseusExpressionWidgetOptions = { buttonSets: ["basic"], functions: ["f", "g", "h"], buttonsVisible: "focused", + visibleLabel: "number of cm", + ariaLabel: "number of centimeters", }; export const expressionItem3: PerseusItem = createItemJson( diff --git a/packages/perseus/src/widgets/__tests__/__snapshots__/input-number.test.ts.snap b/packages/perseus/src/widgets/__tests__/__snapshots__/input-number.test.ts.snap index c43509320a..d9d3651a3a 100644 --- a/packages/perseus/src/widgets/__tests__/__snapshots__/input-number.test.ts.snap +++ b/packages/perseus/src/widgets/__tests__/__snapshots__/input-number.test.ts.snap @@ -182,7 +182,7 @@ exports[`rendering supports mobile rendering: mobile render 1`] = ` class="mq-textarea" > { - beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); - }); + describe("getOneCorrectAnswerFromRubric", () => { + beforeEach(() => { + jest.spyOn(Dependencies, "getDependencies").mockReturnValue( + testDependencies, + ); + }); - it("should return undefined if rubric.value is null/undefined", () => { - // Arrange - const rubric = { - answerForms: [], - buttonSets: [], - functions: [], - times: true, - } as const; + it("should return undefined if rubric.value is null/undefined", () => { + // Arrange + const rubric = { + answerForms: [], + buttonSets: [], + functions: [], + times: true, + } as const; - // Act - const result = Expression.getOneCorrectAnswerFromRubric(rubric); + // Act + const result = Expression.getOneCorrectAnswerFromRubric(rubric); + + // Assert + expect(result).toBeUndefined(); + }); - // Assert - expect(result).toBeUndefined(); + it("returns a correct answer when there is one correct answer", () => { + // Arrange + const rubric = { + answerForms: [ + { + value: "123", + form: true, + simplify: false, + considered: "correct", + }, + ], + buttonSets: [], + functions: [], + times: true, + } as const; + + // Act + const result = Expression.getOneCorrectAnswerFromRubric(rubric); + + // Assert + expect(result).toEqual("123"); + }); + + it("returns the first correct answer when there are multiple correct answers", () => { + // Arrange + const rubric = { + answerForms: [ + { + value: "123", + form: true, + simplify: false, + considered: "correct", + }, + { + value: "456", + form: true, + simplify: false, + considered: "correct", + }, + ], + buttonSets: [], + functions: [], + times: true, + } as const; + + // Act + const result = Expression.getOneCorrectAnswerFromRubric(rubric); + + // Assert + expect(result).toEqual("123"); + }); }); - it("returns a correct answer when there is one correct answer", () => { - // Arrange - const rubric = { - answerForms: [ - { - value: "123", - form: true, - simplify: false, - considered: "correct", - }, - ], - buttonSets: [], - functions: [], - times: true, - } as const; + describe("labels", () => { + it("renders visible label", async () => { + // Arrange, Act + renderQuestion(expressionItemWithLabels.question); - // Act - const result = Expression.getOneCorrectAnswerFromRubric(rubric); + // Assert + const label = await screen.findByText("Test visible label"); + expect(label).toBeVisible(); + }); - // Assert - expect(result).toEqual("123"); - }); + it("renders aria label", async () => { + // Arrange, Act + renderQuestion(expressionItemWithLabels.question); - it("returns the first correct answer when there are multiple correct answers", () => { - // Arrange - const rubric = { - answerForms: [ - { - value: "123", - form: true, - simplify: false, - considered: "correct", - }, - { - value: "456", - form: true, - simplify: false, - considered: "correct", - }, - ], - buttonSets: [], - functions: [], - times: true, - } as const; + // Assert + const input = await screen.findByLabelText("Test aria label:"); + expect(input).toBeVisible(); + }); - // Act - const result = Expression.getOneCorrectAnswerFromRubric(rubric); + it("input can be focused by visible label", async () => { + // Arrange + renderQuestion(expressionItemWithLabels.question); - // Assert - expect(result).toEqual("123"); - }); -}); + // Act + const label = await screen.findByText("Test visible label"); + const input = await screen.findByLabelText("Test aria label:"); + await userEvent.click(label); -describe("focus state", () => { - beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); + // Assert + expect(input).toHaveFocus(); + }); }); - it("supports directly focusing", () => { - // Arrange - renderQuestion(expressionItem2.question); + describe("focus state", () => { + beforeEach(() => { + jest.spyOn(Dependencies, "getDependencies").mockReturnValue( + testDependencies, + ); + }); - // Act - const expressionInput = screen.getByRole("textbox"); - act(() => expressionInput.focus()); + it("supports directly focusing", () => { + // Arrange + renderQuestion(expressionItem2.question); - // Assert - expect(expressionInput).toHaveFocus(); - }); + // Act + const expressionInput = screen.getByRole("textbox"); + act(() => expressionInput.focus()); - it("supports directly blurring", () => { - // Arrange - renderQuestion(expressionItem2.question); + // Assert + expect(expressionInput).toHaveFocus(); + }); - // Act - const expressionInput = screen.getByRole("textbox"); - act(() => expressionInput.focus()); - act(() => expressionInput.blur()); + it("supports directly blurring", () => { + // Arrange + renderQuestion(expressionItem2.question); - // Assert - expect(expressionInput).not.toHaveFocus(); - }); + // Act + const expressionInput = screen.getByRole("textbox"); + act(() => expressionInput.focus()); + act(() => expressionInput.blur()); - it("can be focused via a function", () => { - // arrange - const {renderer} = renderQuestion(expressionItem2.question); - const expression = renderer.findWidgets("expression 1")[0]; + // Assert + expect(expressionInput).not.toHaveFocus(); + }); - // act - act(() => expression.focusInputPath()); + it("can be focused via a function", () => { + // arrange + const {renderer} = renderQuestion(expressionItem2.question); + const expression = renderer.findWidgets("expression 1")[0]; - // Assert - const expressionInput = screen.getByRole("textbox"); - expect(expressionInput).toHaveFocus(); - }); -}); + // act + act(() => expression.focusInputPath()); -describe("rendering", () => { - beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); + // Assert + const expressionInput = screen.getByRole("textbox"); + expect(expressionInput).toHaveFocus(); + }); }); - it("supports mobile rendering", async () => { - // arrange and act - renderQuestion(expressionItem2.question, { - // Setting this triggers mobile rendering - // it would be nice if this was more clear in the code - customKeypad: true, + describe("rendering", () => { + beforeEach(() => { + jest.spyOn(Dependencies, "getDependencies").mockReturnValue( + testDependencies, + ); }); - // Assert - const mobileInput = await screen.findByRole("textbox"); - expect(mobileInput).toBeVisible(); - }); -}); + it("supports mobile rendering", async () => { + // arrange and act + renderQuestion(expressionItem2.question, { + // Setting this triggers mobile rendering + // it would be nice if this was more clear in the code + customKeypad: true, + }); -describe("interaction", () => { - beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); - jest.useFakeTimers(); + // Assert + const mobileInput = await screen.findByRole("textbox"); + expect(mobileInput).toBeVisible(); + }); }); - it("sets input value directly", () => { - // arrange - const {renderer} = renderQuestion(expressionItem2.question); - act(() => jest.runOnlyPendingTimers()); - - // act - act(() => renderer.setInputValue(["expression 1"], "123-x", () => {})); - act(() => jest.runOnlyPendingTimers()); - const score = renderer.guessAndScore()[1]; - - // Assert - expect(score.type).toBe("points"); - // Score.total doesn't exist if the input is invalid - // In this case we know that it'll be valid so we can assert directly - // @ts-expect-error - TS2339 - Property 'earned' does not exist on type 'PerseusScore'. | TS2339 - Property 'total' does not exist on type 'PerseusScore'. - expect(score.earned).toBe(score.total); - }); + describe("interaction", () => { + beforeEach(() => { + jest.spyOn(Dependencies, "getDependencies").mockReturnValue( + testDependencies, + ); + jest.useFakeTimers(); + }); - it("has a developer facility for inserting", async () => { - // arrange - const {renderer} = renderQuestion(expressionItem2.question); - act(() => jest.runOnlyPendingTimers()); + it("sets input value directly", () => { + // arrange + const {renderer} = renderQuestion(expressionItem2.question); + act(() => jest.runOnlyPendingTimers()); - const expression = renderer.findWidgets("expression 1")[0]; - act(() => expression.insert("x+1")); - act(() => jest.runOnlyPendingTimers()); + // act + act(() => + renderer.setInputValue(["expression 1"], "123-x", () => {}), + ); + act(() => jest.runOnlyPendingTimers()); + const score = renderer.guessAndScore()[1]; + + // Assert + expect(score.type).toBe("points"); + // Score.total doesn't exist if the input is invalid + // In this case we know that it'll be valid so we can assert directly + // @ts-expect-error - TS2339 - Property 'earned' does not exist on type 'PerseusScore'. | TS2339 - Property 'total' does not exist on type 'PerseusScore'. + expect(score.earned).toBe(score.total); + }); - // act - const score = renderer.score(); + it("has a developer facility for inserting", async () => { + // arrange + const {renderer} = renderQuestion(expressionItem2.question); + act(() => jest.runOnlyPendingTimers()); - // Assert - // Score.total doesn't exist if the input is invalid - // In this case we know that it'll be valid so we can assert directly - // @ts-expect-error - TS2339 - Property 'total' does not exist on type 'PerseusScore'. - expect(score.total).toBe(1); - }); -}); + const expression = renderer.findWidgets("expression 1")[0]; + act(() => expression.insert("x+1")); + act(() => jest.runOnlyPendingTimers()); -describe("error tooltip", () => { - beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue( - testDependencies, - ); - jest.useFakeTimers(); - }); + // act + const score = renderer.score(); - it("shows error text in tooltip", async () => { - // Arrange - const {renderer} = renderQuestion(expressionItem2.question); - const expression = renderer.findWidgets("expression 1")[0]; - - // Act - // Note(jeremy): You might think you could collapse all of these calls - // inside a single act() block, but that didn't work. The only way this - // test passes is with each statement in its own act() call. :/ - act(() => expression.insert("x&&&&&^1")); - act(() => jest.runOnlyPendingTimers()); - act(() => screen.getByRole("textbox").blur()); - act(() => jest.runOnlyPendingTimers()); - renderer.guessAndScore(); - - // Assert - await waitFor(() => - expect( - screen.getByText("Oops! Sorry, I don't understand that!"), - ).toBeVisible(), - ); + // Assert + // Score.total doesn't exist if the input is invalid + // In this case we know that it'll be valid so we can assert directly + // @ts-expect-error - TS2339 - Property 'total' does not exist on type 'PerseusScore'. + expect(score.total).toBe(1); + }); }); - it("does not show error text when the sen() function is used (Portuguese for sin())", async () => { - // Arrange - const {renderer} = renderQuestion(expressionItem2.question); - const expression = renderer.findWidgets("expression 1")[0]; - - // Act - act(() => expression.insert("sen(x)")); - act(() => jest.runOnlyPendingTimers()); - act(() => screen.getByRole("textbox").blur()); - renderer.guessAndScore(); - - // Assert - expect(screen.queryByText("Oops!")).toBeNull(); - expect( - screen.queryByText("Sorry, I don't understand that!"), - ).toBeNull(); + describe("error tooltip", () => { + beforeEach(() => { + jest.spyOn(Dependencies, "getDependencies").mockReturnValue( + testDependencies, + ); + jest.useFakeTimers(); + }); + + it("shows error text in tooltip", async () => { + // Arrange + const {renderer} = renderQuestion(expressionItem2.question); + const expression = renderer.findWidgets("expression 1")[0]; + + // Act + // Note(jeremy): You might think you could collapse all of these calls + // inside a single act() block, but that didn't work. The only way this + // test passes is with each statement in its own act() call. :/ + act(() => expression.insert("x&&&&&^1")); + act(() => jest.runOnlyPendingTimers()); + act(() => screen.getByRole("textbox").blur()); + act(() => jest.runOnlyPendingTimers()); + renderer.guessAndScore(); + + // Assert + await waitFor(() => + expect( + screen.getByText("Oops! Sorry, I don't understand that!"), + ).toBeVisible(), + ); + }); + + it("does not show error text when the sen() function is used (Portuguese for sin())", async () => { + // Arrange + const {renderer} = renderQuestion(expressionItem2.question); + const expression = renderer.findWidgets("expression 1")[0]; + + // Act + act(() => expression.insert("sen(x)")); + act(() => jest.runOnlyPendingTimers()); + act(() => screen.getByRole("textbox").blur()); + renderer.guessAndScore(); + + // Assert + expect(screen.queryByText("Oops!")).toBeNull(); + expect( + screen.queryByText("Sorry, I don't understand that!"), + ).toBeNull(); + }); }); }); diff --git a/packages/perseus/src/widgets/expression.tsx b/packages/perseus/src/widgets/expression.tsx index a426009aa5..9f54eef39d 100644 --- a/packages/perseus/src/widgets/expression.tsx +++ b/packages/perseus/src/widgets/expression.tsx @@ -4,8 +4,11 @@ import {Errors} from "@khanacademy/perseus-core"; import {linterContextDefault} from "@khanacademy/perseus-linter"; import {View} from "@khanacademy/wonder-blocks-core"; import Tooltip from "@khanacademy/wonder-blocks-tooltip"; +import {LabelSmall} from "@khanacademy/wonder-blocks-typography"; +import {css, StyleSheet} from "aphrodite"; import classNames from "classnames"; import * as React from "react"; +import ReactDOM from "react-dom"; import _ from "underscore"; import {PerseusI18nContext} from "../components/i18n-context"; @@ -76,6 +79,8 @@ type RenderProps = { buttonsVisible?: PerseusExpressionWidgetOptions["buttonsVisible"]; functions: PerseusExpressionWidgetOptions["functions"]; times: PerseusExpressionWidgetOptions["times"]; + visibleLabel: PerseusExpressionWidgetOptions["visibleLabel"]; + ariaLabel: PerseusExpressionWidgetOptions["ariaLabel"]; keypadConfiguration: ReturnType; }; @@ -90,6 +95,8 @@ export type Props = ExternalProps & onBlur: NonNullable; onFocus: NonNullable; times: NonNullable; + visibleLabel: PerseusExpressionWidgetOptions["visibleLabel"]; + ariaLabel: PerseusExpressionWidgetOptions["ariaLabel"]; value: string; }; @@ -148,6 +155,7 @@ export class Expression extends React.Component { static contextType = PerseusI18nContext; declare context: React.ContextType; + _textareaId = `expression_textarea_${Date.now()}`; _isMounted = false; //#region Previously a class extension @@ -356,6 +364,17 @@ export class Expression extends React.Component { // TODO(scottgrant): This is a hack to remove the deprecated call to // this.isMounted() but is still considered an anti-pattern. this._isMounted = true; + + // HACK: imperatively add an ID onto the Mathquill input + // (which in mobile is a span; desktop a textarea) + // in order to associate a visual label with it + if (this.refs.input) { + const isMobile = this.props.apiOptions.customKeypad; + const container = ReactDOM.findDOMNode(this.refs.input); + const selector = isMobile ? ".mq-textarea > span" : "textarea"; + const inputElement = (container as Element).querySelector(selector); + inputElement?.setAttribute("id", this._textareaId); + } }; // Whenever the input value changes, attempt to parse it. @@ -537,32 +556,41 @@ export class Expression extends React.Component { ); }; - render(): - | React.ReactNode - | React.ReactElement> { + render() { if (this.props.apiOptions.customKeypad) { return ( - { - // this.props.keypadElement should always be set - // when apiOptions.customKeypad is set, but how - // to convince TypeScript of this? - this.props.keypadElement?.configure( - this.props.keypadConfiguration, - () => { - if (this._isMounted) { - this._handleFocus(); - } - }, - ); - }} - onBlur={this._handleBlur} - /> + + {!!this.props.visibleLabel && ( + + {this.props.visibleLabel} + + )} + { + // this.props.keypadElement should always be set + // when apiOptions.customKeypad is set, but how + // to convince TypeScript of this? + this.props.keypadElement?.configure( + this.props.keypadConfiguration, + () => { + if (this._isMounted) { + this._handleFocus(); + } + }, + ); + }} + onBlur={this._handleBlur} + /> + ); } @@ -574,59 +602,81 @@ export class Expression extends React.Component { const {ERROR_MESSAGE, ERROR_TITLE} = this.context.strings; return ( -
- this.state.invalid && - this.setState({ - showErrorTooltip: true, - showErrorStyle: true, - }) - } - onFocus={() => - this.setState({ - showErrorTooltip: false, - }) - } - > - {/** + + {!!this.props.visibleLabel && ( + + {this.props.visibleLabel} + + )} +
+ 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 && + ERROR_TITLE + " " + ERROR_MESSAGE} + + + - -
+ extraKeys={ + this.props.keypadConfiguration?.extraKeys + } + analytics={ + this.props.analytics ?? { + onAnalyticsEvent: async () => {}, + } + } + /> + +
+ ); } } +const styles = StyleSheet.create({ + mobileLabelInputWrapper: { + padding: "15px 4px 0", + }, + desktopLabelInputWrapper: { + margin: "5px 5px 0", + }, +}); + /** * Determine the keypad configuration parameters for the input, based on the * provided properties. @@ -705,6 +755,8 @@ const propUpgrades = { buttonSets: v0props.buttonSets, functions: v0props.functions, buttonsVisible: v0props.buttonsVisible, + visibleLabel: v0props.visibleLabel, + ariaLabel: v0props.ariaLabel, answerForms: [ { @@ -745,13 +797,22 @@ export default { defaultAlignment: "inline-block", widget: ExpressionWithDependencies, transform: (widgetOptions: PerseusExpressionWidgetOptions): RenderProps => { - const {times, functions, buttonSets, buttonsVisible} = widgetOptions; + const { + times, + functions, + buttonSets, + buttonsVisible, + visibleLabel, + ariaLabel, + } = widgetOptions; return { keypadConfiguration: keypadConfigurationForProps(widgetOptions), times, functions, buttonSets, buttonsVisible, + visibleLabel, + ariaLabel, }; }, version: {major: 1, minor: 0},