Did I set up this ClickHandler correctly?

JavaScript
1

And would you change any of the names in the javascript to something else?

https://jsfiddle.net/2njyo05t/

const switchbox = document.querySelector(".switchbox");

(function manageSwitch() {
  "use strict";

  function show(el) {
    el.classList.remove("hide");
    switchbox.classList.remove("activated");
  }

  function hide(el) {
    el.classList.add("hide");
    switchbox.classList.add("activated");
  }

  function switchButtonClickHandler(evt) {
    const button = evt.currentTarget;
    if (button.classList.contains("hide")) {
      show(button)
    } else {
      hide(button);
    }
  }

  const button = document.querySelector(".button");
  button.addEventListener("click", switchButtonClickHandler);
}());
2

The show and hide functions are doing things that aren’t related at all to the el parameter being given to those functions.

It’s far better to remove that, and for the handler to do those statements instead.

  function show(el) {
    el.classList.remove("hide");
  }

  function hide(el) {
    el.classList.add("hide");
  }
...
    if (button.classList.contains("hide")) {
      show(button)
      switchbox.classList.remove("activated");
    } else {
      hide(button);
      switchbox.classList.add("activated");
    }

Otherwise, looking good. Well . . .

1 Like
3

Even better, I would remove the separate show/hide functions and just have a single toggle function. That way we can use a second parameter for the classname to toggle.

  function toggle(el, className) {
    el.classList.toggle(className);
  }

  function switchButtonClickHandler(evt) {
    const button = evt.currentTarget;
    toggle(button, "hide");
    toggle(switchbox, "activated");
  }

Keep it simple, eh? https://jsfiddle.net/6Loy5ja1/

4

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.