Skip to content

Commit

Permalink
[🔥AUDIT🔥] WB-1645.fix Dropdown: Fix regression in SingleSelect (#2146)
Browse files Browse the repository at this point in the history
🖍 _This is an audit!_ 🖍

## Summary:

Fixes a regression in `SingleSelect` where the opener is not able to use an
empty string as a label.

In #2139, I changed the logic to use `1 item` for those cases, but turns out
that this "empty" value feature is required by Perseus.

Issue: WB-1645

## Test plan:

Verify that the "Two with Text" example in the `SingleSelect` storybook page
shows empty string as the label for both openers:

http://localhost:6061/?path=/story/dropdown-singleselect--two-with-text

<img width="1584" alt="Screenshot 2023-12-15 at 1 52 16 PM" src="https://github.com/Khan/wonder-blocks/assets/843075/5a65edeb-334b-43db-9a72-ce2fe2058139">

Author: jandrade

Auditors: jeresig, jeremywiebe, nedredmond

Required Reviewers:

Approved By: jeresig, jeremywiebe

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 16.x), ✅ Lint (ubuntu-latest, 16.x), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ⏭  Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ✅ gerald, ✅ codecov/project, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 16.x), ✅ Lint (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ⏭  Chromatic - Skip on Release PR (changesets), ✅ gerald, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ⏭  dependabot

Pull Request URL: #2146
  • Loading branch information
jandrade authored Dec 15, 2023
1 parent 1b21747 commit 5d34b4b
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/warm-hats-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-dropdown": patch
---

Fix issue with empty option items
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable max-lines */
import * as React from "react";
import {fireEvent, render, screen} from "@testing-library/react";
import userEvent from "@testing-library/user-event";
Expand Down Expand Up @@ -33,6 +34,96 @@ describe("SingleSelect", () => {
</SingleSelect>
);

describe("opener", () => {
it("should render the placeholder if no selections are made", () => {
// Arrange
render(
<SingleSelect
placeholder="Default placeholder"
onChange={jest.fn()}
>
<OptionItem label="Toggle A" value="toggle_a" />
<OptionItem label="Toggle B" value="toggle_b" />
</SingleSelect>,
);

// Act
const opener = screen.getByRole("button");

// Assert
expect(opener).toHaveTextContent("Default placeholder");
});

it("should render empty if the selected option has an empty value", () => {
// Arrange
render(
<SingleSelect
placeholder="Default placeholder"
onChange={jest.fn()}
selectedValue=""
>
<OptionItem label="" value="" />
<OptionItem label="Toggle A" value="toggle_a" />
<OptionItem label="Toggle B" value="toggle_b" />
</SingleSelect>,
);

// Act
const opener = screen.getByRole("button");

// Assert
expect(opener).toHaveTextContent("");
});

it("should render the label of the selected option", () => {
// Arrange
render(
<SingleSelect
placeholder="Default placeholder"
onChange={jest.fn()}
selectedValue="toggle_a"
>
<OptionItem label="Toggle A" value="toggle_a" />
<OptionItem label="Toggle B" value="toggle_b" />
</SingleSelect>,
);

// Act
const opener = screen.getByRole("button");

// Assert
expect(opener).toHaveTextContent("Toggle A");
});

it("should render labelAsText of the selected option", () => {
// Arrange
render(
<SingleSelect
placeholder="Default placeholder"
onChange={jest.fn()}
selectedValue="toggle_a"
>
<OptionItem
label={<div>custom item A</div>}
value="toggle_a"
labelAsText="Plain Toggle A"
/>
<OptionItem
label={<div>custom item B</div>}
value="toggle_b"
labelAsText="Plain Toggle B"
/>
</SingleSelect>,
);

// Act
const opener = screen.getByRole("button");

// Assert
expect(opener).toHaveTextContent("Plain Toggle A");
});
});

describe("mouse", () => {
it("should open when clicking on the default opener", () => {
// Arrange
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ export default class SingleSelect extends React.Component<Props, State> {
// If nothing is selected, or if the selectedValue doesn't match any
// item in the menu, use the placeholder.
const menuText = selectedItem
? getLabel(selectedItem.props) || defaultLabels.someSelected(1)
? getLabel(selectedItem.props)
: placeholder;

const dropdownOpener = opener ? (
Expand Down

0 comments on commit 5d34b4b

Please sign in to comment.