-
Notifications
You must be signed in to change notification settings - Fork 3
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
VE-252 Combine dark and light themes for mui v6 #1009
Changes from 9 commits
3861160
264fb14
ba38641
b5a45bb
b19b8df
c254436
4a9ab81
8a12323
4bab5b4
07fd7ec
f0935bb
e4d7efd
9993061
0f7089a
e5313f4
4add550
f88a2af
a067452
913d61e
13bb88e
db855f7
c507a4e
3c8608d
15df447
ec2aae7
a0a443d
190b5c5
d1ad40a
0c01d06
4294cc2
0a72cfc
18a10b8
e0072d4
a18e56b
74f22fe
d235e5c
87d45b6
0079504
f65f34e
47d7b5e
3a2a01c
f508bcd
a0ff0d3
7247bca
6971838
c972d9d
55a6e6f
4fda39d
1364279
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,70 +1,72 @@ | ||
.dark { | ||
background: #1b1c1d; | ||
|
||
.sb-nopreview_main { | ||
background: #1B1C1D; | ||
background: #1b1c1d; | ||
} | ||
|
||
.sb-nopreview_heading { | ||
text-align: center; | ||
text-align: center; | ||
} | ||
|
||
.sb-errordisplay { | ||
background: #2c2f33; | ||
color: white; | ||
background: #2c2f33; | ||
color: white; | ||
} | ||
|
||
.sb-errordisplay_main { | ||
background: #1e1e1e; | ||
box-shadow: 0 0 64px rgba(255, 255, 255, 0.1); | ||
background: #1e1e1e; | ||
box-shadow: 0 0 64px rgba(255, 255, 255, 0.1); | ||
} | ||
|
||
.sb-errordisplay_code { | ||
background: #1c1c1c; | ||
color: #d4d4d4; | ||
background: #1c1c1c; | ||
color: #d4d4d4; | ||
} | ||
|
||
.sb-preparing-story, | ||
.sb-preparing-docs { | ||
background-color: #1B1C1D; | ||
background-color: #1b1c1d; | ||
} | ||
|
||
.sb-loader { | ||
border-color: rgba(158, 158, 158, 0.29); | ||
border-color: rgba(158, 158, 158, 0.29); | ||
} | ||
|
||
.sb-previewBlock { | ||
background: #2c2f33; | ||
border: 1px solid rgba(255, 255, 255, 0.1); | ||
box-shadow: rgba(255, 255, 255, 0.1) 0 1px 3px 0; | ||
background: #2c2f33; | ||
border: 1px solid rgba(255, 255, 255, 0.1); | ||
box-shadow: rgba(255, 255, 255, 0.1) 0 1px 3px 0; | ||
} | ||
|
||
.sb-previewBlock_header { | ||
box-shadow: rgba(255, 255, 255, 0.1) 0 -1px 0 0 inset; | ||
box-shadow: rgba(255, 255, 255, 0.1) 0 -1px 0 0 inset; | ||
} | ||
|
||
.sb-previewBlock_icon { | ||
background: #3c3f43; | ||
background: #3c3f43; | ||
} | ||
|
||
.sb-argstableBlock th span, | ||
.sb-argstableBlock td span { | ||
background-color: rgba(255, 255, 255, 0.1); | ||
background-color: rgba(255, 255, 255, 0.1); | ||
} | ||
|
||
.sb-argstableBlock-body { | ||
box-shadow: | ||
rgba(255, 255, 255, 0.1) 0 1px 3px 1px, | ||
rgba(255, 255, 255, 0.065) 0 0 0 1px; | ||
box-shadow: | ||
rgba(255, 255, 255, 0.1) 0 1px 3px 1px, | ||
rgba(255, 255, 255, 0.065) 0 0 0 1px; | ||
} | ||
|
||
.sb-argstableBlock-body tr:not(:first-child) { | ||
border-top: 1px solid #3c3f43; | ||
border-top: 1px solid #3c3f43; | ||
} | ||
|
||
.sb-argstableBlock-body td { | ||
background: #1B1C1D; | ||
background: #1b1c1d; | ||
} | ||
|
||
.sb-argstableBlock-body button { | ||
background-color: rgba(255, 255, 255, 0.1); | ||
background-color: rgba(255, 255, 255, 0.1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,7 +59,15 @@ export function SidebarFilter({ | |
<IconButton | ||
data-testid="filter-close-button" | ||
onClick={() => setOpen(false)} | ||
sx={{ position: "absolute", right: 12, top: 12 }} | ||
sx={theme => ({ | ||
color: "#9e9e9e", | ||
position: "absolute", | ||
right: 12, | ||
top: 12, | ||
...theme.applyStyles("dark", { | ||
color: "#ffffff" | ||
}) | ||
})} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you introducing more hardcoded colours? Lets not add to the problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aligned with @Sowbhagya-ipg to go with default action.active for icon buttons |
||
> | ||
<Close /> | ||
</IconButton> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,12 @@ export default function LabelSetter({ onChange = () => {}, rows = [] }) { | |
headerName: "Action", | ||
renderCell: params => ( | ||
<IconButton | ||
color="primary" | ||
sx={theme => ({ | ||
color: "#9e9e9e", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the hardcoded colour? |
||
...theme.applyStyles("dark", { | ||
color: theme.palette.primary.main | ||
}) | ||
})} | ||
data-testid="deleteButton" | ||
onClick={event => handleOnDeleteClick(event, params)} | ||
> | ||
|
@@ -98,7 +103,12 @@ export default function LabelSetter({ onChange = () => {}, rows = [] }) { | |
/> | ||
<Box> | ||
<IconButton | ||
color="primary" | ||
sx={theme => ({ | ||
color: "#9e9e9e", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the hardcoded colour? |
||
...theme.applyStyles("dark", { | ||
color: theme.palette.primary.main | ||
}) | ||
})} | ||
data-testid="addButton" | ||
onClick={handleOnAddClick} | ||
> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,7 @@ describe("ThemeProvider", () => { | |
test("renders light theme by default", () => { | ||
// render the component | ||
render( | ||
<ThemeProvider theme="light"> | ||
<ThemeProvider> | ||
<ThemeText /> | ||
</ThemeProvider> | ||
); | ||
|
@@ -73,9 +73,16 @@ describe("ThemeProvider", () => { | |
// set the theme in local storage | ||
window.localStorage.setItem("mui-theme", mode); | ||
|
||
// get the theme from local storage | ||
const defaultMode = window.localStorage.getItem("mui-theme") as | ||
| "light" | ||
| "dark"; | ||
|
||
console.log(defaultMode); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leftover console.log |
||
|
||
// render the component | ||
render( | ||
<ThemeProvider theme={mode}> | ||
<ThemeProvider theme={defaultMode}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't pass the mode into ThemeProvider. The original behaviour of this test was that it passed no props in and the ThemeProvider read localStorage itself. |
||
<ThemeText /> | ||
</ThemeProvider> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
/* eslint-disable no-undef */ | ||
|
||
import type {} from "@mui/x-data-grid/themeAugmentation"; | ||
import type {} from "@mui/material/themeCssVarsAugmentation"; | ||
|
||
import { | ||
ThemeProvider as MuiThemeProvider, | ||
|
@@ -17,14 +18,6 @@ import darkScrollbar from "@mui/material/darkScrollbar"; | |
// extend the theme to include custom properties | ||
// https://mui.com/material-ui/customization/theming/#custom-variables | ||
declare module "@mui/material/styles" { | ||
// eslint-disable-next-line no-unused-vars | ||
interface Theme { | ||
layout: { | ||
content: { | ||
maxWidth: number; | ||
}; | ||
}; | ||
} | ||
// allow configuration using `createTheme` | ||
// eslint-disable-next-line no-unused-vars | ||
interface ThemeOptions { | ||
|
@@ -33,17 +26,21 @@ declare module "@mui/material/styles" { | |
maxWidth?: number; | ||
}; | ||
}; | ||
colorSchemes?: { | ||
dark?: ThemeOptions; | ||
light?: ThemeOptions; | ||
}; | ||
} | ||
// eslint-disable-next-line no-unused-vars | ||
interface Theme { | ||
colorSchemes: { | ||
light: ThemeOptions; | ||
dark: ThemeOptions; | ||
layout: { | ||
content: { | ||
maxWidth: number; | ||
}; | ||
}; | ||
} | ||
interface ThemeOptions { | ||
colorSchemes?: { | ||
light?: ThemeOptions; | ||
dark?: ThemeOptions; | ||
light?: ThemeOptions; | ||
}; | ||
} | ||
} | ||
|
@@ -82,6 +79,15 @@ const defaultComponents = { | |
} | ||
} | ||
}, | ||
MuiFormControl: { | ||
styleOverrides: { | ||
root: { | ||
"& .MuiFormLabel-root, .MuiFormHelperText-root ": { | ||
color: "var(--ipg-palette-text-secondary)" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
} | ||
} | ||
} | ||
}, | ||
MuiFormLabel: { | ||
styleOverrides: { | ||
asterisk: { | ||
|
@@ -93,7 +99,8 @@ const defaultComponents = { | |
styleOverrides: { | ||
tooltip: { | ||
fontFamily: "Montserrat, Arial, sans-serif", | ||
fontSize: "12px" | ||
fontSize: "12px", | ||
fontWeight: 400 | ||
} | ||
} | ||
} | ||
|
@@ -119,6 +126,16 @@ const mainTheme: ThemeOptions = { | |
} | ||
} | ||
}, | ||
MuiAutocomplete: { | ||
styleOverrides: { | ||
clearIndicator: { | ||
color: "#ffffff" | ||
}, | ||
popupIndicator: { | ||
color: "#ffffff" | ||
} | ||
} | ||
}, | ||
MuiCssBaseline: { | ||
styleOverrides: { | ||
body: darkScrollbar() | ||
|
@@ -151,7 +168,6 @@ const mainTheme: ThemeOptions = { | |
}) | ||
} | ||
}, | ||
MuiIconButton: { styleOverrides: { root: { color: "#9e9e9e" } } }, | ||
MuiStepper: { | ||
styleOverrides: { | ||
root: { | ||
|
@@ -179,8 +195,7 @@ const mainTheme: ThemeOptions = { | |
}, | ||
"&:hover": { | ||
backgroundColor: "rgba(0, 95, 168, 0.15)" | ||
}, | ||
borderColor: "rgb(196, 196, 196)" | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -233,30 +248,34 @@ export default function ThemeProvider({ | |
}: ThemeProviderProps) { | ||
// wrap mui theme provider and children in theme context | ||
return ( | ||
<MuiThemeProvider theme={mainThemeWithColorSchemes}> | ||
<ChildWrapper theme={controlledTheme}>{children}</ChildWrapper> | ||
<MuiThemeProvider theme={mainThemeWithColorSchemes} defaultMode="light"> | ||
<ControlledThemeWrapper theme={controlledTheme}> | ||
{children} | ||
</ControlledThemeWrapper> | ||
</MuiThemeProvider> | ||
); | ||
} | ||
|
||
/** | ||
* Child wrapper to handle controlled theme changes | ||
* ControlledThemeWrapper Component | ||
* | ||
* This component is used to enforce a specific theme mode (`light` or `dark`) | ||
* for its child components based on the `controlledTheme` prop. It synchronizes | ||
* the theme mode with the provided value and ensures the children use the correct | ||
* theme. The wrapper relies on MuI's `useColorScheme` hook for theme mode management. | ||
* | ||
* @param props.children - The child components to render inside the wrapper. | ||
* @param props.theme - The desired theme mode (`light` or `dark`) to enforce. | ||
* | ||
* @returns The wrapped children with the enforced theme mode. | ||
*/ | ||
function ChildWrapper({ | ||
function ControlledThemeWrapper({ | ||
children, | ||
theme: controlledTheme | ||
}: ThemeProviderProps) { | ||
// use hook from MUI to get and set the theme mode | ||
const { mode, setMode } = useColorScheme(); | ||
|
||
// update the theme mode to "light" if current mode is "system" | ||
useEffect(() => { | ||
if (mode === "system") { | ||
setMode("light"); | ||
} | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, [mode]); | ||
|
||
// update the theme mode when the controlled theme changes | ||
useEffect(() => { | ||
if (controlledTheme && mode !== controlledTheme) { | ||
|
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.
added background , this was getting overwritten by dark color-scheme from mui