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

Add visible label and ARIA label to Dropdown widget #1845

Merged
merged 23 commits into from
Dec 2, 2024
Merged
6 changes: 6 additions & 0 deletions .changeset/hot-chairs-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-editor": minor
---

Add labels to Dropdown widget
60 changes: 50 additions & 10 deletions packages/perseus-editor/src/widgets/dropdown-editor.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/* eslint-disable @khanacademy/ts-no-error-suppressions */
import {components, EditorJsonify, iconTrash} from "@khanacademy/perseus";
import {View} from "@khanacademy/wonder-blocks-core";
import {LabeledTextField} from "@khanacademy/wonder-blocks-form";
import {LabelLarge} from "@khanacademy/wonder-blocks-typography";
import PropTypes from "prop-types";
import * as React from "react";
import ReactDOM from "react-dom";
Expand Down Expand Up @@ -34,11 +37,16 @@ class DropdownEditor extends React.Component<Props> {
],
};

onPlaceholderChange: (arg1: React.ChangeEvent<HTMLInputElement>) => void = (
e,
) => {
const placeholder = e.target.value;
this.props.onChange({placeholder: placeholder});
onVisibleLabelChange: (arg1: string) => void = (visibleLabel) => {
this.props.onChange({visibleLabel});
};

onAriaLabelChange: (arg1: string) => void = (ariaLabel) => {
this.props.onChange({ariaLabel});
};

onPlaceholderChange: (arg1: string) => void = (placeholder) => {
this.props.onChange({placeholder});
};

onCorrectChange: (
Expand Down Expand Up @@ -102,9 +110,9 @@ class DropdownEditor extends React.Component<Props> {
render(): React.ReactNode {
const dropdownGroupName = _.uniqueId("perseus_dropdown_");
return (
<div className="perseus-widget-dropdown">
<View className="perseus-widget-dropdown">
<div className="dropdown-info">
Dropdown
<LabelLarge>Dropdown</LabelLarge>
<InfoTip>
<p>
The drop down is useful for making inequalities in a
Expand All @@ -115,9 +123,41 @@ class DropdownEditor extends React.Component<Props> {
</p>
</InfoTip>
</div>
<div className="dropdown-visible-label">
<LabeledTextField
label="Visible label"
value={this.props.visibleLabel}
onChange={this.onVisibleLabelChange}
/>
<InfoTip>
<p>Optional visible label</p>
</InfoTip>
Copy link
Contributor

Choose a reason for hiding this comment

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

The tooltip icons seem oddly placed (being below the input field instead of after the label text). Instead of using <LabeledTextField>, would it be possible to have a separate label, followed by the <InfoTip> and then a regular <TextField>?

Tooltip Placement

</div>
<div className="dropdown-aria-label">
<LabeledTextField
label="Aria label"
value={this.props.ariaLabel || ""}
onChange={this.onAriaLabelChange}
/>
<InfoTip>
<p>
Label text that's read by screen readers. Highly
recommend adding a label here to ensure your
exercise is accessible. For more information on
writing accessible labels, please see{" "}
<a
href="https://www.w3.org/WAI/tips/designing/#ensure-that-form-elements-include-clearly-associated-labels"
target="_blank"
>
this article. If left blank, the value will
default to "Select an answer".
</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Perhaps linking the article this way:

Please see this
<a
    href="https://www.w3.org/WAI/tips/designing/#ensure-that-form-elements-include-clearly-associated-labels"
    target="_blank"
>
    article on writing accessible labels
</a>
for more information. If left blank, the value will default to "Select an answer".

</p>
</InfoTip>
</div>
<div className="dropdown-placeholder">
<input
type="text"
<LabeledTextField
label="Placeholder"
placeholder="Placeholder value"
value={this.props.placeholder}
onChange={this.onPlaceholderChange}
Expand Down Expand Up @@ -204,7 +244,7 @@ class DropdownEditor extends React.Component<Props> {
<InlineIcon {...iconPlus} /> Add a choice{" "}
</a>
</div>
</div>
</View>
);
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/perseus/src/__testdata__/renderer.testdata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export const dropdownWidget: DropdownWidget = {
graded: true,
options: {
static: false,
ariaLabel: "Test ARIA label",
visibleLabel: "Test visible label",
placeholder: "greater/less than or equal to",
choices: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,15 @@ exports[`renderer snapshots correct answer: correct answer 1`] = `
<div
class="perseus-widget-container widget-nohighlight widget-inline-block"
>
<div>
<div
class="default_xu2jcg"
>
<label
class="text_f1191h-o_O-LabelLarge_5s82ln"
id="uid-dropdown-widget-2-dropdown-label"
>
Test visible label
</label>
<div
class="default_xu2jcg-o_O-menuWrapper_wvrnr4"
>
Expand All @@ -357,12 +365,15 @@ exports[`renderer snapshots correct answer: correct answer 1`] = `
data-testid="dropdown-live-region"
/>
<button
aria-controls="uid-single-select-dropdown-2-wb-id"
aria-controls="uid-single-select-dropdown-3-wb-id"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="listbox"
aria-label="Test ARIA label"
aria-labelledby="uid-dropdown-widget-2-dropdown-label"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-single-select-opener-3-wb-id"
id="uid-dropdown-widget-2-dropdown"
role="combobox"
type="button"
>
<span
Expand Down Expand Up @@ -413,7 +424,15 @@ exports[`renderer snapshots incorrect answer: incorrect answer 1`] = `
<div
class="perseus-widget-container widget-nohighlight widget-inline-block"
>
<div>
<div
class="default_xu2jcg"
>
<label
class="text_f1191h-o_O-LabelLarge_5s82ln"
id="uid-dropdown-widget-4-dropdown-label"
>
Test visible label
</label>
<div
class="default_xu2jcg-o_O-menuWrapper_wvrnr4"
>
Expand All @@ -425,12 +444,15 @@ exports[`renderer snapshots incorrect answer: incorrect answer 1`] = `
data-testid="dropdown-live-region"
/>
<button
aria-controls="uid-single-select-dropdown-4-wb-id"
aria-controls="uid-single-select-dropdown-5-wb-id"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="listbox"
aria-label="Test ARIA label"
aria-labelledby="uid-dropdown-widget-4-dropdown-label"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-single-select-opener-5-wb-id"
id="uid-dropdown-widget-4-dropdown"
role="combobox"
type="button"
>
<span
Expand Down Expand Up @@ -481,7 +503,15 @@ exports[`renderer snapshots initial render: initial render 1`] = `
<div
class="perseus-widget-container widget-nohighlight widget-inline-block"
>
<div>
<div
class="default_xu2jcg"
>
<label
class="text_f1191h-o_O-LabelLarge_5s82ln"
id="uid-dropdown-widget-0-dropdown-label"
>
Test visible label
</label>
<div
class="default_xu2jcg-o_O-menuWrapper_wvrnr4"
>
Expand All @@ -493,12 +523,15 @@ exports[`renderer snapshots initial render: initial render 1`] = `
data-testid="dropdown-live-region"
/>
<button
aria-controls="uid-single-select-dropdown-0-wb-id"
aria-controls="uid-single-select-dropdown-1-wb-id"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="listbox"
aria-label="Test ARIA label"
aria-labelledby="uid-dropdown-widget-0-dropdown-label"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-single-select-opener-1-wb-id"
id="uid-dropdown-widget-0-dropdown"
role="combobox"
type="button"
>
<span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ describe("ExtractPerseusData", () => {
type: "dropdown",
options: {
placeholder: "Select an option",
ariaLabel: "",
static: false,
choices: [
{
Expand Down
21 changes: 11 additions & 10 deletions packages/perseus/src/__tests__/renderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe("renderer", () => {
const {container} = renderQuestion(question1);

// Act
await userEvent.click(screen.getByRole("button"));
await userEvent.click(screen.getByRole("combobox"));
await userEvent.click(screen.getAllByRole("option")[2]);
act(() => jest.runOnlyPendingTimers());

Expand All @@ -97,7 +97,7 @@ describe("renderer", () => {
const {container} = renderQuestion(question1);

// Act
await userEvent.click(screen.getByRole("button"));
await userEvent.click(screen.getByRole("combobox"));
await userEvent.click(screen.getAllByRole("option")[1]);
act(() => jest.runOnlyPendingTimers());

Expand Down Expand Up @@ -172,11 +172,13 @@ describe("renderer", () => {
expect(renderer.state.widgetProps).toMatchInlineSnapshot(`
{
"dropdown 1": {
"ariaLabel": "Test ARIA label",
"choices": [
"greater than or equal to",
"less than or equal to",
],
"placeholder": "greater/less than or equal to",
"visibleLabel": "Test visible label",
},
}
`);
Expand Down Expand Up @@ -283,7 +285,7 @@ describe("renderer", () => {
);

// Assert
expect(screen.getByRole("button")).toHaveTextContent(
expect(screen.getByRole("combobox")).toHaveTextContent(
/^less than or equal to$/,
);
});
Expand Down Expand Up @@ -518,7 +520,7 @@ describe("renderer", () => {
screen.getByText("This is a placeholder"),
).toBeInTheDocument();
// Make sure the 'dropdown' widget wasn't rendered!
expect(screen.queryAllByRole("button")).toHaveLength(0);
expect(screen.queryAllByRole("combobox")).toHaveLength(0);
});

it("should render columns", () => {
Expand Down Expand Up @@ -772,7 +774,7 @@ describe("renderer", () => {
originalWidgetProps = clone(renderer.state.widgetProps);

// Poke the renderer so it's not in it's initial-render state
await userEvent.click(screen.getByRole("button"));
await userEvent.click(screen.getByRole("combobox"));
act(() => jest.runOnlyPendingTimers()); // There's a setTimeout to open the dropdown
await userEvent.click(screen.getAllByRole("option")[1]);
});
Expand Down Expand Up @@ -1314,7 +1316,7 @@ describe("renderer", () => {
},
});

expect(screen.getByRole("button")).toHaveTextContent(
expect(screen.getByRole("combobox")).toHaveTextContent(
/greater than or equal to/,
);
});
Expand All @@ -1333,7 +1335,7 @@ describe("renderer", () => {
});

// Assert
let el = screen.getByRole("button");
let el = screen.getByRole("combobox");
while (el != null) {
if (el.classList.contains("widget-full-width")) {
break;
Expand Down Expand Up @@ -1490,9 +1492,8 @@ describe("renderer", () => {
const node = renderer.getDOMNodeForPath(["dropdown 1"]);

// Assert
// "button" role is the WB dropdown's "opener" element
// @ts-expect-error - TS2345 - Argument of type 'Element | Text | null | undefined' is not assignable to parameter of type 'HTMLElement'.
expect(within(node).queryAllByRole("button")).toHaveLength(1);
expect(within(node).queryAllByRole("combobox")).toHaveLength(1);
});

it("should return the widget's getDOMNodeForPath() result for the widget at requested FocusPath", () => {
Expand Down Expand Up @@ -1719,7 +1720,7 @@ describe("renderer", () => {
await userEvent.type(screen.getAllByRole("textbox")[1], "200");

// Open the dropdown and select the second (idx: 1) item
await userEvent.click(screen.getByRole("button"));
await userEvent.click(screen.getByRole("combobox"));
act(() => jest.runOnlyPendingTimers());
await userEvent.click(screen.getAllByRole("option")[1]);
act(() => jest.runOnlyPendingTimers());
Expand Down
4 changes: 4 additions & 0 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,10 @@ export type PerseusDropdownWidgetOptions = {
placeholder: string;
// Always false. Not used for this widget
static: boolean;
// Translatable Text; visible label for the dropdown
visibleLabel?: string;
// Translatable Text; aria label that screen readers will read
ariaLabel?: string;
};

export type PerseusDropdownChoice = {
Expand Down
3 changes: 3 additions & 0 deletions packages/perseus/src/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export type PerseusStrings = {
}) => string;
srInteractiveElements: ({elements}: {elements: string}) => string;
srNoInteractiveElements: string;
selectAnAnswer: string;
};

/**
Expand Down Expand Up @@ -325,6 +326,7 @@ export const strings: {
},
srInteractiveElements: "Interactive elements: %(elements)s",
srNoInteractiveElements: "No interactive elements",
selectAnAnswer: "Select an answer",
};

/**
Expand Down Expand Up @@ -487,4 +489,5 @@ export const mockStrings: PerseusStrings = {
srPointAtCoordinates: ({num, x, y}) => `Point ${num} at ${x} comma ${y}`,
srInteractiveElements: ({elements}) => `Interactive elements: ${elements}`,
srNoInteractiveElements: "No interactive elements",
selectAnAnswer: "Select an answer",
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
constant,
object,
string,
optional,
} from "../general-purpose-parsers";

import {parseWidget} from "./widget";
Expand All @@ -15,6 +16,8 @@ export const parseDropdownWidget: Parser<DropdownWidget> = parseWidget(
constant("dropdown"),
object({
placeholder: string,
ariaLabel: optional(string),
visibleLabel: optional(string),
static: boolean,
choices: array(
object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const question1: PerseusRenderer = {
correct: true,
},
],
ariaLabel: "Test ARIA label",
visibleLabel: "Test visible label",
},
version: {
major: 0,
Expand Down Expand Up @@ -77,7 +79,7 @@ describe("Dropdown AI utils", () => {
const {renderer} = renderQuestion(question1);

// Act
const dropdown = screen.getByRole("button");
const dropdown = screen.getByRole("combobox");
await userEvent.click(dropdown);
await userEvent.click(screen.getByText("greater than or equal to"));

Expand Down
Loading