-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: Encourage response font sizes #357
Conversation
breakpoints: { | ||
values: { | ||
lg: 1280, | ||
md: 960, | ||
mobile: 720, | ||
sm: 720, | ||
xl: 1280, | ||
xs: 375, | ||
}, | ||
}, |
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.
I removed this because apparently it was never working?? And design for E4S have been using MUIs standard breakpoints.
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.
Huh, interesting 🤔
Do you know what was missing that caused these breakpoints to not "stick" at runtime?
@@ -169,10 +169,10 @@ export const components: ThemeOptions['components'] = { | |||
}, | |||
MuiBreadcrumbs: { | |||
styleOverrides: { | |||
separator: { | |||
separator: () => ({ |
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.
If this doesn't depend on any theme parameters, does it still have to be a lambda?
? pxToNumber(fontSize as Px) | ||
: fontSize; | ||
}).reverse(); | ||
const sortedVariants = variants; |
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.
Can we get rid of this variable now that it's the same as variants? The name is now a little misleading.
breakpoints: { | ||
values: { | ||
lg: 1280, | ||
md: 960, | ||
mobile: 720, | ||
sm: 720, | ||
xl: 1280, | ||
xs: 375, | ||
}, | ||
}, |
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.
Huh, interesting 🤔
Do you know what was missing that caused these breakpoints to not "stick" at runtime?
🎉 This PR is included in version 7.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Context: https://www.figma.com/file/4JazmqwcfRVZyRq6vRkf05/E4S-Web---Design-System?type=design&node-id=3333-13090&mode=design&t=cL7I6NTCh8NbZ0na-0