5
\$\begingroup\$

I have an image toggle triggered by button clicks and checkboxes. My code is currently working how I need it to, but I'm very new to JavaScript so I'm sure there is a cleaner way to do this.

The desired functionality is this: There are three images that need to toggle, a before image, after image, and a combo image. There are two ways to toggle the images. One is a toggle button, that toggles between the before and after images. I've used a checkbox for this. The other button has an onclick that changes the image to the combo image, hides the checkbox and shows a 'toggle view' button that when clicked, switches back to the toggle mode. (Hopefully all of that makes sense..)

A few notes:

This is for a client, so for confidentiality reasons, I cannot share the actual images, but the alt tags should tell the story.

I'm not allowed to use anything other than vanilla JS on the platform this will live, and all variables and functions have to have custom names, hence the funky naming.

var csDMU_checkbox = document.getElementById("csDMU_checkbox");
var csDMU_imageBefore = document.getElementById("before-image");
var csDMU_imageAfter = document.getElementById("after-image");
var csDMU_imageCombo = document.getElementById("combo-image");
var csDMU_switch = document.getElementById("switch");
var csDMU_toggle = document.getElementById("toggle");

function csDMU_toggleImage() {
  if (csDMU_checkbox.checked == true) {
    csDMU_imageBefore.style.display = "none";
    csDMU_imageAfter.style.display = "block";
    csDMU_imageCombo.style.display = "none";
  } else {
    csDMU_imageBefore.style.display = "block";
    csDMU_imageAfter.style.display = "none";
    csDMU_imageCombo.style.display = "none";
  }
}

function csDMU_comboView() {
  csDMU_imageCombo.style.display = "block";
  csDMU_imageBefore.style.display = "none";
  csDMU_imageAfter.style.display = "none";
  csDMU_switch.style.display = "none";
  csDMU_toggle.style.display = "block";
}

function csDMU_toggleView() {
  csDMU_switch.style.display = "block";
  csDMU_toggle.style.display = "none";
  csDMU_imageBefore.style.display = "block";
  csDMU_imageCombo.style.display = "none";
}
@import url('https://fonts.googleapis.com/css2?family=Libre+Franklin:ital,wght@0,400;0,700;1,400;1,700&display=swap');

body {
 font-family: 'Libre Franklin', sans-serif;
}
.flexRow {
    display: -webkit-box;
    display: -ms-flexbox;
    display: flex;
    -webkit-box-align: center;
        -ms-flex-align: center;
            align-items: center;
}

/* The switch - the box around the slider */
.switch {
  position: relative;
  display: inline-block;
  width: 119px;
  height: 40px;
}

/* Hide default HTML checkbox */
.switch input {
  opacity: 0;
  width: 0;
  height: 0;
}

/* The slider */
.slider {
  position: absolute;
  cursor: pointer;
  top: 0;
  left: 0;
  right: 0;
  bottom: 0;
  background-color: #243b43;
  -webkit-transition: .4s;
  transition: .4s;
}

.slider:before {
  position: absolute;
  content: "";
  height: 32px;
  width: 33px;
  right: 4px;
  bottom: 3px;
  background: transparent -webkit-gradient(linear, left top, left bottom, from(#FFFFFF), color-stop(47%, #EDEDED), color-stop(73%, #D0D0D0), to(#E5E5E5)) 0% 0% no-repeat padding-box;
  background: transparent linear-gradient(180deg, #FFFFFF 0%, #EDEDED 47%, #D0D0D0 73%, #E5E5E5 100%) 0% 0% no-repeat padding-box;
  -webkit-box-shadow: 0px 3px 6px #00000029;
  box-shadow: 0px 3px 6px #00000029;
    border: 1px solid #FFFFFF;
  -webkit-transition: .4s;
  transition: .4s;
}
.slider:after {
    content: "BEFORE";
    display: block;
    font-size: 14px;
    line-height: 14px;
    letter-spacing: 0.16px;
    font-weight: bold;
    color: #FFF;
    position: relative;
    top: 13px;
    left: 10px;
}

input:checked + .slider {
  background-color: #F26322;
}

input:focus + .slider {
  -webkit-box-shadow: 0 0 1px #2196F3;
  box-shadow: 0 0 1px #2196F3;
}

input:checked + .slider:before {
  -webkit-transform: translateX(-75px);
  transform: translateX(-75px);
}
input:checked + .slider:after {  
    content:'AFTER';
    left: 50px;
}
/* Rounded sliders */
.slider.round {
  border-radius: 34px;
}

.slider.round:before {
  border-radius: 50%;
}
.combo-button,
.toggle-button{
    width: 172px;
    height: 40px;
    margin-left: 20px;
    border-radius: 100px;
    border: 1px solid #C4C4C4;
    color: #4a4b4d;
    letter-spacing: 0.16px;
}
.combo-button:hover,
.combo-button:focus {
    background-color: #002D5E;
    color: #FFF;
}
.combo-button:focus {
    outline: 0;
}
.toggle-button {
    display: none;
    width: 119px;
    margin: 0;
}
.hand-img {
    max-width: 700px;
    margin-right: -20px;
    display: block;
}
#after-image,
#combo-image {
    display: none;
}
<html>
<body>
<div id="image-change">
  <img src="" alt="before image" class="hand-img" id="before-image" />
  <img src="" alt="after image" class="hand-img" id="after-image" />
  <img src="" alt="combo image" class="hand-img" id="combo-image" />
</div>
<div class="flexRow">
  <label class="switch" id="switch">
    <input type="checkbox" id="csDMU_checkbox" onclick="csDMU_toggleImage()">
    <span class="slider round"></span>
  </label>
  <button class="toggle-button" id="toggle" onclick="csDMU_toggleView()">TOGGLE VIEW</button>
  <button class="combo-button" onclick="csDMU_comboView()">COMPARISON</button>
</div>

</body>
</html>

\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

Inline handlers have a multitude of problems, including a crazy scope chain and string escaping issues. Nowadays, there arguably isn't really any good reason to use them - better to leave the HTML code to be the content itself, and apply the required functionality and presentation elsewhere, ideally in a self-contained script. That is, rather than

<input type="checkbox" id="csDMU_checkbox" onclick="csDMU_toggleImage()">

you can do

document.querySelector('#csDMU_checkbox').addEventListener('click', csDMU_toggleImage);

Another thing to consider is - is there any chance of the page ever getting expanded to include more elements, perhaps another before-after pair or few? If so, you will run into problems due to your IDs. (Every element with an ID must be unique in a document, or the HTML will be invalid.) IDs are probably best reserved for elements which will absolutely always be completely unique in a document. Another problem with IDs is that every element with an ID unfortunately becomes a global variable, which can result in bugs and hard-to-understand behavior (I've seen this happen many times on Stack Overflow questions).

I'd consider using just classes instead, by default.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you! I will change the onclick to be an event listener. This particular page won't be expanded in the future, its for a 6-month campaign that runs once and then has to be taken down. But if it's best practice to use classes instead, I will change to just using classes, that makes sense. Thank you for your help! \$\endgroup\$
    – katiebry
    Commented Sep 13, 2020 at 20:09
2
\$\begingroup\$

we can use class instead of using inline style for show/hide elements.

Added a show class for the first/init image.

<div id="image-change">
  <img src="https://via.placeholder.com/150" alt="before image" class="hand-img show" id="before-image" />
  <img src="https://via.placeholder.com/151" alt="after image" class="hand-img" id="after-image" />
  <img src="https://via.placeholder.com/152" alt="combo image" class="hand-img" id="combo-image" />
</div>
<div class="flexRow">
  <label class="switch" id="switch">
    <input type="checkbox" id="csDMU_checkbox" onclick="csDMU_toggleImage()">
    <span class="slider round"></span>
  </label>
  <button class="toggle-button" id="toggle" onclick="csDMU_toggleView()">TOGGLE VIEW</button>
  <button class="combo-button" onclick="csDMU_comboView()">COMPARISON</button>
</div>

Hide all images by default except who has a show class:

@import url('https://fonts.googleapis.com/css2?family=Libre+Franklin:ital,wght@0,400;0,700;1,400;1,700&display=swap');

body {
 font-family: 'Libre Franklin', sans-serif;
}
.flexRow {
    display: -webkit-box;
    display: -ms-flexbox;
    display: flex;
    -webkit-box-align: center;
        -ms-flex-align: center;
            align-items: center;
}

/* The switch - the box around the slider */
.switch {
  position: relative;
  display: inline-block;
  width: 119px;
  height: 40px;
}

/* Hide default HTML checkbox */
.switch input {
  opacity: 0;
  width: 0;
  height: 0;
}

/* The slider */
.slider {
  position: absolute;
  cursor: pointer;
  top: 0;
  left: 0;
  right: 0;
  bottom: 0;
  background-color: #243b43;
  -webkit-transition: .4s;
  transition: .4s;
}

.slider:before {
  position: absolute;
  content: "";
  height: 32px;
  width: 33px;
  right: 4px;
  bottom: 3px;
  background: transparent -webkit-gradient(linear, left top, left bottom, from(#FFFFFF), color-stop(47%, #EDEDED), color-stop(73%, #D0D0D0), to(#E5E5E5)) 0% 0% no-repeat padding-box;
  background: transparent linear-gradient(180deg, #FFFFFF 0%, #EDEDED 47%, #D0D0D0 73%, #E5E5E5 100%) 0% 0% no-repeat padding-box;
  -webkit-box-shadow: 0px 3px 6px #00000029;
  box-shadow: 0px 3px 6px #00000029;
    border: 1px solid #FFFFFF;
  -webkit-transition: .4s;
  transition: .4s;
}
.slider:after {
    content: "BEFORE";
    display: block;
    font-size: 14px;
    line-height: 14px;
    letter-spacing: 0.16px;
    font-weight: bold;
    color: #FFF;
    position: relative;
    top: 13px;
    left: 10px;
}

input:checked + .slider {
  background-color: #F26322;
}

input:focus + .slider {
  -webkit-box-shadow: 0 0 1px #2196F3;
  box-shadow: 0 0 1px #2196F3;
}

input:checked + .slider:before {
  -webkit-transform: translateX(-75px);
  transform: translateX(-75px);
}
input:checked + .slider:after { 
    content:'AFTER';
    left: 50px;
}
/* Rounded sliders */
.slider.round {
  border-radius: 34px;
}

.slider.round:before {
  border-radius: 50%;
}
.combo-button,
.toggle-button{
    width: 172px;
    height: 40px;
    margin-left: 20px;
    border-radius: 100px;
    border: 1px solid #C4C4C4;
    color: #4a4b4d;
    letter-spacing: 0.16px;
}
.combo-button:hover,
.combo-button:focus {
    background-color: #002D5E;
    color: #FFF;
}
.combo-button:focus {
    outline: 0;
}
.toggle-button {
    display: none;
    width: 119px;
    margin: 0;
}
.hand-img {
    max-width: 700px;
    margin-right: -20px;
    display: block;
}
#after-image,
#before-image,
#combo-image {
    display: none;
}
.show {
  display: block !important
}

and this is the JS:

const _ = el => {
  return document.querySelector(el);
}

const csDMU_checkbox = _("#csDMU_checkbox"),
      csDMU_imageBefore = _("#before-image"),
      csDMU_imageAfter = _("#after-image"),
      csDMU_imageCombo = _("#combo-image"),
      csDMU_switch = _("#switch"),
      csDMU_toggle = _("#toggle"),
      image_change = document.querySelectorAll('#image-change img');


const hideAllImages = () => {
  image_change.forEach(item => item.classList.remove('show'))
}
const toggleButtons = el => {
  [csDMU_switch, csDMU_toggle].forEach(item => item.style.display = "none");
  el.style.display = "block";
}

const csDMU_toggleImage = () => {
  hideAllImages()
  const imageToShow = csDMU_checkbox.checked ? csDMU_imageAfter : csDMU_imageBefore;
  imageToShow.classList.toggle("show");
}

const csDMU_comboView  = () => {
  hideAllImages()
  csDMU_imageCombo.classList.toggle("show");
  toggleButtons(csDMU_toggle);
}
 
const csDMU_toggleView = () => {
  csDMU_toggleImage();
  toggleButtons(csDMU_switch);
}

This is a demo on CodePen https://codepen.io/moamen/pen/yLOqQWp

\$\endgroup\$
3
  • \$\begingroup\$ oh that is so much cleaner! thank you! \$\endgroup\$
    – katiebry
    Commented Sep 16, 2020 at 12:29
  • \$\begingroup\$ I'm unfamiliar with some of the syntax you're using (im a newbie). can you explain what the underscores do? In this: const _ = el => { return document.querySelector(el); } and below that, when setting the variables \$\endgroup\$
    – katiebry
    Commented Sep 16, 2020 at 13:26
  • \$\begingroup\$ @katiebry I use this function to select elements, so I can select an element by _(element) instead of type document.querySelector(element), just a shortcut. \$\endgroup\$
    – Moamen
    Commented Sep 16, 2020 at 15:09
1
\$\begingroup\$

Well you could minimize your JS code by first selecting all the elements using one line selector, note that you need to write the selected elements in their order in HTML to get them right because we're destructuring the returned node list into variables so the order matters because it's an array and not an object, then you are writing repeatedly element.style.display = "some value" so you can write a function to do that using an array of elements as input and their display value to set as two arrays and just loop over the first array of the elements and assign the correct CSS display value according to the index, and use a ternary expression instead of If-Else statement to write less and clean code, here is the full JS code

let [csDMU_imageBefore, csDMU_imageAfter, csDMU_imageCombo, csDMU_switch, csDMU_checkbox, csDMU_toggle] = document.querySelectorAll("#before-image, #after-image, #combo-image, #switch, #csDMU_checkbox, #toggle");

const setCssDisplay = (elements, values) => elements.forEach((element, index) => element.style.display = values[index]);

function csDMU_toggleImage() {
  setCssDisplay([csDMU_imageBefore, csDMU_imageAfter, csDMU_imageCombo], csDMU_checkbox.checked ? ["none", "block", "none"] : ["block", "none", "none"]);
}

function csDMU_comboView() {
  setCssDisplay([csDMU_imageCombo, csDMU_imageBefore, csDMU_imageAfter, csDMU_switch, csDMU_toggle], ["block", "none", "none", "none", "block"]);
}

function csDMU_toggleView() {
  setCssDisplay([csDMU_switch, csDMU_toggle, csDMU_imageBefore, csDMU_imageCombo], ["block", "none", "block", "none"]);
}
\$\endgroup\$
1
  • \$\begingroup\$ this makes sense, thank you!! \$\endgroup\$
    – katiebry
    Commented Sep 13, 2020 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.