improved mode toggle #24
Conversation
src/pages/LogIn.js
Outdated
| @@ -47,7 +51,7 @@ const LogIn = () => { | |||
| } | |||
| return ( | |||
| <> | |||
| <h2>Log In</h2> | |||
| <Heading1 content = {'Log in to your account here'} /> | |||
There was a problem hiding this comment.
This is great, we can also drop the { } when a prop is a string, and I think we want to close up the space here?
<Heading1 content='Log in to your account here' />There was a problem hiding this comment.
**The spaces between content and = etc
There was a problem hiding this comment.
is it a bad practice to use space around = ? I often use it
There was a problem hiding this comment.
Ah interesting, I think my linter etc always removes it. I am used to always seeing it with no spaces.
| @@ -66,9 +66,9 @@ const App = () => { | |||
| <AuthProvider> | |||
| <Router> | |||
| {/* <Header toggleTheme={toggleTheme} theme={theme} /> */} | |||
| <ThemeProvider theme={darkTheme}> | |||
| <ThemeProvider theme = {state.currentTheme}> | |||
| `; | ||
|
|
||
| const Heading1 = ({content}) => { | ||
| return ( |
There was a problem hiding this comment.
A small thing for me I really like to remove anything not in use so could just return without the curly brackets, but maybe makes sense to leave in case we are doing some logic in here soon etc. If we removed it then could be
const Heading1 = ({content}) =>
<Heading>
{content}
</Heading>Another options is since content is being used as children can use the children prop that we get in react.
const Heading1 = ({children}) =>
<Heading>
{children}
</Heading>
and then to use it we just wrap around our stuff
<Heading1>some header</Heading1>There was a problem hiding this comment.
But yes let's merge and then iterate as we add more stuff :-)
There was a problem hiding this comment.
yeah I think we'll come back to it anyways
now it's possible to toggle between dark & light mode