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

[Avatar] Use variants api #40324

Merged
merged 11 commits into from
Jan 17, 2024
Merged

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Dec 26, 2023

Note: I won't be adding documentation about this API yet, we can document it once we start with the migration guide from emotion to zero-runtime.

@mnajdova mnajdova added component: avatar This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Dec 26, 2023
@mui-bot
Copy link

mui-bot commented Dec 26, 2023

Netlify deploy preview

https://deploy-preview-40324--material-ui.netlify.app/

@material-ui/core: parsed: +0.04% , gzip: +0.08%
@material-ui/lab: parsed: +0.08% , gzip: +0.13%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 906693d

@mnajdova mnajdova marked this pull request as ready for review December 29, 2023 11:38
@@ -16,15 +16,10 @@ const Html = styled.html({
color: 'red',
});

const Body = styled.body({
Copy link
Member Author

@mnajdova mnajdova Jan 9, 2024

Choose a reason for hiding this comment

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

This leaking on other pages would complicate our testing, so I moved it to the home page only.

Comment on lines 6 to 9
import { styled } from '@mui/zero-runtime';
import { useThemeProps } from '@mui/material/styles';
import Person from '@mui/material/internal/svg-icons/Person';
import { getAvatarUtilityClass } from '@mui/material/Avatar';
Copy link
Member Author

Choose a reason for hiding this comment

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

Only these imports were updated.

@mnajdova
Copy link
Member Author

mnajdova commented Jan 9, 2024

@brijeshb42 I only had to remove the sx prop, everything else worked as expected 🎉

Copy link
Contributor

@brijeshb42 brijeshb42 left a comment

Choose a reason for hiding this comment

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

👏🏼

@mnajdova mnajdova merged commit 8e849ae into mui:master Jan 17, 2024
19 checks passed
@@ -5,12 +5,6 @@ const { experimental_extendTheme: extendTheme } = require('@mui/material/styles'

const theme = extendTheme();

theme.applyDarkStyles = function applyDarkStyles(obj) {

Choose a reason for hiding this comment

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

@mnajdova it appears that this change caused a breaking change in the Avatar component. Since when our team started consuming 5.15.6 we started to observe this error:
"TypeError: t.applyDarkStyles is not a function"

Copy link
Contributor

Choose a reason for hiding this comment

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

This should not happen as this deletion is not part of the core @mui/material package and the method has been added in material-ui here.

Choose a reason for hiding this comment

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

We experience same issue. Starting from version @mui/material@5.15.6 the Avatar component crashes with the message TypeError: t.applyDarkStyles is not a function, or TypeError: e.applyStyles is not a function for the later versions. We had to downgrade to @mui/material@5.15.5 in order to fix the problem.

Copy link

Choose a reason for hiding this comment

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

Is there any update on this issue ??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: avatar This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants