[Badge] Create unstyled component & move to emotion #23745

mnajdova · 2020-11-27T17:34:55Z

This PR creates an unstyled version of the Badge component, and changes the styling engine to @material-ui/styled-engine.

Steps necessary for making the changes:

The Badge's component logic should be moved into a new BadgeUnstyled component. In the BadgeUnstyled component there should not be any styles associated with the component (these will go into the core Badge component). Additional things we need to do:

Add a mapping for the classes as done here.
Use the components & componentsProps properties for the slots inside the component, instead of the component prop and other potential props associated with different slots.
Update the API to contain the new components & componentsProps props and remove the previous component prop.

The styling props logic should be removed from the Unstyled component and moved to the core component.
Export a badgeClasses const containing all utility classes under one object.
Update the core Badge component to simply use the BadgeUnstyled component, providing styled slots for each of the slots of the component. With this conversion, we should update the old JSS styles to the new styled() syntax, and split them into different slots components. Additional things we need to do:

Update the typings to extend the typings of the Unstyled version.
Add an overridesResolver method that should resolve how the theme overrides will be mapped to the new styles.
Update the tests to use the badgeClasses when testing if the correct classes are being applied.
Replace the describeConformance with describeConformanceV5.

Update the docs examples for the component to use the v5 overrides proposal (using the styled() API from @material-ui/core/styles) or the sx prop where possible.
Add demo of how to use the BadgeUnstyled component

mui-pr-bot · 2020-11-27T17:37:33Z

@material-ui/core: parsed: +0.91% , gzip: +0.81%

Details of bundle changes

Generated by 🚫 dangerJS against 733c140

mnajdova · 2020-11-30T11:34:31Z

packages/material-ui/src/Badge/Badge.js


-  let displayValue = '';
+const BadgeBadge = styled(


This looks really ugly :( I wonder if we should rename the BadgeRoot to BadgeWrapper and the BadgeBadge to BadgeRoot

With this we currently have a bug, where the theme's variants go to the Root instead of the badge component, which is wrong with this component..

For example, adding a dashed variants results in the following:

Which in my opinion is wrong (it should affect only the badge itself)

From my perspective, the current names are as good as they could get. I think that it surfaces that we could change the composition API of the component. The current approach is a bit weird.

We could also leave this for a follow-up PR so that we don't pollute this one

I will leave this comment open and we can resolve it after this one is merged, so we will avoid unnecessary changes as part of this PR. Would that work?

💯, the point around the weird API isn't specific to unstyled. It's more about how <Badge> could be a <BadgeContainer>, it's not really a badge as developers can find in other UI libraries.

oliviertassinari

Do we miss an unstyled customization demo? Maybe reproduce Ant Design?

packages/material-ui/src/styles/experimentalStyled.js

packages/material-ui-unstyled/src/BadgeUnstyled/BadgeUnstyled.test.js

oliviertassinari · 2020-11-30T11:52:25Z

packages/material-ui/src/Badge/Badge.js


-  let displayValue = '';
+const BadgeBadge = styled(


From my perspective, the current names are as good as they could get. I think that it surfaces that we could change the composition API of the component. The current approach is a bit weird.

mnajdova · 2020-11-30T12:57:17Z

Do we miss an unstyled customization demo? Maybe reproduce Ant Design?

Done, let me know if you think we should somehow extend it

mnajdova · 2020-11-30T17:21:15Z

There is an issue on RTL

Looks like the styles are not transformed at all.. Will need to debug to see what is the issue...

Should the class name be .css-gd7jph-MuiBadge-root and the styles only applied once?

We can rename the class 👍 , don't have a strong opinion. I don't understand what you are referring to with the styles being applied only once. For each props combination we have one classname. If we have color: "primary", we cannot for example apply the classname you have on the screenshot as the background color would need to be different. I am probably not understanding your point on this one :)

test/utils/describeConformance.js

oliviertassinari · 2020-12-07T15:18:31Z

packages/material-ui/src/Badge/Badge.js

@@ -317,6 +342,10 @@ Badge.propTypes = {
   * @default false
   */
  showZero: PropTypes.bool,
+  /**
+   * The system prop that allows defining system overrides as well as additional CSS styles.


A note for later, I think that it would be great to add a link to the system page of the documentation as we do for the classes prop:

It should probably done in the documentation generator to be done once.

Makes sense, we can add it, will leave the comment open for visibility

packages/material-ui/src/Badge/Badge.js

oliviertassinari · 2020-12-07T15:21:22Z

packages/material-ui/src/Badge/Badge.js

+const BadgeRoot = styled(
+  'span',
+  {},
+  { muiName: 'MuiBadge', overridesResolver },


Would this work?

Suggested change

{ muiName: 'MuiBadge', overridesResolver },

{ muiName: 'MuiBadge-root', overridesResolver },

This is used as key for the overrides inside the theme, not sure we want to actually change it. Should we maybe add a new prop for the class suffix we want to have?

I wouldn't be against adding a second option to isolate the theming and CSS class name tip use cases. I doubt that developers should be able to override MuiBadge-badge with the theme. However, it's likely a topic for a different effort.

I've tried to work around this, the problem is that whatever we pass in the label, will be used for both the component display name and classname suffix. It would feel awkward seeing component names like MuiBadge-root. This is currently on all slots components at the moment. I would even suggest we use as muiName: 'MuiBadge', MuiSlider, MuiSliderTrack etc, to have better display names on the components. The utility classes are anyway added to the components, so people can easily copy them.

Let me know what do you think and I'll update the migrated dcomponents.

My motivation for the proposal was to mitigate this issue #23841 (review). to make it easier to match the styles seen in the dev tools with the global class name to use to customize them.

Got ya, created #23964 hopefully it will help to resolve the confusion

vercel · 2020-12-10T10:37:08Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mui-org/material-ui/gdbvtf5a5
✅ Preview: https://material-ui-git-feat-convert-badge-to-v5.mui-org.vercel.app

mnajdova added 2 commits November 27, 2020 18:03

wip

1ac862b

fixed typo in class prop

cfec629

mnajdova added 9 commits November 27, 2020 19:40

prettier, proptypes & docs:api

aa327db

fixed test & build

53b180c

isRtl fix

d15b438

framer fix

41ee75b

fixed ts issue

8ba3c6d

build issues

61131b3

prettier

69d3184

fixes

8907799

fixed invisible issue

805e893

mnajdova changed the title ~~[WIP][Badge] Create unstyled component & move to emotion~~ [Badge] Create unstyled component & move to emotion Nov 30, 2020

mnajdova commented Nov 30, 2020

View reviewed changes

mnajdova marked this pull request as ready for review November 30, 2020 11:41

mnajdova requested a review from oliviertassinari November 30, 2020 11:41

oliviertassinari reviewed Nov 30, 2020

View reviewed changes

Updated tests & created unstyled demo

253aa80

mnajdova added 2 commits November 30, 2020 14:05

fixed lint errors

da8dfed

removed duplicated tests

a595544

oliviertassinari approved these changes Nov 30, 2020

View reviewed changes

mnajdova requested review from eps1lon and mbrookes November 30, 2020 15:27

This comment has been minimized.

Sign in to view

eps1lon reviewed Nov 30, 2020

View reviewed changes

test/utils/describeConformance.js Outdated Show resolved Hide resolved

fix rtl

e420878

mnajdova requested review from mbrookes, eps1lon and oliviertassinari December 7, 2020 12:38

oliviertassinari reviewed Dec 7, 2020

View reviewed changes

mnajdova added 2 commits December 7, 2020 17:20

addressed comments

a868f38

fixed overrides

2b2e218

oliviertassinari approved these changes Dec 7, 2020

View reviewed changes

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

mnajdova requested review from eps1lon and mbrookes and removed request for mbrookes and eps1lon December 9, 2020 10:31

Merge branch 'next' into feat/convert-badge-to-v5

75f54ce

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

oliviertassinari and others added 4 commits December 9, 2020 12:20

yarn docs:api

616f089

reverted some changes

199d7cf

revert more changes

2958c06

fixed unstyled test failing

3faeec9

vercel bot deployed to Preview December 10, 2020 10:37 View deployment

reverted unnecessary docs updates

3ae6bbe

vercel bot deployed to Preview December 10, 2020 10:48 View deployment

updated test

733c140

vercel bot deployed to Preview December 10, 2020 12:06 View deployment

mnajdova merged commit a4b6a96 into mui:next Dec 10, 2020

oliviertassinari mentioned this pull request Dec 10, 2020

[core] Fix failing CI on HEAD #23947

Merged

eps1lon mentioned this pull request Dec 11, 2020

[docs] Fix production deploy #23963

Merged

2 tasks

mnajdova mentioned this pull request Dec 11, 2020

[experimentalStyled] Add name and slot options #23964

Merged

mnajdova mentioned this pull request Jan 9, 2021

[Badge] Typescript Custom Variant does not work #24323

Closed

2 tasks

michaldudak mentioned this pull request Feb 27, 2024

Unstyled components progress mui/base-ui#10

Open

32 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Badge] Create unstyled component & move to emotion #23745

[Badge] Create unstyled component & move to emotion #23745

mnajdova commented Nov 27, 2020 •

edited

Loading

mui-pr-bot commented Nov 27, 2020 •

edited

Loading

mnajdova Nov 30, 2020

mnajdova Nov 30, 2020 •

edited

Loading

oliviertassinari Nov 30, 2020

mnajdova Nov 30, 2020

mnajdova Nov 30, 2020

oliviertassinari Nov 30, 2020

oliviertassinari left a comment •

edited

Loading

oliviertassinari Nov 30, 2020

mnajdova commented Nov 30, 2020

This comment has been minimized.

This comment has been minimized.

mnajdova commented Nov 30, 2020

This comment has been minimized.

oliviertassinari Dec 7, 2020

mnajdova Dec 7, 2020

oliviertassinari Dec 7, 2020

mnajdova Dec 7, 2020

oliviertassinari Dec 7, 2020 •

edited

Loading

mnajdova Dec 11, 2020

oliviertassinari Dec 11, 2020 •

edited

Loading

mnajdova Dec 11, 2020

vercel bot commented Dec 10, 2020 •

edited

Loading

	{ muiName: 'MuiBadge', overridesResolver },
	{ muiName: 'MuiBadge-root', overridesResolver },

[Badge] Create unstyled component & move to emotion #23745

[Badge] Create unstyled component & move to emotion #23745

Conversation

mnajdova commented Nov 27, 2020 • edited Loading

mui-pr-bot commented Nov 27, 2020 • edited Loading

Choose a reason for hiding this comment

mnajdova Nov 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnajdova commented Nov 30, 2020

This comment has been minimized.

This comment has been minimized.

mnajdova commented Nov 30, 2020

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Dec 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Dec 10, 2020 • edited Loading

mnajdova commented Nov 27, 2020 •

edited

Loading

mui-pr-bot commented Nov 27, 2020 •

edited

Loading

mnajdova Nov 30, 2020 •

edited

Loading

oliviertassinari left a comment •

edited

Loading

oliviertassinari Dec 7, 2020 •

edited

Loading

oliviertassinari Dec 11, 2020 •

edited

Loading

vercel bot commented Dec 10, 2020 •

edited

Loading