From f77bd8c417b35de2cf51ad686feed1e60e4f70fd Mon Sep 17 00:00:00 2001 From: Matthew Curtis Date: Wed, 10 Jul 2024 12:07:14 -0500 Subject: [PATCH 01/19] add inputs for labels to the expression editor --- .../src/widgets/expression-editor.tsx | 33 ++++- packages/perseus/src/perseus-types.ts | 2 + .../__stories__/expression.stories.tsx | 2 + packages/perseus/src/widgets/expression.tsx | 114 +++++++++++------- 4 files changed, 104 insertions(+), 47 deletions(-) diff --git a/packages/perseus-editor/src/widgets/expression-editor.tsx b/packages/perseus-editor/src/widgets/expression-editor.tsx index 89f68a35fe..c4462b276a 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,37 @@ 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 screenreaders.

+ +
+
; // Use x for rendering multiplication instead of a center dot. times: boolean; + visibleLabel?: string; + 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/widgets/__stories__/expression.stories.tsx b/packages/perseus/src/widgets/__stories__/expression.stories.tsx index 421706ca88..cdabce5256 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/expression.tsx b/packages/perseus/src/widgets/expression.tsx index 8318ae7097..be24b36361 100644 --- a/packages/perseus/src/widgets/expression.tsx +++ b/packages/perseus/src/widgets/expression.tsx @@ -75,6 +75,8 @@ type RenderProps = { buttonsVisible?: PerseusExpressionWidgetOptions["buttonsVisible"]; functions: PerseusExpressionWidgetOptions["functions"]; times: PerseusExpressionWidgetOptions["times"]; + visibleLabel: PerseusExpressionWidgetOptions["visibleLabel"]; + ariaLabel: PerseusExpressionWidgetOptions["ariaLabel"]; keypadConfiguration: ReturnType; }; @@ -89,6 +91,8 @@ export type Props = ExternalProps & onBlur: NonNullable; onFocus: NonNullable; times: NonNullable; + visibleLabel: PerseusExpressionWidgetOptions["visibleLabel"]; + ariaLabel: PerseusExpressionWidgetOptions["ariaLabel"]; value: string; }; @@ -573,55 +577,62 @@ 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.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} + + + - -
+ analytics={ + this.props.analytics ?? { + onAnalyticsEvent: async () => {}, + } + } + /> + +
+ ); } } @@ -704,6 +715,8 @@ const propUpgrades = { buttonSets: v0props.buttonSets, functions: v0props.functions, buttonsVisible: v0props.buttonsVisible, + visibleLabel: v0props.visibleLabel, + ariaLabel: v0props.ariaLabel, answerForms: [ { @@ -744,13 +757,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}, From 5611df6e1c7a2b27bedb4177bec931bcea8e8ffc Mon Sep 17 00:00:00 2001 From: Matthew Curtis Date: Thu, 11 Jul 2024 10:39:56 -0500 Subject: [PATCH 02/19] add aria label to mobile input --- .../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__/math-input.stories.tsx | 1 + .../components/__tests__/math-input.test.tsx | 6 ++ .../perseus/src/components/math-input.tsx | 6 +- .../__testdata__/expression.testdata.ts | 2 + packages/perseus/src/widgets/expression.tsx | 91 ++++++++++++++----- 11 files changed, 103 insertions(+), 39 deletions(-) diff --git a/packages/math-input/src/components/__tests__/integration.test.tsx b/packages/math-input/src/components/__tests__/integration.test.tsx index b79f734be6..8499b707f7 100644 --- a/packages/math-input/src/components/__tests__/integration.test.tsx +++ b/packages/math-input/src/components/__tests__/integration.test.tsx @@ -47,6 +47,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" /> {}, analytics: {onAnalyticsEvent: () => Promise.resolve()}, + labelText: "Math input", } as const; export const DefaultWithBasicButtonSet = ( diff --git a/packages/perseus/src/components/__tests__/math-input.test.tsx b/packages/perseus/src/components/__tests__/math-input.test.tsx index f0ac28fe3e..98c860e261 100644 --- a/packages/perseus/src/components/__tests__/math-input.test.tsx +++ b/packages/perseus/src/components/__tests__/math-input.test.tsx @@ -55,6 +55,7 @@ describe("Perseus' MathInput", () => { keypadButtonSets={allButtonSets} analytics={{onAnalyticsEvent: () => Promise.resolve()}} convertDotToTimes={false} + labelText="Math input" value="" />, ); @@ -77,6 +78,7 @@ describe("Perseus' MathInput", () => { keypadButtonSets={allButtonSets} analytics={{onAnalyticsEvent: () => Promise.resolve()}} convertDotToTimes={false} + labelText="Math input" value="" />, ); @@ -104,6 +106,7 @@ describe("Perseus' MathInput", () => { buttonSets={["basic+div"]} analytics={{onAnalyticsEvent: () => Promise.resolve()}} convertDotToTimes={false} + labelText="Math input" value="" />, ); @@ -131,6 +134,7 @@ describe("Perseus' MathInput", () => { keypadButtonSets={allButtonSets} analytics={{onAnalyticsEvent: () => Promise.resolve()}} convertDotToTimes={false} + labelText="Math input" value="" />, ); @@ -153,6 +157,7 @@ describe("Perseus' MathInput", () => { keypadButtonSets={allButtonSets} analytics={{onAnalyticsEvent: () => Promise.resolve()}} convertDotToTimes={false} + labelText="Math input" value="" />, ); @@ -178,6 +183,7 @@ describe("Perseus' MathInput", () => { buttonsVisible="always" analytics={{onAnalyticsEvent: () => Promise.resolve()}} convertDotToTimes={false} + labelText="Math input" value="" />, ); diff --git a/packages/perseus/src/components/math-input.tsx b/packages/perseus/src/components/math-input.tsx index f5a0f79b44..95b642f91e 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; + labelText: string; onFocus?: () => void; onBlur?: () => void; hasError?: boolean; @@ -77,6 +77,7 @@ type InnerProps = Props & { type DefaultProps = { value: Props["value"]; + labelText: Props["labelText"]; convertDotToTimes: Props["convertDotToTimes"]; }; @@ -99,6 +100,7 @@ class InnerMathInput extends React.Component { static defaultProps: DefaultProps = { value: "", + labelText: "Math input", convertDotToTimes: false, }; @@ -241,6 +243,7 @@ class InnerMathInput extends React.Component { ); } + this.__mathField?.setAriaLabel(this.props.labelText); return this.__mathField; }; @@ -322,7 +325,6 @@ class InnerMathInput extends React.Component { (this.__mathFieldWrapperRef = ref)} - aria-label={this.props.labelText} onFocus={() => this.focus()} onBlur={() => this.blur()} /> diff --git a/packages/perseus/src/widgets/__testdata__/expression.testdata.ts b/packages/perseus/src/widgets/__testdata__/expression.testdata.ts index a3eb60bdd8..fea8eecd8a 100644 --- a/packages/perseus/src/widgets/__testdata__/expression.testdata.ts +++ b/packages/perseus/src/widgets/__testdata__/expression.testdata.ts @@ -117,6 +117,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/expression.tsx b/packages/perseus/src/widgets/expression.tsx index be24b36361..f9ef4f4499 100644 --- a/packages/perseus/src/widgets/expression.tsx +++ b/packages/perseus/src/widgets/expression.tsx @@ -5,6 +5,7 @@ import {View} from "@khanacademy/wonder-blocks-core"; import Tooltip from "@khanacademy/wonder-blocks-tooltip"; import classNames from "classnames"; import * as React from "react"; +import ReactDOM from "react-dom"; import _ from "underscore"; import {PerseusI18nContext} from "../components/i18n-context"; @@ -540,32 +541,57 @@ export class Expression extends React.Component { ); }; - render(): - | React.ReactNode - | React.ReactElement> { + render() { + const textareaId = `expression_textarea_${Date.now()}`; + if (this.props.apiOptions.customKeypad) { + // HACK: imperatively add an ID onto the Mathquill input + // (which in mobile is a span) + // in order to associate a visual label with it + if (this.refs.input) { + const container = ReactDOM.findDOMNode(this.refs.input); + const inputSpan = (container as Element).querySelector( + ".mq-textarea > span", + ); + inputSpan?.setAttribute("id", textareaId); + } + 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.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} + /> + ); } @@ -576,10 +602,22 @@ export class Expression extends React.Component { const {ERROR_MESSAGE, ERROR_TITLE} = this.context.strings; + // HACK: imperatively add an ID onto the Mathquill textarea + // in order to associate a visual label with it + if (this.refs.input) { + const container = ReactDOM.findDOMNode(this.refs.input); + const textarea = (container as Element).getElementsByTagName( + "textarea", + ); + textarea[0].setAttribute("id", textareaId); + } + return ( {!!this.props.visibleLabel && ( - + )}
{ onFocus={this._handleFocus} onBlur={this._handleBlur} hasError={this.state.showErrorStyle} + labelText={ + this.props.ariaLabel || "Expression input" + } extraKeys={ this.props.keypadConfiguration?.extraKeys } From 2245b4bc9a974138c2548d1f8ea92a7e43aeb39c Mon Sep 17 00:00:00 2001 From: Matthew Curtis Date: Thu, 11 Jul 2024 11:58:58 -0500 Subject: [PATCH 03/19] fix tests --- packages/perseus/src/components/__tests__/math-input.test.tsx | 2 +- .../widgets/__tests__/__snapshots__/input-number.test.ts.snap | 2 +- .../widgets/__tests__/__snapshots__/numeric-input.test.ts.snap | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/perseus/src/components/__tests__/math-input.test.tsx b/packages/perseus/src/components/__tests__/math-input.test.tsx index 98c860e261..656adf9682 100644 --- a/packages/perseus/src/components/__tests__/math-input.test.tsx +++ b/packages/perseus/src/components/__tests__/math-input.test.tsx @@ -43,7 +43,7 @@ describe("Perseus' MathInput", () => { jest.runOnlyPendingTimers(); // Assert - expect(screen.getByLabelText("test")).toBeInTheDocument(); + expect(screen.getByLabelText("test:")).toBeInTheDocument(); }); it("is possible to type in the input", async () => { 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" > Date: Thu, 11 Jul 2024 12:00:27 -0500 Subject: [PATCH 04/19] changeset --- .changeset/stupid-coats-dance.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/stupid-coats-dance.md 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 From 2a5992c9b1200f3f7247a7aac1a66e656b58b9d7 Mon Sep 17 00:00:00 2001 From: Matthew Curtis Date: Wed, 17 Jul 2024 14:57:54 -0500 Subject: [PATCH 05/19] rough out idea for CSS --- packages/perseus/src/styles/perseus-renderer.less | 8 +++++++- packages/perseus/src/widgets/expression.tsx | 10 ++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/perseus/src/styles/perseus-renderer.less b/packages/perseus/src/styles/perseus-renderer.less index 88c363522a..bdcfdf1735 100644 --- a/packages/perseus/src/styles/perseus-renderer.less +++ b/packages/perseus/src/styles/perseus-renderer.less @@ -100,7 +100,13 @@ font-family: "Lato", sans-serif; font-weight: 400; font-size: 18px; - line-height: 22px; + // line-height: 22px; + display: flex; + flex-wrap: wrap; + align-items: end; + } + + [data-perseus-paragraph-index] { margin: 22px 0px; } diff --git a/packages/perseus/src/widgets/expression.tsx b/packages/perseus/src/widgets/expression.tsx index 7548a81c8d..b9fe6d5a0a 100644 --- a/packages/perseus/src/widgets/expression.tsx +++ b/packages/perseus/src/widgets/expression.tsx @@ -614,9 +614,15 @@ export class Expression extends React.Component { } return ( - + {!!this.props.visibleLabel && ( - @@ -603,21 +614,11 @@ export class Expression extends React.Component { const {ERROR_MESSAGE, ERROR_TITLE} = this.context.strings; - // HACK: imperatively add an ID onto the Mathquill textarea - // in order to associate a visual label with it - if (this.refs.input) { - const container = ReactDOM.findDOMNode(this.refs.input); - const textarea = (container as Element).getElementsByTagName( - "textarea", - ); - textarea[0].setAttribute("id", textareaId); - } - return ( {!!this.props.visibleLabel && (