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

fix(component): SHIPPING-1526 adds description to Select & MultiSelect #378

Merged
merged 1 commit into from
Apr 24, 2020

Conversation

agchan12
Copy link
Contributor

@agchan12 agchan12 commented Apr 21, 2020

What?

Add description prop to Select dropdown

Why?

We support the description prop, but typescript doesn't recognise it as a prop

Testing/Proof

Select
Screen Shot 2020-04-24 at 2 50 54 pm

MultiSelect
Screen Shot 2020-04-24 at 2 51 05 pm

ping @deini @animesh1987 @chanceaclark

@agchan12 agchan12 requested review from jorgemoya and a team as code owners April 21, 2020 07:36
@@ -20,6 +20,7 @@ import { SelectAction, SelectOption, SelectOptionGroup, SelectProps } from './ty
export const Select = typedMemo(
<T extends any>({
action,
description,
Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't need to destructure this, since it gets passed into the Input via rest.

@@ -6,6 +6,7 @@ import { ListItemProps } from '../List/Item';

interface BaseSelect extends Omit<React.HTMLAttributes<HTMLInputElement>, 'children'> {
action?: SelectAction;
description?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can actually accept a string | FormControlDescription. See our Input implements it.

@chanceaclark
Copy link
Contributor

Just remembered, we probably want this for MultiSelect's too.

@jorgemoya
Copy link
Contributor

Yeah, if we can get this for MultiSelect that'd be 💯

@agchan12 agchan12 changed the title fix(component): adds description to Select component fix(component): SHIPPING-1526 adds description to Select & MultiSelect Apr 24, 2020
@agchan12 agchan12 force-pushed the SELECT-BRANCH branch 3 times, most recently from b1ce68e to efa27cd Compare April 24, 2020 07:57
Copy link
Contributor

@jorgemoya jorgemoya left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@deini deini merged commit b66c733 into bigcommerce:master Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants