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

WB-1779: Add startIcon prop to Combobox #2364

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
5 changes: 5 additions & 0 deletions .changeset/old-pears-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-dropdown": minor
---

Add `startIcon` prop to Combobox
50 changes: 49 additions & 1 deletion __docs__/wonder-blocks-dropdown/combobox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import {Meta, StoryObj} from "@storybook/react";
import {expect, userEvent, within} from "@storybook/test";
import {StyleSheet} from "aphrodite";
import * as React from "react";
import {LabelLarge} from "@khanacademy/wonder-blocks-typography";
import magnifyingGlassIcon from "@phosphor-icons/core/regular/magnifying-glass.svg";

import {LabelLarge, LabelMedium} from "@khanacademy/wonder-blocks-typography";
import {color, spacing} from "@khanacademy/wonder-blocks-tokens";
import {Checkbox} from "@khanacademy/wonder-blocks-form";
import {Combobox, OptionItem} from "@khanacademy/wonder-blocks-dropdown";
import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon";
import {PropsFor, View} from "@khanacademy/wonder-blocks-core";
import {allProfilesWithPictures} from "./option-item-examples";

Expand Down Expand Up @@ -428,3 +431,48 @@ export const Error: Story = {
error: true,
},
};

/**
* With start icon, you can customize the icon that appears at the beginning of
* the Combobox. This is useful when you want to add a custom icon to the
* component.
*
* In this example, we show how this is done by setting the `startIcon` prop.
*/
export const StartIcon: Story = {
render: function Render(args: PropsFor<typeof Combobox>) {
const [_, updateArgs] = useArgs();

return (
<View style={{gap: spacing.medium_16}}>
<LabelMedium>Default:</LabelMedium>
<Combobox
{...args}
onChange={(newValue) => {
updateArgs({value: newValue});
action("onChange")(newValue);
}}
/>
<LabelMedium>Disabled:</LabelMedium>
<Combobox
{...args}
disabled={true}
onChange={(newValue) => {
updateArgs({value: newValue});
action("onChange")(newValue);
}}
/>
</View>
);
},
args: {
children: items,
startIcon: (
Copy link
Member

Choose a reason for hiding this comment

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

Let's consolidate on the magnifying icon styling! In SearchField, I updated the following to match the design more closely:

  • size=small
  • color=semanticColor.icon.primary
  • use the bold icon instead @phosphor-icons/core/bold/magnifying-glass-bold.svg

Let me know though if we prefer to stick with the existing styles, I can update SearchField to change it back to how it was! (I was assuming Figma was the source of truth for styling, but that may not be the case!)

Copy link
Member Author

@jandrade jandrade Nov 20, 2024

Choose a reason for hiding this comment

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

Sounds great! I hope GH includes the 💯 emoji in their msg reactions at some point.

To clarify after our last convo, size would be small right? (medium was for the dismiss icon).

Copy link
Member

Choose a reason for hiding this comment

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

The magnifying glass PhosphorIcon size will be small!

The clear/dismiss IconButton size is also small and the x icon is the regular icon weight!

They're both "small", but the PhosphorIcon renders a 16x16 icon and IconButton renders a 24x24 icon!

<PhosphorIcon
icon={magnifyingGlassIcon}
size="medium"
color={color.offBlack64}
/>
),
},
};
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import * as React from "react";
import {render, screen, waitFor} from "@testing-library/react";
import {RenderStateRoot} from "@khanacademy/wonder-blocks-core";
import {render, screen, waitFor} from "@testing-library/react";
import * as React from "react";
import magnifyingGlassIcon from "@phosphor-icons/core/regular/magnifying-glass.svg";

import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon";
import {PointerEventsCheckLevel, userEvent} from "@testing-library/user-event";
import Combobox from "../combobox";
import OptionItem from "../option-item";
import {defaultComboboxLabels} from "../../util/constants";
import {MaybeValueOrValues} from "../../util/types";
import Combobox from "../combobox";
import OptionItem from "../option-item";

const doRender = (element: React.ReactElement) => {
render(element, {wrapper: RenderStateRoot});
Expand Down Expand Up @@ -305,6 +307,31 @@ describe("Combobox", () => {
expect(screen.getByRole("combobox")).not.toHaveFocus();
});

it("should include an icon at the beginning of the combobox", () => {
// Arrange

// Act
doRender(
<Combobox
selectionType="single"
value=""
startIcon={
<PhosphorIcon
icon={magnifyingGlassIcon}
testId="start-icon"
/>
}
>
<OptionItem label="option 1" value="option1" />
<OptionItem label="option 2" value="option2" />
<OptionItem label="option 3" value="option3" />
</Combobox>,
);

// Assert
expect(screen.getByTestId("start-icon")).toBeInTheDocument();
});

describe("dismiss button", () => {
it("should clear the value when the user presses the clear button (x) via Mouse", async () => {
// Arrange
Expand Down
42 changes: 42 additions & 0 deletions packages/wonder-blocks-dropdown/src/components/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import IconButton from "@khanacademy/wonder-blocks-icon-button";
import {border, color, spacing} from "@khanacademy/wonder-blocks-tokens";

import {DetailCell} from "@khanacademy/wonder-blocks-cell";
import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon";
import {useListbox} from "../hooks/use-listbox";
import {useMultipleSelection} from "../hooks/use-multiple-selection";
import {
Expand Down Expand Up @@ -132,6 +133,13 @@ type Props = {
*/
// TODO(WB-1740): Add support to `inline` and `both` values.
autoComplete?: "none" | "list" | undefined;

/**
* An optional decorative icon to display at the start of the combobox.
*/
startIcon?: React.ReactElement<
React.ComponentProps<typeof PhosphorIcon>
> | null;
};

/**
Expand All @@ -158,6 +166,7 @@ export default function Combobox({
opened,
placeholder,
selectionType = "single",
startIcon,
testId,
value = "",
}: Props) {
Expand Down Expand Up @@ -477,6 +486,24 @@ export default function Combobox({
return [labelFromSelected];
}, [children, labelFromSelected, selected]);

/**
* Renders the start icon if provided.
*/
const maybeRenderStartIcon = () => {
if (!startIcon) {
return null;
}

const startIconElement = React.cloneElement(startIcon, {
...startIcon.props,
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: (no changes necessary) What do you think about providing a default size and color if the color and size props aren't defined on the startIcon? That way, teams could pass in the PhosphorIcon without having to worry about what color and size to provide, and they can if they need to override it!

Copy link
Member Author

Choose a reason for hiding this comment

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

Love it!

// Override the disable state of the icon to match the combobox
// state.
color: disabled ? color.offBlack32 : startIcon.props.color,
} as Partial<React.ReactElement<React.ComponentProps<typeof PhosphorIcon>>>);

return <View style={styles.iconWrapper}>{startIconElement}</View>;
};

const pillIdPrefix = id ? `${id}-pill-` : ids.get("pill");

const currentActiveDescendant = !openState
Expand Down Expand Up @@ -534,6 +561,8 @@ export default function Combobox({
removeSelectedLabel={labels.removeSelected}
/>
)}
{maybeRenderStartIcon()}

<TextField
id={ids.get("input")}
testId={testId}
Expand Down Expand Up @@ -682,6 +711,12 @@ const styles = StyleSheet.create({
border: `1px solid ${color.red}`,
color: color.offBlack,
},
/**
* Start icon styles
*/
iconDisabled: {
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think this isn't used!

color: color.offBlack32,
},
/**
* Combobox input styles
*/
Expand Down Expand Up @@ -739,4 +774,11 @@ const styles = StyleSheet.create({
// This is calculated based on the padding + width of the arrow button.
right: spacing.xLarge_32 + spacing.xSmall_8,
},
iconWrapper: {
padding: spacing.xxxSmall_4,
// View has a default minWidth of 0, which causes the label text
// to encroach on the icon when it needs to truncate. We can fix
// this by setting the minWidth to auto.
minWidth: "auto",
},
});