-
Notifications
You must be signed in to change notification settings - Fork 4
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
spike: style poc improvements #864
base: main
Are you sure you want to change the base?
Conversation
export const stickyHeaderStyles = (props: { | ||
theme: Theme; | ||
isSticky: boolean; | ||
}): SxProps => { |
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.
Great example of a refactored style that also handles dynamic styling
export const MainPageHeader = styled(Typography)<TypographyProps>({ | ||
fontWeight: 700, | ||
}); | ||
|
||
MainPageHeader.defaultProps = { | ||
variant: 'h4', | ||
color: 'primary', | ||
component: 'h1', | ||
}; |
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.
Good example of how a styled component could also set default parameters
Preview Firebase Hosting URL: https://mobility-feeds-dev--pr-864-7pjyth8v.web.app |
closes #844
Summary:
Dives into best styling practices going forward. Implements these styling practices in a few pages to demonstrate the use case for each styling method.
Styling Methods
Note
This information will be stored in notion
Important
React does not have strict styling standard, and as such we have a lot of freedom and liberty 🦅 . There is no strict ‘right’ way, a lot will come down to context dependent, these styling methods are strong guidelines
Use MUI Components
The number 1 default should be to use components that exist in the MUI library. These are components that are well tests and cover a fair amount of use cases
Modifying the Theme.ts
When we want to apply a style to the entire application modifying the theme is the way to go ex:
Button font
Using inline
Sx
stylingShould be used for 1 off styling and small style adjustments.
[good]
[not ideal]
Complex
Sx
Styles -> Separate FileShould be added into a separate file closely tied with the component ex:
Feeds.style.ts
.ts
and not.tsx
for clarity of content, these should not have any UI content in themThese style files should hold variables that return
Sx
styles orstyled components
.Note
If there are styles that could be reusable, they should be added in the global
styles
folderStyled Components
Should be used when we want a specific component to have a consistent style throughout the application, but not default styled like it would be done through the Theme
ex: The main page header styling and containers with coloured backgrounds
Warning
When using styled components, it currently does not accept
Sx
shorthand styling elements such asmx: 2
. The compile wont throw an error, the browser will just add the css elementmx: 2
which is invalid. There is an experimental feature from MUI calledunstable_sx
which allows this. It seems like there will be a future version that will support thisCSS files / modules
This method is not recommended, as the way we have our theme setup, we would not be able to easily access values from our Theme (ex: getting the primary colour / spacing). Although not recommended, it does have it's uses for complex css see
web-app/src/app/styles/TextShimmer.css
Why All of This is Important
chipHolderStyles
Extra Notes
CssBaseline
everywhere, just at the root should do itPlease make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.sh
to make sure you didn't break anything