Adjusted dark mode #268
Open
Adjusted dark mode #268
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR adjusts the dark mode visuals by conditionally loading different assets and updating theme colors for improved user experience. Key changes include:
- Conditionally rendering dark mode logos in Home.tsx.
- Updating the dark mode background color in DateRange.tsx.
- Revising theme configuration and style overrides in App.tsx for dark mode support.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/pages/Home.tsx | Adds conditional rendering to display a dark mode logo when the theme is in dark mode. |
| src/components/DateRange.tsx | Changes the dark mode background color from '#1E1E1E' to '#242424'. |
| src/App.tsx | Updates theme definitions and component style overrides with new dark mode color variables. |
Comments suppressed due to low confidence (1)
src/App.tsx:238
- Consider replacing 'default' with theme.palette.background.default to ensure the correct light mode background color is applied.
<Stack sx={{backgroundColor: (theme) => (theme.palette.mode === 'light' ? 'default' : darkBg2)}} marginTop="auto" p={2}>
| @@ -191,13 +196,17 @@ function Dashboard({setThemeMode}: {setThemeMode: (theme: PaletteMode) => void}) | |||
| </IconButton> | |||
| </Toolbar> | |||
| </AppBar> | |||
| <Drawer variant="permanent" open={open}> | |||
| <Drawer | |||
| sx={{backgroundColor: (theme) => (theme.palette.mode === 'light' ? 'default' : darkBg2)}} | |||
There was a problem hiding this comment.
Using the literal 'default' for backgroundColor may not yield the intended theme background. Consider using theme.palette.background.default instead.
Suggested change
| sx={{backgroundColor: (theme) => (theme.palette.mode === 'light' ? 'default' : darkBg2)}} | |
| sx={{backgroundColor: (theme) => (theme.palette.mode === 'light' ? theme.palette.background.default : darkBg2)}} |
Copilot uses AI. Check for mistakes.
Comment on lines
+235
to
+239
| <List sx={{backgroundColor: (theme) => (theme.palette.mode === 'light' ? 'default' : darkBg2)}} component="nav"> | ||
| <NavItems open={open} /> | ||
| </List> | ||
| <Stack marginTop="auto" p={2}> | ||
| <Stack | ||
| sx={{backgroundColor: (theme) => (theme.palette.mode === 'light' ? 'default' : darkBg2)}} |
There was a problem hiding this comment.
Using 'default' as a background color may not render as expected. Replace it with theme.palette.background.default for consistency.
Copilot uses AI. Check for mistakes.
Comment on lines
+341
to
+369
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiButton: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiSvgIcon: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDrawer: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDialog: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg3, |
There was a problem hiding this comment.
Using the literal 'default' for a color may not have the intended effect. Consider referencing theme.palette.text.primary or a similar valid color value.
Suggested change
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg3, | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg3, |
Copilot uses AI. Check for mistakes.
Comment on lines
+341
to
+369
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiButton: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiSvgIcon: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDrawer: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDialog: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg3, |
There was a problem hiding this comment.
Using 'default' as a color value might not render properly; consider using a valid theme color such as theme.palette.text.primary.
Suggested change
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg3, | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg3, |
Copilot uses AI. Check for mistakes.
Comment on lines
+341
to
+369
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiButton: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiSvgIcon: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDrawer: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDialog: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg3, |
There was a problem hiding this comment.
Replace the 'default' literal with theme.palette.text.primary (or an appropriate theme color) to ensure consistent text styling in light mode.
Suggested change
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg3, | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.text.primary : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg3, |
Copilot uses AI. Check for mistakes.
Comment on lines
+362
to
+369
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDialog: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg3, |
There was a problem hiding this comment.
Use theme.palette.background.default instead of 'default' to correctly apply the light mode background color.
Suggested change
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg3, | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg3, |
Copilot uses AI. Check for mistakes.
Comment on lines
+341
to
+369
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiButton: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiSvgIcon: { | ||
| styleOverrides: { | ||
| root: { | ||
| color: mode === 'light' ? 'default' : darkModeText, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDrawer: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | ||
| }, | ||
| }, | ||
| }, | ||
| MuiDialog: { | ||
| styleOverrides: { | ||
| paper: { | ||
| backgroundColor: mode === 'light' ? 'default' : darkBg3, |
There was a problem hiding this comment.
Replace 'default' with theme.palette.background.default to ensure proper rendering of the light mode dialog background.
Suggested change
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? 'default' : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? 'default' : darkBg3, | |
| color: mode === 'light' ? theme.palette.background.default : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiButton: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.background.default : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiSvgIcon: { | |
| styleOverrides: { | |
| root: { | |
| color: mode === 'light' ? theme.palette.background.default : darkModeText, | |
| }, | |
| }, | |
| }, | |
| MuiDrawer: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg2, | |
| }, | |
| }, | |
| }, | |
| MuiDialog: { | |
| styleOverrides: { | |
| paper: { | |
| backgroundColor: mode === 'light' ? theme.palette.background.default : darkBg3, |
Copilot uses AI. Check for mistakes.
| @@ -116,7 +116,7 @@ function ButtonField(props: ButtonFieldProps) { | |||
| color: theme.palette.text.secondary, | |||
| position: 'absolute', | |||
| // HACK: Match dark mode MUI Paper at elevation 1, which is where this is currently used | |||
| backgroundColor: theme.palette.mode === 'light' ? theme.palette.background.default : '#1E1E1E', | |||
| backgroundColor: theme.palette.mode === 'light' ? theme.palette.background.default : '#242424', | |||
There was a problem hiding this comment.
[nitpick] Consider extracting the hard-coded dark mode color '#242424' into a theme constant to maintain consistency across components.
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before:
After:
Light mode remains unchanged