Toggling between light and dark mode using JavaScript

Question

This is some code written in a course I'm busy taking on Udemy. It serves as a basic toggle switch to flip between light and dark mode. This includes the nav bar, background, images and the icon that changes from a sun to a moon.

The objective after the lesson was to clean up the code and make it DRY. I've done so to some of the code already. But there are some instances where it still repeats, for instance isDark in my lightDarkMode function. Is there a way to eliminate the repetitive usage of isDark?

const toggleSwitch = document.querySelector('input[type="checkbox"]');
const nav = document.getElementById('nav');
const toggleIcon = document.getElementById('toggle-icon');
const textBox = document.getElementById('text-box');
const darkLightTheme = ['dark', 'light'];
function imageMode(color) {
 image1.src = `img/undraw_proud_coder_${color}.svg`;
 image2.src = `img/undraw_feeling_proud_${color}.svg`;
 image3.src = `img/undraw_conceptual_idea_${color}.svg`;
}
 lightDarkMode = (isDark) => {
 nav.style.backgroundColor = isDark ? 'rgb(0 0 0 / 50%)' : 'rgb(255 255 255 / 50%)';
 textBox.style.backgroundColor = isDark ? 'rgb(255 255 255 / 50%)' : 'rgb(0 0 0 / 50%)';
 toggleIcon.children[0].textContent = isDark ? 'Dark Mode' : 'Light Mode';
 isDark ? toggleIcon.children[1].classList.replace('fa-sun', 'fa-moon') : toggleIcon.children[1].classList.replace('fa-moon', 'fa-sun');
 isDark ? imageMode(darkLightTheme[0]) : imageMode(darkLightTheme[1]);
}

• pacmaninbw: Welcome to Code Review. The title of the question should be about what the code does, not what your concerns about the code are. You can put any concerns you have about the code into the body of the question. Here are some suggestions on how to improve the question.
• slepic: It seems that there is no code repetition in the posted snippet. Maybe except for the RGB strings. getElementById is a function that you may need to call as many times as how many unique elements you want to look up. That's not code repetition. Anyway if you want some general feedback please include some description of the goal accomplished by the code. Maybe add some calling context of the code...
• Wayne: Thank you for your feedback. I'm currently going over the documentation. I will then edit my post as suggested. @pacmaninbw
• imvain2: Does this have to be primarily javascript based? In the case of toggling light/dark I prefer a simpler approach that applies a class to the body called dark assuming light is the default so it doesn't need a class. Then just reference .dark nav{} and .dark #text-box{} etc in your stylesheet. That would clean up the javascript tremendously as you would be simply toggling a class on the body and then only need to deal with the images.
• Wayne: No it does not have to be primarily Javascript based. But i am struggling to understand Javascript. So i'm trying write as much code in Javascript as possible. But based on everyone's comment's the code looks fine as is. I guess i just needed that type of closure to know weather my code is sufficient enough. I don't want to pick up bad habits along the way and i don't have a mentor that could tell me if something is right or wrong. Thank you for your input about CSS. It is noted and it will be a route i go in future project. @imvain2

Answer 1

One thing that you could update is how you are using the ternary operator for isDark.

Ternary operators are used for returning something based on a boolean response, so either return X if true, or Y if false.

However, every time it is used it is evaluating the statement being passed to it. In this case both times it is evaluating isDark. For the average website, this won't affect your processing time, but it is something to think about.

Instead a classic if/else statement would allow you to run more code off a single evaluation.

You will notice this isn't as sleek looking as ternary and is even more lines of code, but gives more capabilities when you need it.

if (isDark) {
 toggleIcon.children[1].classList.replace('fa-sun', 'fa-moon')
 imageMode(darkLightTheme[0])
} else {
 toggleIcon.children[1].classList.replace('fa-moon', 'fa-sun')
 imageMode(darkLightTheme[1])
}

• Wayne: Thank you @imvain2 , That makes sense. Right now my main goal was to eliminate some of the ternary operators and this achieves that. Not to mention you have given me a better understanding if ternary operators and if else statements. I really appreciate your help.