Reputation: 632

classList.toggle() for multiple divs

I have 3 divs as colors to choose from and 3 blank divs. I want to let the user be able to:

(1) click a colored div and then a blank div, then the blank div is colored as the color the user choose. And the code seems to work.

(2) I want the user to be able to click the colored blank div again and it becomes white. And the code seems to work.

The problem is, if the blank div is colored and the user choose another color and click the colored blank div again, a newer color class will be added to the div, and things become unpredictable. You can open the console and track the messy change of the class of the blank div.

How can I solve this problem? I only want the blank divs to toggle between two classes.

var chosenColor;
function pickColor(arg){
  chosenColor=arg.id;
}

function draw(id){
  document.getElementById(id).classList.toggle("white");
  document.getElementById(id).classList.toggle(chosenColor);
}

.box{
  width: 100px;
  height: 100px;
  border: 1px solid black;
  display: inline-block;
  
}

.red{background: red}
.blue{background: blue;}
.yellow{background: yellow;}
.white{background: white;}

<html>
  <body>
    <div class="box red" id="red" onclick="pickColor(this)">1</div>
    <div class="box blue" id="blue" onclick="pickColor(this)">2</div>
    <div class="box yellow" id="yellow" onclick="pickColor(this)">3</div>
    <br><br>
    <div class="box white" id="4" onclick="draw(4)">4</div>
    <div class="box white" id="5" onclick="draw(5)">5</div>
    <div class="box white" id="6" onclick="draw(6)">6</div>
  </body>
</html>

Upvotes: 2

Answers (3)

Stephanie Schellin

Reputation: 71

Answer

See - https://codepen.io/stephanieschellin/pen/xyYxrj/ (commented code)

or ...

var activeColor

function setPickerColor(event) {
  activeColor = event.target.dataset.boxColorIs
}

function setThisBoxColor(event) {
  let element = event.target
  let the_existing_color_of_this_box = element.dataset.boxColorIs

  if (the_existing_color_of_this_box == activeColor) {
    delete element.dataset.boxColorIs
  } else {
    element.dataset.boxColorIs = activeColor
  }
}

.box {
  width: 100px;
  height: 100px;
  border: 1px solid black;
  display: inline-block;
  background: white;
}

[data-box-color-is="red"] {
  background: red
}

[data-box-color-is="blue"] {
  background: blue;
}

[data-box-color-is="yellow"] {
  background: yellow;
}

<html>

<body>
  <div id="box-1" class="box" data-box-color-is="red" onclick="setPickerColor(event)">1</div>
  <div id="box-2" class="box" data-box-color-is="blue" onclick="setPickerColor(event)">2</div>
  <div id="box-3" class="box" data-box-color-is="yellow" onclick="setPickerColor(event)">3</div>
  <br>
  <br>
  <div id="box-4" class="box" onclick="setThisBoxColor(event)">4</div>
  <div id="box-5" class="box" onclick="setThisBoxColor(event)">5</div>
  <div id="box-6" class="box" onclick="setThisBoxColor(event)">6</div>
</body>

</html>

Using data- attributes you are able to decouple the JavaScript functional concerns form the CSS classes. This simplifies your logic but most importantly it allows folks styling your app to work independently from the folks adding JS functionality. This decoupling becomes really important when your team is using BEM or an OOCSS pattern.

Ideally instead of attaching styles to the data- attribute you would maintain the 'state' using data- and have another function that sets the classList based on the data- state. Allowing you to be 100% sure style changes you make will never effect JS functionality (QA will love you). But that's an evolution beyond this post.

With this setup we are not using the id's but I left them in because its an important best practice. Most likely this code would evolve into a component with listeners instead of inline onClick calls. JavaScript selectors should always be attached to id's or data- variables, never classes. Also, the id's should always be there for the QA team to utilize in their scripts. You risk some one changing a class name or removing it to adjust the styles and inadvertently breaking your JS listener.

I switched the arguments to pass the 'event' instead of the 'this' which is the element. Anyone using your JS event functions is going to expect the event object as the first parameter. You can pass 'this' as the second parameter if you like, but event.target will give you the same thing.

One other thing to note is the syntax change between declaring the data- variable and calling it from the JS.

HTML <div data-box-color-is="red">1</div>

JS event.target.dataset.boxColorIs

Regardless of how you format you data- attribute name it will always be parsed into camelCase when referencing it in JS ... data-box_color--IS would still become ... dataset.boxColorIs

Also as an evolution to your code you could remove the global JS var and store the value on the <body> or some other element on the page using data-. This will give you a single source of truth or 'state' that multiple features/components can reference without cluttering the global space.

<div class="box" onclick="pick(this)" data-color="red">1</div>
<div class="box" onclick="pick(this)" data-color="blue">2</div>
<div class="box" onclick="pick(this)" data-color="yellow">3</div>
<br><br>
<div class="box" onclick="draw(this)">4</div>
<div class="box" onclick="draw(this)">5</div>
<div class="box" onclick="draw(this)">6</div>

What the ternary el.dataset.color = el.dataset.color ? "" : chosenColor; does is:

if the element has already any data-color set data-color to "" (nothing)
otherwise set data-color to the preselected chosenColor

Upvotes: 4

CertainPerformance

Reputation: 371233

Check to see if the element's classname is white. If not, set its class name to white - else, set it to the chosen color. You can put the boxes in a container and use .container > div selector, removing the need to give the boxes the .box class. Also, in a listener, this will refer to the clicked element - there's no need to use getElementById when you already have a reference to the element.

var chosenColor;

function pickColor(arg) {
  chosenColor = arg.id;
}

function draw(element, id) {
  if (element.className !== 'white') element.className = 'white';
  else element.className = chosenColor;
}

.container > div {
  width: 100px;
  height: 100px;
  border: 1px solid black;
  display: inline-block;
}

.red {
  background: red
}

.blue {
  background: blue;
}

.yellow {
  background: yellow;
}

.white {
  background: white;
}

<div class="container">
  <div class="red" id="red" onclick="pickColor(this)">1</div>
  <div class="blue" id="blue" onclick="pickColor(this)">2</div>
  <div class="yellow" id="yellow" onclick="pickColor(this)">3</div>
  <br><br>
  <div class="white" id="4" onclick="draw(this, 4)">4</div>
  <div class="white" id="5" onclick="draw(this, 5)">5</div>
  <div class="white" id="6" onclick="draw(this, 6)">6</div>
</div>

Upvotes: 1

classList.toggle() for multiple divs

Answers (3)

Answer

Further Reading

Related Questions