Skip to content
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.

Sign up for GitHub

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

Dark mode #214

Closed
wants to merge 19 commits into from
Closed

Dark mode #214

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
27758dc
Good commit
Dec 8, 2018
b5b8865
Merge branch 'master' of https://github.com/swapagarwal/swag-for-dev
Dec 8, 2018
47c8640
EOF sync
Dec 17, 2018
2fd3f4d
Solved merge conflict
Dec 17, 2018
2784373
Removed font-awesome
Dec 17, 2018
9a99ba9
Dark mode 2.0
Dec 25, 2018
31a43b5
Last Sync
Dec 25, 2018
dcd6859
Merge branch 'branch2.0'
Dec 25, 2018
6a5c26e
Small syntax error
Dec 25, 2018
9d8b65a
Dark mode 2.1
Jan 7, 2019
76df89a
Merge branch 'branch2.0'
Jan 7, 2019
cfa01a6
Dark mode 2.3
Jan 7, 2019
e8760a2
New line after variable
Jan 7, 2019
1ad9d0b
Grouping dark-theme.styl
Jan 7, 2019
afdfc63
Fixed lint errors
Jan 7, 2019
3fceca9
Merged, rebased
Jan 7, 2019
b75bf1f
Repushing
Jan 7, 2019
7b89b8e
Repushing
Jan 7, 2019
fb2801d
Small changes
Jan 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/js/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const sort = {
const contentEl = document.getElementById('content');
const filterInput = document.getElementById('filter');
const sortingInput = document.getElementById('sorting');
const colorSwitch = document.getElementById('color-switch');
aslafy-z marked this conversation as resolved.
Show resolved Hide resolved

const activateElements = els => Array.from(els).forEach(node => node.classList.add(ACTIVE_CLASS));
const allowDifficultySelect = shouldAllow => sortingInput.querySelectorAll('.difficulty')
Expand Down Expand Up @@ -104,3 +105,26 @@ window.addEventListener('load', () => {

cascade.call(window, true);
});

function setTheme(preference) {
Copy link
Collaborator

@aslafy-z aslafy-z Jan 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is lot of repetitions in this function and the following one, maybe we can do something to get some more generic theme switching engine ? staying simple

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok got it.

if (preference === 'dark') {
document.body.classList.add('dark');
colorSwitch.innerHTML = 'Light mode';
Copy link
Contributor

@itaisteinherz itaisteinherz Jan 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to set colorSwitch.textContent here since the value used is plaintext and doesn't contain any HTML (this also applies to line 115).

} else if (preference === 'light' || preference === null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if (preference === 'light' || preference === null) {
} else if (preference === 'light' || !preference) {

document.body.classList.remove('dark');
colorSwitch.innerHTML = 'Dark mode';
}
}

setTheme(localStorage.getItem('color-preference'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there sould be a fallback value
the first time you load the page, localStorage value is not set so no theme is set (and the "Light/Dark mode" text is not written)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I totally missed that.


colorSwitch.addEventListener('click', () => {
const preference = localStorage.getItem('color-preference');
if (preference === 'light') {
localStorage.setItem('color-preference', 'dark');
setTheme('dark');
} else {
localStorage.setItem('color-preference', 'light');
setTheme('light');
}
});
3 changes: 2 additions & 1 deletion src/pug/includes/body.pug
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
include ./fork.pug

.container

span#color-switch
aslafy-z marked this conversation as resolved.
Show resolved Hide resolved

section.main.flex

.heading.flex
Expand Down
2 changes: 0 additions & 2 deletions src/pug/includes/head.pug
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ meta(http-equiv="X-UA-Compatible" content="IE=edge")

Copy link
Collaborator

@aslafy-z aslafy-z Jan 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes should be reverted.

meta(name="description" content="swag opportunities for developers")


link(rel="icon" type="image/png" href="/assets/img/logo.png")
link(rel="apple-touch-icon" size="128x128" href="/assets/img/logo.png")

link(rel="stylesheet" href=bustedAssets.css)

link(rel='stylesheet' href='https://cdn.jsdelivr.net/npm/[email protected]/dist/selectr.min.css')

noscript
Expand Down
10 changes: 10 additions & 0 deletions src/styl/imports/dark-theme.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
body.dark
background-color #222 !important

& #color-switch
color white !important

& h1, p, span
color white !important
& a
color #d11717 !important
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file misses a ending new line, you need to run lint !

=> Lint should be ran automatically on commit with husky and lint-staged modules
Why is it not working for you ?

I think you should do a clean merge or rebase from upstream because it looks that you misses files.
You may want to do a npm install again also.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran lint but it didn't show any errors! Guess it may be missing something. I will run npm again.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it works for you now?

7 changes: 7 additions & 0 deletions src/styl/index.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
@require "imports/base"
@import "imports/dark-theme"

.container
min-height 100vh
width 100vw
overflow-x hidden

#color-switch
cursor pointer
padding-top 1px
color #111
font-weight bold

section.main
width 90%
Expand Down