-
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 21 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 |
---|---|---|
|
@@ -25,5 +25,5 @@ VirtoLogo.propTypes = { | |
/** | ||
* styles applied to the svg element | ||
*/ | ||
sx: PropTypes.object | ||
sx: PropTypes.func | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,13 @@ | ||
import { SxProps, Theme } from "@mui/material/styles"; | ||
|
||
import { SvgIconProps } from "../SvgIcons.types"; | ||
import VirtoLogo from "./VirtoLogo"; | ||
|
||
export type VirtoLogoProps = SvgIconProps; | ||
export type VirtoLogoProps = SvgIconProps & { | ||
/** | ||
* Styling applied to the component | ||
*/ | ||
sx?: SxProps<Theme>; | ||
}; | ||
|
||
export default VirtoLogo as React.FC<VirtoLogoProps>; |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,25 @@ | ||
import React, { act } from "react"; | ||
import ThemeProvider, { useTheme } from "."; | ||
import { render, screen, waitFor } from "@testing-library/react"; | ||
|
||
import ThemeProvider from "."; | ||
import { useColorScheme } from "@mui/material"; | ||
import { userEvent } from "@testing-library/user-event"; | ||
|
||
/** | ||
* Test component to render the current theme as text | ||
*/ | ||
function ThemeText() { | ||
const [theme] = useTheme(); | ||
return <p>{theme}</p>; | ||
const { mode } = useColorScheme(); | ||
return <p>{mode}</p>; | ||
} | ||
|
||
/** | ||
* Test component to toggle the current theme | ||
*/ | ||
function ThemeToggle() { | ||
const [theme, setTheme] = useTheme(); | ||
const { mode, setMode } = useColorScheme(); | ||
const onClick = () => { | ||
setTheme(theme === "light" ? "dark" : "light"); | ||
setMode(mode === "light" ? "dark" : "light"); | ||
}; | ||
return ( | ||
<> | ||
|
@@ -44,7 +45,7 @@ describe("ThemeProvider", () => { | |
test("renders light theme by default", () => { | ||
// render the component | ||
render( | ||
<ThemeProvider> | ||
<ThemeProvider theme="light"> | ||
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. This no longer does what the test says. You've hard coded it to be light when it should be the default when nothing is passed. |
||
<ThemeText /> | ||
</ThemeProvider> | ||
); | ||
|
@@ -70,24 +71,28 @@ describe("ThemeProvider", () => { | |
); | ||
test.each(["light", "dark"])("renders %s theme from local storage", mode => { | ||
// set the theme in local storage | ||
window.localStorage.setItem("theme", mode); | ||
window.localStorage.setItem("mui-theme", mode); | ||
|
||
// render the component | ||
render( | ||
<ThemeProvider> | ||
<ThemeProvider theme={mode}> | ||
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. This no longer does what the test says. It should read the default theme from localstorage but you've passed the theme into the provider instead. |
||
<ThemeText /> | ||
</ThemeProvider> | ||
); | ||
|
||
// check that the theme is the mode we want to test | ||
const text = screen.getByText(mode); | ||
|
||
expect(text).toBeInTheDocument(); | ||
}); | ||
test.each(["light", "dark"])( | ||
"renders %s theme when theme toggled", | ||
async mode => { | ||
// first set the theme to the opposite of the mode we want to test | ||
window.localStorage.setItem("theme", mode === "light" ? "dark" : "light"); | ||
window.localStorage.setItem( | ||
"mui-theme", | ||
mode === "light" ? "dark" : "light" | ||
); | ||
|
||
// render the toggle button | ||
render( | ||
|
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 is changed to func. The VirtoLogo story also needs to be updated. But like we discussed to migrate VirtoLogo component to typeScript that will align types.