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

Merged
merged 6 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
63 changes: 50 additions & 13 deletions __docs__/wonder-blocks-dropdown/combobox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import {Meta, StoryObj} from "@storybook/react";
import {expect, userEvent, within} from "@storybook/test";
import {StyleSheet} from "aphrodite";
import * as React from "react";
import magnifyingGlassIcon from "@phosphor-icons/core/regular/magnifying-glass.svg";
import magnifyingGlassIcon from "@phosphor-icons/core/bold/magnifying-glass-bold.svg";

import {LabelLarge, LabelMedium} from "@khanacademy/wonder-blocks-typography";
import {color, spacing} from "@khanacademy/wonder-blocks-tokens";
import {color, semanticColor, 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";
Expand Down Expand Up @@ -433,29 +433,73 @@ export const Error: Story = {
};

/**
* With start icon, you can customize the icon that appears at the beginning of
* With `startIcon`, 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.
* **NOTE:** When `startIcon` is set, we set some default values for the icon:
* - `size`: "small"
* - `color`: `semanticColor.icon.default`
*
* You can customize the size and color of the icon by passing the `size` and
* `color` props to the `PhosphorIcon` component.
*/
export const StartIcon: Story = {
render: function Render(args: PropsFor<typeof Combobox>) {
const [_, updateArgs] = useArgs();

return (
<View style={{gap: spacing.medium_16}}>
<LabelMedium>Default:</LabelMedium>
<LabelMedium>With default size and color:</LabelMedium>
<Combobox
{...args}
startIcon={
<PhosphorIcon icon={magnifyingGlassIcon} size="small" />
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: We can remove size="small" here so we can confirm that the default size is being applied!

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! I'll fix it before landing.

}
onChange={(newValue) => {
updateArgs({value: newValue});
action("onChange")(newValue);
}}
/>
<LabelMedium>Disabled:</LabelMedium>
<LabelMedium>With custom size:</LabelMedium>
<Combobox
{...args}
startIcon={
<PhosphorIcon
icon={magnifyingGlassIcon}
size="medium"
/>
}
onChange={(newValue) => {
updateArgs({value: newValue});
action("onChange")(newValue);
}}
/>
<LabelMedium>With custom color:</LabelMedium>
<Combobox
{...args}
startIcon={
<PhosphorIcon
icon={magnifyingGlassIcon}
size="small"
color={semanticColor.icon.action}
/>
}
onChange={(newValue) => {
updateArgs({value: newValue});
action("onChange")(newValue);
}}
/>
<LabelMedium>Disabled (overrides color prop):</LabelMedium>
<Combobox
{...args}
startIcon={
<PhosphorIcon
icon={magnifyingGlassIcon}
size="small"
color={semanticColor.icon.action}
/>
}
disabled={true}
onChange={(newValue) => {
updateArgs({value: newValue});
Expand All @@ -467,12 +511,5 @@ export const StartIcon: Story = {
},
args: {
children: items,
startIcon: (
<PhosphorIcon
icon={magnifyingGlassIcon}
size="medium"
color={color.offBlack64}
/>
),
},
};
23 changes: 14 additions & 9 deletions packages/wonder-blocks-dropdown/src/components/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import {
} from "@khanacademy/wonder-blocks-core";
import {TextField} from "@khanacademy/wonder-blocks-form";
import IconButton from "@khanacademy/wonder-blocks-icon-button";
import {border, color, spacing} from "@khanacademy/wonder-blocks-tokens";
import {
border,
color,
semanticColor,
spacing,
} from "@khanacademy/wonder-blocks-tokens";

import {DetailCell} from "@khanacademy/wonder-blocks-cell";
import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon";
Expand Down Expand Up @@ -495,10 +500,16 @@ export default function Combobox({
}

const startIconElement = React.cloneElement(startIcon, {
// Provide a default size for the icon that can be overridden by
// the consumer.
size: "small",
...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
// Override the disabled state of the icon to match the combobox
// state.
color: disabled ? color.offBlack32 : startIcon.props.color,
color: disabled
? color.offBlack32
: // Use the color passed in, otherwise use the default color.
startIcon.props.color ?? semanticColor.icon.primary,
} as Partial<React.ReactElement<React.ComponentProps<typeof PhosphorIcon>>>);

return <View style={styles.iconWrapper}>{startIconElement}</View>;
Expand Down Expand Up @@ -711,12 +722,6 @@ const styles = StyleSheet.create({
border: `1px solid ${color.red}`,
color: color.offBlack,
},
/**
* Start icon styles
*/
iconDisabled: {
color: color.offBlack32,
},
/**
* Combobox input styles
*/
Expand Down