[Chip] adding 'rounded' property to chip component #37576

cam-cole · 2023-06-12T16:18:22Z

I have followed (at least) the PR section of the contributing guide.

Resolves: #22471

mui-bot · 2023-06-12T16:21:29Z

Netlify deploy preview

docs/data/material/components/chips/chips.md

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against b827001

siriwatknp · 2023-06-14T02:39:37Z

@cam-cole Thanks for working on it. Can you link to the issue that this PR will fix?

cam-cole · 2023-06-14T03:21:06Z

@siriwatknp No problem! The issue is #22471

mnajdova

The implementation is not exactly what the issue is about. I would recommend instead adding the size property, for allowing denser chips. All chips are rounded right now. We can also link this issue as "related to" instead of "fixes" as there are multiple concepts described in the issue.

cam-cole · 2023-06-21T15:00:17Z

@mnajdova Thanks for the feedback! However, I believe my implementation hits on one of the three pieces within the issue in that it allows users to have the chips be rounded instead of circular by adding the rounded prop. Let me know if you think I am missing something here or you have suggestions.

I also asked within the issue for guidance on what other colors are wanted to be supported for this component but haven't heard anything so far.

Also, do you have any guidance for allowing denser chips in what it's wanted to look like? It already seems that the having the size prop as small makes for a denser chip.

Thank you for any help and guidance, I've enjoyed starting to contribute to MUI!

DiegoAndai · 2023-06-22T15:28:12Z

Hey @cam-cole, thanks for the contribution! 😊 🎉

Regarding #22471 proposals:

Broaden the supported colors: The current flexibility using the color prop is enough, which allows for seven preset colors as well as colors defined by extending the palette. The sx prop provides even more flexibility.
Provide a mode with more density: The current small size is enough.
Allow changing the shape from circular to rounded: This PR adds support for it.

So to merge this PR we should:

Extend the color example in the docs for using either the theme extension and the sx prop to create chips with other colors, something like this: https://codesandbox.io/s/broader-color-chips-tvstqy?file=/demo.tsx.
Add docs explaining the rounded prop. It should be a section in the Chip documentation, you can follow the color and size sections for guidance. We could also add it to the Chip playground.
Change rounded to default to false, as it should be opt-in. The circular Chip should be the default still.

Besides that, I leave a couple of code improvements in separate comments. Also remember to run yarn docs:api to update the API documentation (see contributing guide).

And with that, this PR would close #22471 🚀

DiegoAndai · 2023-06-22T15:32:08Z

packages/mui-material/src/Chip/Chip.js

@@ -172,6 +176,9 @@ const ChipRoot = styled('div', {
      ...(ownerState.size === 'small' && {
        height: 24,
      }),
+      ...(ownerState.rounded === true && {


Suggested change

...(ownerState.rounded === true && {

...(ownerState.rounded && {

DiegoAndai · 2023-06-22T15:39:31Z

packages/mui-material/src/Chip/Chip.js

@@ -337,6 +344,7 @@ const Chip = React.forwardRef(function Chip(inProps, ref) {
    onDelete,
    onKeyDown,
    onKeyUp,
+    rounded,


Suggested change

rounded,

rounded = false,

just passing by, why do you need a false default value? (I think you don't)

DiegoAndai · 2023-06-22T15:42:31Z

packages/mui-material/src/Chip/Chip.d.ts

@@ -72,6 +72,11 @@ export interface ChipTypeMap<P = {}, D extends React.ElementType = 'div'> {
     * @default 'medium'
     */
    size?: OverridableStringUnion<'small' | 'medium', ChipPropsSizeOverrides>;
+    /**
+     * If 'true' the component will be rounded instead of circular
+     * @default true


Suggested change

* @default true

* @default false

DiegoAndai · 2023-06-22T15:44:40Z

packages/mui-material/src/Chip/Chip.js

@@ -63,6 +66,7 @@ const ChipRoot = styled('div', {
        [`& .${chipClasses.deleteIcon}`]:
          styles[`deleteIcon${capitalize(variant)}Color${capitalize(color)}`],
      },
+      { [`& .${chipClasses.rounded}`]: styles.rounded },


Suggested change

{ [`& .${chipClasses.rounded}`]: styles.rounded },

rounded && styles.rounded

(Will need to destructure rounded from ownerState in line 53 above)

DiegoAndai · 2023-10-05T13:00:15Z

Hey @cam-cole! Will you be able to continue working on this PR? Otherwise, I will try to finish it.

We'll need to make some changes to accommodate the migration to Material You better.

…xtension or sx prop

cam-cole · 2023-10-16T14:09:37Z

Hey @DiegoAndai, I just pushed a new commit addressing the comments you made. I believe I have everything complete!

Signed-off-by: Cameron Cole <[email protected]>

DiegoAndai · 2023-10-17T18:40:47Z

Hey @cam-cole, thanks!

We've been discussing this change with the MUI core team and decided that we shouldn't add the rounded property for two reasons:

We're implementing Material You for Material UI v6, and this prop would make the migration harder as Material You has rounded chips by default. We would introduce a prop that would probably be removed or deprecated soon.
It's a prop to control a single CSS property. It can be replaced by using the sx prop (e.g., sx={{ borderRadius: '8px' }}) or a theme style override.

We could instead add an example of how to achieve what was stated in #22471, for example: https://codesandbox.io/s/22471-docs-example-xjwfst

That said, thanks for working on this, and if you're interested in adding the proposed example to the docs I would be more than happy to review that PR.

camcole065 added 2 commits June 12, 2023 11:06

added rounded property to chip component

4475309

refactoring changes

f38bd04

running yarn prettier

bac0882

cam-cole changed the title ~~[Chip] adding 'rounded' property to chip component~~ [Chip] adding 'rounded' property to chip component [component: chip] Jun 12, 2023

cam-cole changed the title ~~[Chip] adding 'rounded' property to chip component [component: chip]~~ [Chip] adding 'rounded' property to chip component Jun 12, 2023

zannager added the component: chip This is the name of the generic UI component, not the React module! label Jun 13, 2023

zannager requested a review from siriwatknp June 13, 2023 07:29

mnajdova requested a review from DiegoAndai June 21, 2023 08:52

mnajdova added the package: material-ui Specific to @mui/material label Jun 21, 2023

mnajdova reviewed Jun 21, 2023

View reviewed changes

DiegoAndai self-assigned this Jun 22, 2023

DiegoAndai reviewed Jun 22, 2023

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 30, 2023

DiegoAndai added the Port to v6.x label Sep 20, 2023

This was referenced Sep 20, 2023

[Chip][material-next] Add Material You Chip component #38927

Merged

[Chip][material-next] Add Chip component #38024

Closed

added docs for rounded prop and extended color example to use theme e…

ec7b22e

…xtension or sx prop

camcole065 and others added 2 commits October 16, 2023 11:04

fix import error

83d2452

Merge branch 'master' into support-simple-badges

3d7a5db

Signed-off-by: Cameron Cole <[email protected]>

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 16, 2023

camcole065 added 3 commits October 16, 2023 13:26

run prettier

08b802d

update prop types

190f869

update prop types

b827001

cam-cole closed this Oct 17, 2023

cam-cole deleted the support-simple-badges branch October 17, 2023 18:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Chip] adding 'rounded' property to chip component #37576

[Chip] adding 'rounded' property to chip component #37576

cam-cole commented Jun 12, 2023 •

edited

Loading

mui-bot commented Jun 12, 2023 •

edited

Loading

siriwatknp commented Jun 14, 2023

cam-cole commented Jun 14, 2023

mnajdova left a comment

cam-cole commented Jun 21, 2023

DiegoAndai commented Jun 22, 2023 •

edited

Loading

DiegoAndai Jun 22, 2023

DiegoAndai Jun 22, 2023

o-alexandrov Oct 14, 2023

DiegoAndai Jun 22, 2023

DiegoAndai Jun 22, 2023

DiegoAndai commented Oct 5, 2023

cam-cole commented Oct 16, 2023

DiegoAndai commented Oct 17, 2023

	...(ownerState.rounded === true && {
	...(ownerState.rounded && {

	{ [`& .${chipClasses.rounded}`]: styles.rounded },
	rounded && styles.rounded

[Chip] adding 'rounded' property to chip component #37576

[Chip] adding 'rounded' property to chip component #37576

Conversation

cam-cole commented Jun 12, 2023 • edited Loading

mui-bot commented Jun 12, 2023 • edited Loading

Netlify deploy preview

Bundle size report

siriwatknp commented Jun 14, 2023

cam-cole commented Jun 14, 2023

mnajdova left a comment

Choose a reason for hiding this comment

cam-cole commented Jun 21, 2023

DiegoAndai commented Jun 22, 2023 • edited Loading

DiegoAndai Jun 22, 2023

Choose a reason for hiding this comment

DiegoAndai Jun 22, 2023

Choose a reason for hiding this comment

o-alexandrov Oct 14, 2023

Choose a reason for hiding this comment

DiegoAndai Jun 22, 2023

Choose a reason for hiding this comment

DiegoAndai Jun 22, 2023

Choose a reason for hiding this comment

DiegoAndai commented Oct 5, 2023

cam-cole commented Oct 16, 2023

DiegoAndai commented Oct 17, 2023

cam-cole commented Jun 12, 2023 •

edited

Loading

mui-bot commented Jun 12, 2023 •

edited

Loading

DiegoAndai commented Jun 22, 2023 •

edited

Loading