Skip to content

Commit

Permalink
Fix Dropdown items not closing
Browse files Browse the repository at this point in the history
  • Loading branch information
moroshko committed May 7, 2020
1 parent b0cbd07 commit 0528be3
Show file tree
Hide file tree
Showing 9 changed files with 895 additions and 3,287 deletions.
2,801 changes: 451 additions & 2,350 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"date-fns": "2.12.0",
"deep-object-diff": "1.1.0",
"deepmerge": "4.2.2",
"downshift": "^5.2.4",
"downshift": "5.2.4",
"klona": "1.1.1",
"nanoid": "3.1.4",
"polished": "3.6.1",
Expand Down
52 changes: 32 additions & 20 deletions src/components/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function isOptionSelected(options, value) {
const DEFAULT_PROPS = {
hideLabel: false,
color: InternalDropdown.DEFAULT_PROPS.color,
renderPlaceholder: InternalDropdown.DEFAULT_PROPS.renderPlaceholder,
placeholderComponent: InternalDropdown.DEFAULT_PROPS.placeholderComponent,
disabled: false,
validate: (value, { isEmpty }) => {
if (isEmpty(value)) {
Expand All @@ -42,10 +42,10 @@ function Dropdown(props) {
const mergedProps = mergeProps(props, DEFAULT_PROPS, inheritedProps, {
hideLabel: (hideLabel) => typeof hideLabel === "boolean",
color: (color) => COLORS.includes(color),
renderPlaceholder: (renderPlaceholder) =>
typeof renderPlaceholder === "function",
renderOption: (renderOption) => typeof renderOption === "function",
placeholderComponent: (placeholderComponent) =>
typeof placeholderComponent === "function",
optionToString: (optionToString) => typeof optionToString === "function",
optionComponent: (optionComponent) => typeof optionComponent === "function",
helpText: (helpText) => typeof helpText === "string",
disabled: (disabled) => typeof disabled === "boolean",
options: (options) => areDropdownOptionsValid(options),
Expand All @@ -55,9 +55,9 @@ function Dropdown(props) {
color,
label,
hideLabel,
renderPlaceholder,
renderOption,
placeholderComponent,
optionToString,
optionComponent,
options,
helpText,
disabled,
Expand Down Expand Up @@ -108,9 +108,9 @@ function Dropdown(props) {
const {
isOpen,
selectedItem: selectedOption,
highlightedIndex,
getToggleButtonProps,
getMenuProps,
highlightedIndex,
getItemProps,
} = useSelect({
items: options,
Expand All @@ -125,15 +125,27 @@ function Dropdown(props) {
});
const isValid = !hasErrors;
const describedBy = helpText || hasErrors ? auxId : null;
const toggleButtonProps = getToggleButtonProps({
onFocus,
onBlur,
onChange,
disabled,
"aria-invalid": isValid ? null : "true",
"aria-describedby": describedBy,
ref: buttonRef,
});
const toggleButtonProps = useMemo(
() =>
getToggleButtonProps({
onFocus,
onBlur,
onChange,
disabled,
"aria-invalid": isValid ? null : "true",
"aria-describedby": describedBy,
ref: buttonRef,
}),
[
getToggleButtonProps,
onFocus,
onBlur,
onChange,
disabled,
isValid,
describedBy,
]
);
const maxHeightProps = useAllResponsiveProps(props, "maxHeight");

return (
Expand All @@ -151,8 +163,8 @@ function Dropdown(props) {
<InternalDropdown
name={name}
color={color}
renderPlaceholder={renderPlaceholder}
renderOption={renderOption}
placeholderComponent={placeholderComponent}
optionComponent={optionComponent}
options={options}
selectedOption={selectedOption}
isOpen={isOpen}
Expand All @@ -175,9 +187,9 @@ Dropdown.propTypes = {
label: PropTypes.string.isRequired,
hideLabel: PropTypes.bool,
color: PropTypes.oneOf(COLORS),
renderPlaceholder: PropTypes.func,
renderOption: PropTypes.func.isRequired,
placeholderComponent: PropTypes.func,
optionToString: PropTypes.func.isRequired,
optionComponent: PropTypes.func.isRequired,
options: PropTypes.arrayOf(
PropTypes.shape({
data: PropTypes.object.isRequired,
Expand Down
136 changes: 136 additions & 0 deletions src/components/Dropdown.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import React from "react";
import { render, screen, userEvent } from "../utils/test";
import "@testing-library/jest-dom/extend-expect";
import Form from "./Form";
import Text from "./Text";
import Dropdown from "./Dropdown";
import Container from "./Container";

const movieOptions = [
{
data: {
name: "Movie 1",
},
value: "movie-1",
},
{
data: {
name: "Movie 2",
},
value: "movie-2",
},
{
data: {
name: "Movie 3",
},
value: "movie-3",
},
];

/* eslint-disable react/prop-types */
function MovieOption({ data }) {
return <Text>{data.name}</Text>;
}
/* eslint-enable react/prop-types */

function movieOptionToString({ data }) {
return data.name;
}

const initialValues = {
movie: "",
};

function FormWithDropdown(props) {
return (
<Form initialValues={initialValues}>
<Dropdown
name="movie"
label="Movie"
options={movieOptions}
optionToString={movieOptionToString}
optionComponent={MovieOption}
{...props}
/>
</Form>
);
}

function sleep(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}

describe("Dropdown", () => {
it("renders the label and options are not visible", () => {
render(<FormWithDropdown />);

const label = screen.getByText("Movie");
const button = screen.getByRole("button", { name: /Please select/ });
const buttonId = button.getAttribute("id");

expect(buttonId).toBeTruthy();
expect(label).toHaveAttribute("for", buttonId);

expect(screen.queryByText("Movie 1")).not.toBeInTheDocument();
expect(screen.queryByText("Movie 2")).not.toBeInTheDocument();
expect(screen.queryByText("Movie 3")).not.toBeInTheDocument();
});

it("reveals options", async () => {
render(<FormWithDropdown />);

const button = screen.getByRole("button", { name: /Please select/ });

userEvent.click(button);

expect(screen.queryByText("Movie 1")).toBeInTheDocument();
expect(screen.queryByText("Movie 2")).toBeInTheDocument();
expect(screen.queryByText("Movie 3")).toBeInTheDocument();
});

it("hides options", async () => {
render(<FormWithDropdown />);

const button = screen.getByRole("button", { name: /Please select/ });

// Open
userEvent.click(button);

await sleep(0); // Without this, the test passes when it should fail.

// Close
userEvent.click(button);

expect(screen.queryByText("Movie 1")).not.toBeInTheDocument();
expect(screen.queryByText("Movie 2")).not.toBeInTheDocument();
expect(screen.queryByText("Movie 3")).not.toBeInTheDocument();
});

it("hides the label", () => {
render(<FormWithDropdown hideLabel />);

expect(screen.getByText("Movie")).toBeVisuallyHidden();
});

it("inside dark container", () => {
render(
<Container bg="grey.t05">
<FormWithDropdown />
</Container>
);
const button = screen.getByRole("button", { name: /Please select/ });

expect(button).toHaveStyle(`
background-color: #ffffff;
`);
});

it("with testId", () => {
const { container } = render(<FormWithDropdown testId="my-dropdown" />);

expect(container.querySelector("form").firstChild).toHaveAttribute(
"data-testid",
"my-dropdown"
);
});
});
24 changes: 16 additions & 8 deletions src/components/internal/InternalDropdown.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import React from "react";
import PropTypes from "prop-types";
import { Icon } from "..";
import { Icon, Text } from "..";
import useTheme from "../../hooks/useTheme";
import { responsiveMaxHeightType } from "../../hooks/useResponsiveProp";
import useResponsivePropsCSS from "../../hooks/useResponsivePropsCSS";
import { responsiveSize } from "../../utils/css";

const COLORS = ["grey.t05", "white"];

function DropdownDefaultPlaceholder() {
return <Text>Please select</Text>;
}

const DEFAULT_PROPS = {
color: "grey.t05",
renderPlaceholder: () => "Please select",
placeholderComponent: DropdownDefaultPlaceholder,
disabled: false,
isValid: true,
maxHeight: "600",
Expand All @@ -27,8 +31,8 @@ function InternalDropdown(_props) {
name,
parentName,
color,
renderPlaceholder,
renderOption,
placeholderComponent: PlaceholderComponent,
optionComponent: OptionComponent,
options,
selectedOption,
isOpen,
Expand Down Expand Up @@ -70,7 +74,11 @@ function InternalDropdown(_props) {
{...toggleButtonProps}
>
<div css={theme.dropdownButtonContent}>
{selectedOption ? renderOption(selectedOption) : renderPlaceholder()}
{selectedOption ? (
<OptionComponent {...selectedOption} />
) : (
<PlaceholderComponent />
)}
</div>
<div css={theme.dropdownButtonChevron}>
<Icon name="triangle-down" color="black" />
Expand All @@ -95,7 +103,7 @@ function InternalDropdown(_props) {
{...getItemProps({ item: option, index })}
key={index}
>
{renderOption(option)}
<OptionComponent {...option} />
</li>
))}
</ul>
Expand All @@ -107,8 +115,8 @@ InternalDropdown.propTypes = {
name: PropTypes.string.isRequired,
parentName: PropTypes.string, // While Dropdown doesn't pass a parentName, a composite component that contains an InternalDropdown would.
color: PropTypes.oneOf(COLORS),
renderPlaceholder: PropTypes.func,
renderOption: PropTypes.func.isRequired,
PlaceholderComponent: PropTypes.func,
optionComponent: PropTypes.func.isRequired,
options: PropTypes.arrayOf(
PropTypes.shape({
data: PropTypes.object.isRequired,
Expand Down
1 change: 0 additions & 1 deletion src/themes/default/dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export default (theme) => ({
margin: 0,
border: 0,
borderRadius: theme.radii[0],
...theme.textStyles.body1,
...theme.focusStyles.focusVisible,
},
dropdownButtonPlaceholder: {
Expand Down
Loading

1 comment on commit 0528be3

@vercel
Copy link

@vercel vercel bot commented on 0528be3 May 7, 2020

Choose a reason for hiding this comment

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

Please sign in to comment.