Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support visible/aria label for the Expression widget #1404

Merged
merged 22 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/stupid-coats-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@khanacademy/math-input": minor
"@khanacademy/perseus": minor
"@khanacademy/perseus-editor": minor
---

Add label options for Expression
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ function InputWithContext({keypadConfiguration}) {
onBlur={() => {
keypadElement?.dismiss();
}}
ariaLabel="Math input"
/>
);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
2 changes: 2 additions & 0 deletions packages/math-input/src/components/input/math-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const constrainingFrictionFactor = 0.8;

type Props = {
keypadElement?: KeypadAPI;
ariaLabel: string;
onBlur: () => void;
onChange: (value: string, callback: any) => void;
onFocus: () => void;
Expand Down Expand Up @@ -97,6 +98,7 @@ class MathInput extends React.Component<Props, State> {

this.mathField = new MathWrapper(
this._mathContainer,
this.props.ariaLabel,
this.context.strings,
this.context.locale,
{
Expand Down
28 changes: 18 additions & 10 deletions packages/math-input/src/components/input/math-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,28 @@ class MathWrapper {
mobileKeyTranslator: Record<Key, MathFieldUpdaterCallback>;

constructor(
element,
handeyeco marked this conversation as resolved.
Show resolved Hide resolved
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aria-label for mobile


this.callbacks = callbacks;

this.mobileKeyTranslator = {
Expand Down
1 change: 1 addition & 0 deletions packages/math-input/src/full-mobile-input.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ const Basic = ({keypadElement, setKeypadElement}) => {
onBlur={() => {
keypadElement?.dismiss();
}}
ariaLabel="Mobile input"
/>

<MobileKeypad
Expand Down
41 changes: 41 additions & 0 deletions packages/perseus-editor/src/__stories__/article-editor.stories.tsx
handeyeco marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import {ApiOptions} from "@khanacademy/perseus";
import * as React from "react";
import {useRef, useState} from "react";

import ArticleEditor from "../article-editor";
import {registerAllWidgetsAndEditorsForTesting} from "../util/register-all-widgets-and-editors-for-testing";

registerAllWidgetsAndEditorsForTesting();

export default {
title: "PerseusEditor/ArticleEditor",
};

export const Base = (): React.ReactElement => {
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 (
<>
<button onClick={serialize}>Serialize</button>
<hr />
<ArticleEditor
apiOptions={ApiOptions.defaults}
imageUploader={() => {}}
json={state}
onChange={handleChange}
previewURL="/perseus/frame"
ref={articleEditorRef as any}
/>
</>
);
};
35 changes: 34 additions & 1 deletion packages/perseus-editor/src/widgets/expression-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import type {PerseusExpressionWidgetOptions} from "@khanacademy/perseus";

const {InfoTip, PropCheckBox} = components;
const {InfoTip, PropCheckBox, TextInput} = components;

type Props = {
widgetId?: any;
Expand Down Expand Up @@ -175,6 +175,37 @@
<div className="perseus-widget-expression-editor">
<h3 className="expression-editor-h3">Global Options</h3>

<div className="perseus-widget-row">
<label>
Visible label:{" "}
<TextInput
value={this.props.visibleLabel}
onChange={this.change("visibleLabel")}
/>
</label>
<InfoTip>
<p>
Optional visible text; strongly encouraged to help
handeyeco marked this conversation as resolved.
Show resolved Hide resolved
learners using dictation software, but can be
omitted if the surrounding content provides enough
context.
</p>
</InfoTip>
</div>

<div className="perseus-widget-row">
<label>
Aria label:{" "}
<TextInput
value={this.props.ariaLabel}
onChange={this.change("ariaLabel")}
/>
</label>
<InfoTip>
<p>Label text that's read by screenreaders.</p>
catandthemachines marked this conversation as resolved.
Show resolved Hide resolved
</InfoTip>
</div>

<div>
<PropCheckBox
times={this.props.times}
Expand Down Expand Up @@ -254,6 +285,8 @@
"buttonSets",
"functions",
"times",
"visibleLabel",
"ariaLabel",

Check warning on line 289 in packages/perseus-editor/src/widgets/expression-editor.tsx

View check run for this annotation

Codecov / codecov/patch

packages/perseus-editor/src/widgets/expression-editor.tsx#L288-L289

Added lines #L288 - L289 were not covered by tests
];

const answerForms = this.props.answerForms.map((form) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const defaultObject = {
value: "",
onChange: () => {},
analytics: {onAnalyticsEvent: () => Promise.resolve()},
labelText: "Math input",
} as const;

export const DefaultWithBasicButtonSet = (
Expand All @@ -33,7 +34,7 @@ export const DefaultWithBasicButtonSet = (
return <MathInput {...defaultObject} />;
};
export const DefaultWithAriaLabel = (args: StoryArgs): React.ReactElement => {
return <MathInput {...defaultObject} labelText="Sample label" />;
return <MathInput {...defaultObject} ariaLabel="Sample label" />;
};

export const KeypadOpenByDefault = (args: StoryArgs): React.ReactElement => {
Expand Down
55 changes: 50 additions & 5 deletions packages/perseus/src/components/__tests__/math-input.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ describe("Perseus' MathInput", () => {
<MathInput
onChange={() => {}}
keypadButtonSets={allButtonSets}
labelText="test"
analytics={{onAnalyticsEvent: () => Promise.resolve()}}
convertDotToTimes={false}
value=""
Expand All @@ -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(
<MathInput
onChange={() => {}}
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(
<MathInput
onChange={() => {}}
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 () => {
Expand All @@ -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
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
8 changes: 6 additions & 2 deletions packages/perseus/src/components/math-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
* Overrides deprecated `buttonSets` prop.
*/
keypadButtonSets?: KeypadButtonSets;
labelText?: string;
ariaLabel: string;
onFocus?: () => void;
onBlur?: () => void;
hasError?: boolean;
Expand Down Expand Up @@ -241,6 +241,7 @@
);
}

this.__mathField?.setAriaLabel(this.props.ariaLabel);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aria-label for desktop

return this.__mathField;
};

Expand Down Expand Up @@ -322,7 +323,6 @@
<span
className={className}
ref={(ref) => (this.__mathFieldWrapperRef = ref)}
aria-label={this.props.labelText}
onFocus={() => this.focus()}
onBlur={() => this.blur()}
/>
Expand Down Expand Up @@ -394,6 +394,10 @@
declare context: React.ContextType<typeof MathInputI18nContext>;
inputRef = React.createRef<InnerMathInput>();

static defaultProps: Pick<Props, "ariaLabel"> = {
ariaLabel: "Math input",
};

Check warning on line 400 in packages/perseus/src/components/math-input.tsx

View check run for this annotation

Codecov / codecov/patch

packages/perseus/src/components/math-input.tsx#L400

Added line #L400 was not covered by tests
blur() {
this.inputRef.current?.blur();
}
Expand Down
2 changes: 2 additions & 0 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ export type PerseusExpressionWidgetOptions = {
functions: ReadonlyArray<string>;
// Use x for rendering multiplication instead of a center dot.
times: boolean;
visibleLabel?: string;
handeyeco marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
3 changes: 3 additions & 0 deletions packages/perseus/src/styles/perseus-renderer.less
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ export const DesktopKitchenSink = (args: StoryArgs): React.ReactElement => {
<expressionExport.widget
alignment={null}
value=""
visibleLabel=""
ariaLabel=""
containerSizeClass="small"
findWidgets={(callback) => []}
isLastUsedWidget={false}
Expand Down
15 changes: 15 additions & 0 deletions packages/perseus/src/widgets/__testdata__/expression.testdata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down Expand Up @@ -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(
Expand Down
Loading
Loading