-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[blog] Improve the width of the layout #33706
[blog] Improve the width of the layout #33706
Conversation
@@ -295,4 +305,4 @@ if (process.env.NODE_ENV !== 'production') { | |||
TopLayoutBlog.propTypes = exactProp(TopLayoutBlog.propTypes); | |||
} | |||
|
|||
export default styled(TopLayoutBlog)(styles); | |||
export default TopLayoutBlog; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BrandingProvider
theme was not applied at this point yet. theme.spacing
would be 8 vs. 10 pixels based on either the context is provided or not.
@@ -456,16 +456,6 @@ export function getThemedComponents(theme: Theme): { components: Theme['componen | |||
}, | |||
}, | |||
}, | |||
MuiContainer: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This contradicts the default values we have for the <Container>
that we have in MUI System as best practices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a small margin-bottom comment there.
Co-authored-by: danilo leal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if you can capture the screenshots for before
and after
.
@siriwatknp I have tried, but it doesn't reproduce the experience well. The best is to open:
and compare with a screen width of 1000px and 1400px |
I believe that the current width on medium size devices is buggy. The had overlooked the max width logic.
Implement #33670 (comment). I have also updated https://www.notion.so/mui-org/Blog-247ec2bff5fa46e799ef06a693c94917#b51c4255803840479e39cf2861af7ed8.
Preview: https://deploy-preview-33706--material-ui.netlify.app/blog/aggregation-functions/