lilweege
lilweege

Reputation: 71

Why does DOM element only update after event listener is finished? (pure js)

I am making a program in javascript that will change the css style (in this case background color) of an element after some delay, similar to a traffic light.

The problem is this: when you click the button, the button's onclick listener function is entered. In the function are for loops to iterate through each cell and change its color. Each pass through the loop should update one of the cells colors, but does not. What actually happens is all the cells are updated after the button's onclick listener function is finished.

Here is a link to a shortened version of my problem that I made https://jsfiddle.net/lilweege/4291vjto/6/

let main = document.getElementById('main')

// create 'cells' which are just empty divs (refer to css styles above)
let cells = []
for (let x = 0, numCells = 15; x < numCells; x++) {
  let cell = document.createElement("DIV")
  cell.setAttribute('class', 'cell')
  cells.push(cell)
  main.appendChild(cell)
}


// create button to run main logic
const run = document.createElement("BUTTON")
run.innerHTML = 'change colors'
run.addEventListener('click', function() {
  console.log('starting program');
  // reset all cell colors
  for (let cell of cells) {
    cell.style.background = 'white'
  }
  for (let cell of cells) {
    // change color of cell
    cell.style.background = `hsl(${cells.indexOf(cell) * (360 / cells.length)}, 100%, 50%)`
    // halt program for 500ms
    sleep(100)
  }
  console.log('done');
})
main.appendChild(run)

// sleep function halts entire program during period of ms
function sleep(milliseconds) {
  console.log(`waiting ${milliseconds} milliseconds`);
  const start = Date.now();
  let current = null;
  do {
    current = Date.now();
  } while (current - start < milliseconds);
}
.main {
  display: flex;
}

.cell {
  width: 20px;
  height: 20px;
  border: 1px solid black;
  margin: 1px;
}
<div id="main" class="main"></div>

This is also the case when adding an element, changing some other property such as innerHTML, or some other change in the DOM.

Also I don't suspect my 'sleep' function to be the problem, because all it is doing is halting the entire program in the browser until an amount of milliseconds have passed (it calls Date.now() until the delta of current time and start time is larger than the passed in amount).

Any pointers or solutions would be much appreciated, thanks!

Upvotes: 3

Views: 1099

Answers (2)

CertainPerformance
CertainPerformance

Reputation: 370699

Your sleep function is the problem. It's semi-permanently running a blocking loop. All of the current tab (or frame)'s resources are going towards resolving that loop, so that once it finishes, the event loop can continue on and start rendering. But if the nested loop (and the event listener) doesn't resolve until 10 seconds later, the browser won't be able to even start to update the display until 10 seconds later.

Await a Promise which uses setTimeout instead - setTimeout doesn't block processing or rendering, unlike a loop that goes through tens of thousands of iterations:

const sleep = ms => new Promise(res => setTimeout(res, ms));
const main = document.getElementById('main')
const cells = []
for (let x = 0, numCells = 20; x < numCells; x++) {
  const cell = document.createElement("DIV")
  cell.setAttribute('class', 'cell')
  cells.push(cell)
  main.appendChild(cell)
}
const run = document.createElement("BUTTON")
run.innerHTML = 'change colors'
run.addEventListener('click', async function() {
  for (let cell of cells) {
    cell.style.background = `hsl(${cells.indexOf(cell) * (360 / cells.length)}, 100%, 50%)`
    await sleep(500)
  }
  console.log('done');
})
main.appendChild(run)
.main {
  display: flex;
}
.cell {
  width: 20px;
  height: 20px;
  border: 1px solid black;
  margin: 1px;
}
<div id="main" class="main"></div>

There's almost never a good reason to use a while loop to wait for a Date.now() to reach a threshold - it's extremely computationally expensive, is user-unfriendly (since the frame can't be interacted with during that time), and can produce unexpected results (like what you're experiencing here).

Upvotes: 4

Mister Jojo
Mister Jojo

Reputation: 22274

CertainPerformance responded faster than me while I was working on my side on the same response.

I just made some improvements to the code,
I also believe that it is better to place the sleep before the coloring...

const main     = document.getElementById('main')
  ,   cells    = []
  ,   numCells = 16
  ,   run      = document.createElement('button')
  ,   sleep    = ms => new Promise(res => setTimeout(res, ms))
  ;
let InOut = 0  // to reverse colors order
  ;
run.textContent = 'set colors'
  ;
for (let x=0; x < numCells; x++)    // create 'cells' which are just empty divs (refer to css above)
  {
  cells[x] = main.appendChild(document.createElement('div'))
  }
main.appendChild(run)  //  button to run main logic
  ;    
run.onclick = async _=>
  {
  for (let cell of cells)
    { cell.style.backgroundColor = null }
  for (let x in cells)
    {
    await sleep(200)
    cells[x].style.backgroundColor = `hsl(${(Math.abs(InOut-x)) *(360 /numCells)}, 100%, 50%)`
    }
  InOut = InOut ? 0 : (numCells-1)
  }
#main {
  display: flex;
}
#main div   {
  display: inline-block;
  width: 1.2em;
  height: 1.2em;
  border: 1px solid black;
  margin: .1em;
  background-color: white;
}
#main button   {
  margin-left:.3em
}
<div id="main"></div>

Upvotes: 0

Related Questions