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.

Sign up for GitHub

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

Jump to bottom

[examples] Add example component using styled api #29434

Closed
wants to merge 1 commit into from
Closed

[examples] Add example component using styled api #29434

wants to merge 1 commit into from

Conversation

amaralc
Copy link

@amaralc amaralc commented Oct 31, 2021 •
edited
Loading

Relates to #29428.

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 31, 2021 •
edited
Loading

No bundle size changes

Generated by 🚫 dangerJS against 3f82ac0

@oliviertassinari oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Oct 31, 2021
sebkasanzew
sebkasanzew reviewed Oct 31, 2021
View reviewed changes
examples/nextjs-with-typescript/pages/index.tsx
@@ -2,13 +2,14 @@ import * as React from 'react';
import Container from '@mui/material/Container';
Copy link

Choose a reason for hiding this comment

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

The Circle CI linter workflow is complaining about this now unused dependency.

mnajdova
mnajdova requested changes Nov 1, 2021
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

It doesn't look like this PR will fix the #29428 It is a completely different problem.

@amaralc amaralc mentioned this pull request Nov 1, 2021
Closed
2 tasks
@amaralc
Copy link
Author

amaralc commented Nov 1, 2021

@mnajdova, maybe I can clarify the PR's intention, which was to add the usage of the styled API, and avoid the added complexity of using the makeStyles API.

What do you think?

The className mismatch issue does not happen when using the styled API, for some reason.

@mnajdova
Copy link
Member

mnajdova commented Nov 2, 2021

The className mismatch issue does not happen when using the styled API, for some reason.

The reason was described in #29428. We intentionally don't document how to configure JSS in v5 as it is not the engine that we use and the @mui/styles API is marked as legacy. I don't have anything against the change, all for it, but we should update all examples in that case and remove the "Fixes" claim in the PR description :)

@amaralc
Copy link
Author

amaralc commented Nov 4, 2021

"Fixes" claim, removed from the description.

@mnajdova
Copy link
Member

mnajdova commented Nov 5, 2021

@amaralc would you like to add this to the other examples too? To keep them consistent

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 2, 2021
@mnajdova mnajdova closed this Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebkasanzew sebkasanzew sebkasanzew left review comments

@mnajdova mnajdova mnajdova requested changes

Assignees
No one assigned
Labels
PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@amaralc @mui-pr-bot @mnajdova @sebkasanzew @oliviertassinari