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

[Chip] variant default value #18914

Closed
1 task done
LorenzHenk opened this issue Dec 18, 2019 · 2 comments · Fixed by #22683
Closed
1 task done

[Chip] variant default value #18914

LorenzHenk opened this issue Dec 18, 2019 · 2 comments · Fixed by #22683
Labels
breaking change component: chip This is the name of the generic UI component, not the React module!
Milestone

Comments

@LorenzHenk
Copy link

The Chip variant default value default does not follow the general api of standard.

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Many components use the variant value standard as default value (e.g. TextField, Tabs, Badge).

For some reason, the Chip component uses the value default instead (types, PropTypes).

I think this should be changed from default to standard or filled.

Fixing this would be a breaking change without much gain.

Is it worth the trouble?
Should this be normalized somewhere, so it's less likely to happen again?

Additional context

The PR, that introduced the variant prop: #12680

@mbrookes mbrookes added the component: chip This is the name of the generic UI component, not the React module! label Dec 18, 2019
@oliviertassinari
Copy link
Member

@mbrookes A v5 breaking change?

@eps1lon
Copy link
Member

eps1lon commented Dec 27, 2019

This is loosely related to #13028

I really like standard as opposed to the current tautology of 'default' being the default value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: chip This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants