Feat: dark mode #919
Feat: dark mode #919
Conversation
|
Preview Firebase Hosting URL: https://mobility-feeds-dev--pr-919-yyd7cpbv.web.app |
davidgamez
left a comment
davidgamez
left a comment
There was a problem hiding this comment.
Great work and super clean! I added two comments to reflect before approving. Next step dark mode in the swagger UI 😉 , example
| theme.palette.mode === ThemeModeEnum.light | ||
| ? theme.palette.primary.dark | ||
| : theme.palette.primary.light, |
There was a problem hiding this comment.
[question]: Can this be accomplished with a specific color? like theme.palette.primaryContrast? With more than one theme(not currently) this can be problematic and hard to spot before deploying.
There was a problem hiding this comment.
I see and yes, I set them to the primary color which works well on the background for each
| export const Map = (props: React.PropsWithChildren<MapProps>): JSX.Element => { | ||
| const theme = useTheme(); | ||
| const mapTiles = | ||
| theme.palette.mode === ThemeModeEnum.dark |
There was a problem hiding this comment.
[note]: I'm concerned about future modifications; this will be hard to spot and modify accordingly.
There was a problem hiding this comment.
The Map file is quite small and contained. If you're talking about the addition of future themes, perhaps a good idea would be add the map tile urls to the theme
There was a problem hiding this comment.
Yes, I'm talking about adding a theme in the future. I won't be clear to the dev that this URL is impacted by a new theme.
Summary:
Dark mode included
alinks replaced by inline buttonsNote: Analytics not dark mode supported (can be worked on in future)
Expected behavior:
Application should behave the same and look good when dark mode is enabled
Testing tips:
Go through the application toggling between light and dark mode and assure all is ok. To toggle dark mode, the button can be found in the header
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything