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
Show file tree
Hide file tree
Changes from all 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
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
11 changes: 8 additions & 3 deletions packages/perseus-editor/src/styles/perseus-editor.less
Original file line number Diff line number Diff line change
Expand Up @@ -517,11 +517,16 @@
}

.dropdown-info {
float: left;
display: inline-flex;
margin-bottom: 16px;
}

.dropdown-placeholder {
float: right;
.dropdown-field {
display: flex;
flex-direction: row;
align-items: center;
min-width: 0;
margin-bottom: 16px;
}

.remove-choice {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,9 @@ export default {
} as Story;

export const Default = (args: StoryArgs): React.ReactElement => {
return <DropdownEditor onChange={action("onChange")} />;
return (
<div className="framework-perseus">
<DropdownEditor onChange={action("onChange")} />
</div>
);
};
74 changes: 61 additions & 13 deletions packages/perseus-editor/src/widgets/dropdown-editor.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint-disable @khanacademy/ts-no-error-suppressions */
import {components, EditorJsonify, iconTrash} from "@khanacademy/perseus";
import {TextField} from "@khanacademy/wonder-blocks-form";
import {LabelLarge, LabelMedium} 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 +36,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 @@ -104,7 +111,7 @@ class DropdownEditor extends React.Component<Props> {
return (
<div 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,13 +122,53 @@ class DropdownEditor extends React.Component<Props> {
</p>
</InfoTip>
</div>
<div className="dropdown-placeholder">
<input
type="text"
placeholder="Placeholder value"
value={this.props.placeholder}
onChange={this.onPlaceholderChange}
/>
<div className="dropdown-field">
<LabelMedium>
Visible label
<TextField
value={this.props.visibleLabel}
onChange={this.onVisibleLabelChange}
/>
</LabelMedium>
<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-field">
<LabelMedium>
Aria label
<TextField
value={this.props.ariaLabel}
onChange={this.onAriaLabelChange}
type={"text"}
/>
</LabelMedium>
<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.
</a>{" "}
If left blank, the value will default to "Select an
answer".
</p>
</InfoTip>
</div>
<div className="dropdown-field">
<LabelMedium>
Placeholder
<TextField
value={this.props.placeholder}
onChange={this.onPlaceholderChange}
placeholder={"Placeholder value"}
/>
</LabelMedium>
<InfoTip>
<p>
This value will appear as the drop down default. It
Expand All @@ -132,6 +179,7 @@ class DropdownEditor extends React.Component<Props> {
</InfoTip>
</div>
<div className="clearfix" />
<LabelMedium>Choices</LabelMedium>
<ul className="dropdown-choices">
{this.props.choices.map(function (choice, i) {
const checkedClass = choice.correct
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 @@ -292,6 +292,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 @@ -87,7 +87,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 @@ -100,7 +100,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 @@ -175,11 +175,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 @@ -286,7 +288,7 @@ describe("renderer", () => {
);

// Assert
expect(screen.getByRole("button")).toHaveTextContent(
expect(screen.getByRole("combobox")).toHaveTextContent(
/^less than or equal to$/,
);
});
Expand Down Expand Up @@ -521,7 +523,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 @@ -775,7 +777,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 @@ -1317,7 +1319,7 @@ describe("renderer", () => {
},
});

expect(screen.getByRole("button")).toHaveTextContent(
expect(screen.getByRole("combobox")).toHaveTextContent(
/greater than or equal to/,
);
});
Expand All @@ -1336,7 +1338,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 @@ -1493,9 +1495,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 @@ -1722,7 +1723,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 @@ -376,6 +376,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 @@ -146,6 +146,7 @@ export type PerseusStrings = {
}) => string;
srInteractiveElements: ({elements}: {elements: string}) => string;
srNoInteractiveElements: string;
selectAnAnswer: string;
};

/**
Expand Down Expand Up @@ -334,6 +335,7 @@ export const strings: {
context: "Screenreader-accessible description of a point on a graph",
message: "Point %(num)s at %(x)s comma %(y)s",
},
selectAnAnswer: "Select an answer",
};

/**
Expand Down Expand Up @@ -498,4 +500,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",
};
Loading