-
-
Notifications
You must be signed in to change notification settings - Fork 11
Added dark theme implementation #154
base: master
Are you sure you want to change the base?
Conversation
|
Deploy preview for eclipsefdn-solstice-assets ready! Built with commit 56515fe https://deploy-preview-154--eclipsefdn-solstice-assets.netlify.app |
| @@ -0,0 +1,13 @@ | |||
| .prefers-color-scheme{ | |||
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.
@Linus-CS why is this wrapped in a class?
If you remove that class, we should be able to see your changes live in the preview: https://deploy-preview-154--eclipsefdn-solstice-assets.netlify.app
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.
@chrisguindon Oops I accidentally left that there. At the beginning I thought it maybe would be a good idea to wrap it in a class, so that instead of modifying the content directly, you would just use the class when creating a site to specify where changes should be applied after turning on dark theme.
I will remove that and push the changes.
| @@ -0,0 +1,13 @@ | |||
| .prefers-color-scheme{ | |||
| @media (prefers-color-scheme: dark) { | |||
| body footer{ | |||
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.
did you mean body, footer?
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.
@chrisguindon Yeah you are right, sorry for that.
| color: @text-dark-theme; | ||
| } | ||
|
|
||
| body a{ |
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.
I don't think we want to do that, this will set all our links to @brand-primary; which would include the menu.
Maybe we only change the link color in <main>?
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.
@chrisguindon I will implement that in my commit too.
Signed-off-by: LinusCS <[email protected]>
e377587 to
56515fe
Compare
I have tested the css on the website directly. There probably need to be some adjustments.