javascript - How to get my Dark Mode switch to work on each click?

I'm building an app with a dark mode switch. It works on the first click, but after that, it works on every second click.

(this snippet shows a checkbox. In the project, it looks like a real switch)

Any idea how I can get it to work on a single click?

const body = document.getElementById('body');
let currentBodyClass = body.className;
const darkModeSwitch = document.getElementById('darkModeSwitch');

//Dark Mode
function darkMode() {
    darkModeSwitch.addEventListener('click', () => {
        if (currentBodyClass === "lightMode") {
            body.className = currentBodyClass = "darkMode";
        } else if (currentBodyClass === "darkMode") {
            body.className = currentBodyClass = "lightMode";
        }
    });
}

#darkModeSwitch {
        position: absolute;
        left: 15px;
        top: 15px;
    }

.darkMode { background-color: black; transition: ease .3s; }
.lightMode { background-color: #FFF; transition: ease .3s; }

#darkModeSwitch input[type="checkbox"] {
  width: 40px;
  height: 20px;
  background: #fff89d;
}

#darkModeSwitch input:checked[type="checkbox"] {
  background: #757575;
}

#darkModeSwitch input[type="checkbox"]:before {
  width: 20px;
  height: 20px;
  background: #fff;
}

#darkModeSwitch input:checked[type="checkbox"]:before {
  background: #000;
}

<body id="body" class="lightMode">

  <div id="darkModeSwitch">
     <input type="checkbox" onclick="darkMode()" title="Toggle Light/Dark Mode" />
  </div>

</body>

I'm building an app with a dark mode switch. It works on the first click, but after that, it works on every second click.

(this snippet shows a checkbox. In the project, it looks like a real switch)

Any idea how I can get it to work on a single click?

const body = document.getElementById('body');
let currentBodyClass = body.className;
const darkModeSwitch = document.getElementById('darkModeSwitch');

//Dark Mode
function darkMode() {
    darkModeSwitch.addEventListener('click', () => {
        if (currentBodyClass === "lightMode") {
            body.className = currentBodyClass = "darkMode";
        } else if (currentBodyClass === "darkMode") {
            body.className = currentBodyClass = "lightMode";
        }
    });
}

#darkModeSwitch {
        position: absolute;
        left: 15px;
        top: 15px;
    }

.darkMode { background-color: black; transition: ease .3s; }
.lightMode { background-color: #FFF; transition: ease .3s; }

#darkModeSwitch input[type="checkbox"] {
  width: 40px;
  height: 20px;
  background: #fff89d;
}

#darkModeSwitch input:checked[type="checkbox"] {
  background: #757575;
}

#darkModeSwitch input[type="checkbox"]:before {
  width: 20px;
  height: 20px;
  background: #fff;
}

#darkModeSwitch input:checked[type="checkbox"]:before {
  background: #000;
}

<body id="body" class="lightMode">

  <div id="darkModeSwitch">
     <input type="checkbox" onclick="darkMode()" title="Toggle Light/Dark Mode" />
  </div>

</body>

Share Improve this question edited Jan 30, 2020 at 14:19 CoderCharmander 1,91012 silver badges18 bronze badges asked Jan 30, 2020 at 14:04 PizzaCompany88 751 silver badge6 bronze badges

Why add the click handler to a different element than which you're selecting? You add the click handler to the input but query the parent of the input. Additionally, if you're using a checkbox to represent the state, why not use the checked value of the checkbox? If it's checked then you're in dark mode and if it isn't you're in light mode. – Seth Commented Jan 30, 2020 at 14:23
Hey PizzaCompany88; if any of the answers to this question have helped you, it would be great if you could accept an answer - This will mark your question as solved, helping people finding your question in the future, and reward both yourself and the answer with some reputation – Shiny Commented Feb 4, 2020 at 21:58
You can use change eventListener instead of click eventListener and you can also use label instead of div like <label id="darkModeSwitch"> – Raeesh Alam Commented Feb 13, 2020 at 3:45

Add a comment |

8 Answers 8

Sorted by: Reset to default 7

Every time you click on your checkbox, you're adding another eventListener to darkModeSwitch - Take your addEventListener out of the function and remove the onclick

Then, you need to move let currentBodyClass = body.className; inside the darkModeSwitch function, so that the value is updated each time. Having it outside the function, you're assigning it a value once at run time, and then never updating it

Finally, this makes limited sense

body.className = currentBodyClass = "darkMode";

Instead, just do

body.className = "darkMode";

const darkModeSwitch = document.getElementById('darkModeSwitch');
const body = document.getElementById('body');

//Dark Mode
darkModeSwitch.addEventListener('click', () => {
  let currentBodyClass = body.className;

  if (body.className === "lightMode") {
    body.className = "darkMode";
  } else if (currentBodyClass === "darkMode") {
    body.className = "lightMode";
  }
});

#darkModeSwitch {
  position: absolute;
  left: 15px;
  top: 15px;
}

.darkMode {
  background-color: black;
  transition: ease .3s;
}

.lightMode {
  background-color: #FFF;
  transition: ease .3s;
}

#darkModeSwitch input[type="checkbox"] {
  width: 40px;
  height: 20px;
  background: #fff89d;
}

#darkModeSwitch input:checked[type="checkbox"] {
  background: #757575;
}

#darkModeSwitch input[type="checkbox"]:before {
  width: 20px;
  height: 20px;
  background: #fff;
}

#darkModeSwitch input:checked[type="checkbox"]:before {
  background: #000;
}

<body id="body" class="lightMode">

  <div id="darkModeSwitch">
    <input type="checkbox" title="Toggle Light/Dark Mode" />
  </div>

</body>

Finally, you might want to consider using .classList when working with an Element's classes, as it will allow you to make use of methods such as .add() / .remove() / .contains() - Your current method will fail if the Element has more than one class set at a time, while these methods avoid this

let currentBodyClass = body.classList;

if (currentBodyClass.contains("lightMode")) {
  currentBodyClass.add('darkMode');
  currentBodyClass.remove('lightMode');
} else if (currentBodyClass.contains("darkMode")) {
  currentBodyClass.add('lightMode');
  currentBodyClass.remove('darkMode');
}

On every click event of the checkbox you are setting a new event listener on the darkModeSwitch element, which can be removed

const body = document.getElementById('body');
let currentBodyClass = body.className;
const darkModeSwitch = document.getElementById('darkModeSwitch');

//Dark Mode
function darkMode() {
    
        if (currentBodyClass === "lightMode") {
            body.className = currentBodyClass = "darkMode";
        } else if (currentBodyClass === "darkMode") {
            body.className = currentBodyClass = "lightMode";
        }
    
}

#darkModeSwitch {
        position: absolute;
        left: 15px;
        top: 15px;
    }

.darkMode { background-color: black; transition: ease .3s; }
.lightMode { background-color: #FFF; transition: ease .3s; }

#darkModeSwitch input[type="checkbox"] {
  width: 40px;
  height: 20px;
  background: #fff89d;
}

#darkModeSwitch input:checked[type="checkbox"] {
  background: #757575;
}

#darkModeSwitch input[type="checkbox"]:before {
  width: 20px;
  height: 20px;
  background: #fff;
}

#darkModeSwitch input:checked[type="checkbox"]:before {
  background: #000;
}

<body id="body" class="lightMode">

  <div id="darkModeSwitch">
     <input type="checkbox" onclick="darkMode()" title="Toggle Light/Dark Mode" />
  </div>

</body>

This is easier to check using the input type

var isChecked= document.getElementById('input[type="checkbox"]').checked;
if(isChecked){ //checked
  //execute code here
}else{ //unchecked
  //execute code here
}

Before Answer:

<input type="checkbox" onclick="darkMode()" title="Toggle Light/Dark Mode" />

You call function darkMode on every click, and than

function darkMode() {
    darkModeSwitch.addEventListener('click', () => { // you add event listener
        if (currentBodyClass === "lightMode") {
            body.className = currentBodyClass = "darkMode";
        } else if (currentBodyClass === "darkMode") {
            body.className = currentBodyClass = "lightMode";
        }
    });
}

In result of it you will have as many listeners as you click on checkbox.

You need oen from 2 options:

onClick in html and remove darkModeSwitch.addEventListener('click', () => { or
remove onClick and darkMode function and keep darkModeSwitch.addEventListener('click', () => { - this will add a listener and will handle all the clicks

Getting back to answer, there are many working answers, I just want to advice a little on optimization

Simplified demo:

const body = document.getElementById('body');
const darkModeSwitch = document.getElementById('darkModeSwitch');

// put classes into array
const themeClesses = [`lightMode`, `darkMode`];

//Dark Mode
function darkMode() {
  // simply map through it and toggle
  themeClesses.map(str => body.classList.toggle(str))
}

#darkModeSwitch {
        position: absolute;
        left: 15px;
        top: 15px;
    }

.darkMode { background-color: black; transition: ease .3s; }
.lightMode { background-color: #FFF; transition: ease .3s; }

#darkModeSwitch input[type="checkbox"] {
  width: 40px;
  height: 20px;
  background: #fff89d;
}

#darkModeSwitch input:checked[type="checkbox"] {
  background: #757575;
}

#darkModeSwitch input[type="checkbox"]:before {
  width: 20px;
  height: 20px;
  background: #fff;
}

#darkModeSwitch input:checked[type="checkbox"]:before {
  background: #000;
}

<body id="body" class="lightMode">

  <div id="darkModeSwitch">
     <input type="checkbox" onclick="darkMode()" title="Toggle Light/Dark Mode" />
  </div>

</body>

Each time you click the button you add a new event listener to the switch. So every second time you are clicking the button the dark mode it toggled an even number of times resulting in no change.

An easy way to see this is to add a log in the

let eventCount = 0;
function darkMode() {
    console.log("Event was fired for the " + eventCount + "th time!");
    eventCount++;
    darkModeSwitch.addEventListener('click', () => {
        if (currentBodyClass === "lightMode") {
            body.className = currentBodyClass = "darkMode";
        } else if (currentBodyClass === "darkMode") {
            body.className = currentBodyClass = "lightMode";
        }
    });
}

The solution is, like the other answers suggest, to remove the addEventListener call in the method.

I think your problem is stemming from the fact that you're applying two event handlers to facilitate the context switch, one inline and one via addEventListener. In regards to state management, it's a bit more idiomatic to leverage the checkbox checked state as your indicator to toggle dark mode.

const body = document.getElementById('body');
let currentBodyClass = body.className;
const darkModeSwitch = document.querySelector('#darkModeSwitch input[type="checkbox"]');

darkModeSwitch.addEventListener("click", function(e) {
  var isChecked = e.target.checked;
  if (isChecked) {
    body.className = currentBodyClass = "darkMode";
  } else { //unchecked
    body.className = currentBodyClass = "lightMode";
  }
});

#darkModeSwitch {
  position: absolute;
  left: 15px;
  top: 15px;
}

.darkMode {
  background-color: black;
  transition: ease .3s;
}

.lightMode {
  background-color: #FFF;
  transition: ease .3s;
}

#darkModeSwitch input[type="checkbox"] {
  width: 40px;
  height: 20px;
  background: #fff89d;
}

#darkModeSwitch input:checked[type="checkbox"] {
  background: #757575;
}

#darkModeSwitch input[type="checkbox"]:before {
  width: 20px;
  height: 20px;
  background: #fff;
}

#darkModeSwitch input:checked[type="checkbox"]:before {
  background: #000;
}

<body id="body" class="lightMode">
  <div id="darkModeSwitch">
    <input type="checkbox" title="Toggle Light/Dark Mode" />
  </div>
</body>

Keeping it simple...

 //Dark Mode
function darkMode() {
    body.className = currentBodyClass = (currentBodyClass==="lightMode") ? "darkMode":"lightMode";
}

If you are performing more than just a "toggle" feature, a bespoke function that will just swap classes out will be enough.

Don't use click event for input type=checkbox so you can use change eventListener so this is easy way to check using the input type checked or not.

const body = document.getElementById('body');
const darkModeSwitch = document.getElementById('darkModeSwitch');
//Dark/Light Mode Switch Function
darkModeSwitch.addEventListener('change', ()=> {
  if (darkModeSwitch.checked) {
    body.className = "darkMode";
  }
  else{
    body.className = "lightMode";
  }
});

#darkModeSwitch{
  position: absolute;
  left: 15px;
  top: 15px;
  width: 30px;
  height: 30px;
}
.darkMode{background-color: black; transition: ease .3s; color: #FFF}
.lightMode{background-color: #FFF; transition: ease .3s; }

<body id="body" class="lightMode">
	
<label>
  <input type="checkbox" id="darkModeSwitch" title="Toggle Light/Dark Mode">
</label>

<br><br>
<p>
  Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
  tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
  quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
  consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
  cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non
  proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
</p>
</body>

科技改变生活-雨落星辰 - 所有的伟大,都源于一个勇敢的开始

javascript - How to get my Dark Mode switch to work on each click? - Stack Overflow

8 Answers 8

与本文相关的文章

评论列表(0)