Skip to main content
added 1455 characters in body
Source Link
Gerrit0
  • 3.5k
  • 1
  • 13
  • 25

Your code already looks pretty clean. Good work! There are only a few things I would recommend changing.

  1. Don't define timings in CSS that should be defined in JavaScript. In this specific case using a transition made it possible to simplify the code significantly, however in a more involved example it would become problematic. What happens if one progress bar takes 2 seconds to complete, and another takes 5?

  2. Instead of using getElementsByClassName('cls')[0], you can use querySelector('.cls') for more readable code. Though this can be slower, it really isn't an issue unless it is being run thousands of times per second.

  3. There is a built in <progress> element that you may want to use.

  4. It is a good idea to wrap template elements in a <template> element.

Here's a simple demo that defines the timing in the JavaScript.

function Loader(element, time) {
  return new Promise(resolve => {
    let intervalId;
    function tick() {
      element.value = parseInt(element.value, 10) + 1
      if (element.value == element.max) {
        clearInterval(intervalId)
        resolve()
      }
    }
    setInterval(tick, time / element.max)
  })
}

let promise = Promise.resolve()
let container = document.querySelector('.container')
let template = document.querySelector('template')

function addProgress() {
  let progress = document.importNode(template.content, true)
  container.appendChild(progress)
  // progress is a document fragment, not the template element
  return container.querySelector('progress:last-child')
}

document.querySelector('button').addEventListener('click', () => {
  let bar = addProgress()
  promise = promise.then(() => Loader(bar, 3000))
})
progress {
  display: block;
  width: 100%;
}
<template>
  <progress max="100" value="0"></progress>
</template>
<div class="container"></div>
<button>Click me!</button>

Your code already looks pretty clean. Good work! There are only a few things I would recommend changing.

  1. Don't define timings in CSS that should be defined in JavaScript. In this specific case using a transition made it possible to simplify the code significantly, however in a more involved example it would become problematic. What happens if one progress bar takes 2 seconds to complete, and another takes 5?

  2. Instead of using getElementsByClassName('cls')[0], you can use querySelector('.cls') for more readable code. Though this can be slower, it really isn't an issue unless it is being run thousands of times per second.

  3. There is a built in <progress> element that you may want to use.

  4. It is a good idea to wrap template elements in a <template> element.

Your code already looks pretty clean. Good work! There are only a few things I would recommend changing.

  1. Don't define timings in CSS that should be defined in JavaScript. In this specific case using a transition made it possible to simplify the code significantly, however in a more involved example it would become problematic. What happens if one progress bar takes 2 seconds to complete, and another takes 5?

  2. Instead of using getElementsByClassName('cls')[0], you can use querySelector('.cls') for more readable code. Though this can be slower, it really isn't an issue unless it is being run thousands of times per second.

  3. There is a built in <progress> element that you may want to use.

  4. It is a good idea to wrap template elements in a <template> element.

Here's a simple demo that defines the timing in the JavaScript.

function Loader(element, time) {
  return new Promise(resolve => {
    let intervalId;
    function tick() {
      element.value = parseInt(element.value, 10) + 1
      if (element.value == element.max) {
        clearInterval(intervalId)
        resolve()
      }
    }
    setInterval(tick, time / element.max)
  })
}

let promise = Promise.resolve()
let container = document.querySelector('.container')
let template = document.querySelector('template')

function addProgress() {
  let progress = document.importNode(template.content, true)
  container.appendChild(progress)
  // progress is a document fragment, not the template element
  return container.querySelector('progress:last-child')
}

document.querySelector('button').addEventListener('click', () => {
  let bar = addProgress()
  promise = promise.then(() => Loader(bar, 3000))
})
progress {
  display: block;
  width: 100%;
}
<template>
  <progress max="100" value="0"></progress>
</template>
<div class="container"></div>
<button>Click me!</button>

Source Link
Gerrit0
  • 3.5k
  • 1
  • 13
  • 25

Your code already looks pretty clean. Good work! There are only a few things I would recommend changing.

  1. Don't define timings in CSS that should be defined in JavaScript. In this specific case using a transition made it possible to simplify the code significantly, however in a more involved example it would become problematic. What happens if one progress bar takes 2 seconds to complete, and another takes 5?

  2. Instead of using getElementsByClassName('cls')[0], you can use querySelector('.cls') for more readable code. Though this can be slower, it really isn't an issue unless it is being run thousands of times per second.

  3. There is a built in <progress> element that you may want to use.

  4. It is a good idea to wrap template elements in a <template> element.