Skip to main content
Code Review

Return to Answer

only set order once and only have one loop over items
Source Link
Sᴀᴍ Onᴇᴌᴀ ♦
Sᴀᴍ Onᴇᴌᴀ ♦
  • 29.6k
  • 16
  • 46
  • 203

See this demonstrated in the updated code below. I realize that this still loops over the elements 1.5 times. One could move theThe code to set the order style intowill only run when those styles are not yet set so those won't get updated each time the loop overfunction runs. That functionality could also be run when the other elements in order to remove that excess looppage loads.

(function() { //IIFE to keep scope of vars limited
  var flexItems = document.getElementsByClassName("child"),
    flexItemsCount = flexItems.length,
    reverseAt = flexItems.length / 2,
    breakPoint = 480;
  document.addEventListener('DOMContentLoaded', function() {
    for (var i = reverseAt; i < flexItemsCount; i++) {
      flexItems[i].style.order = flexItemsCount - i;
    }
    reverseBoxes();
  });
  var reverseBoxes = function() {
    let height = (100 / flexItemsCount) * 2 - 2 + "%";
    let width = "auto";
    let i = 0;
    if (window.innerWidth > breakPoint) {
      width = height;    //use value calculated above
      height = "auto";    //then set this to "auto"
    }
    for (const item of flexItems) {
      item.style.width = width;
      item.style.height = height;
      if (i++ >= reverseAt && !item.style.order) {
        item.style.order = flexItemsCount - i;
      }
    }
  }
  window.addEventListener("resize", reverseBoxes);
  document.addEventListener("DOMContentLoaded", reverseBoxes);
})();
body {
  font-family: Arial, sans-serif;
  font-size: 18px;
  margin: 0;
  padding: 0;
}

.parent {
  display: flex;
  flex-wrap: wrap;
  list-style-type: none;
  padding: 0;
  height: 100vh;
}

.child {
  margin: 1%;
  text-align: center;
  background: #069;
  color: #fff;
  display: flex;
  align-items: center;
  justify-content: center;
}

@media only screen and (max-width: 480px) {
  .parent {
    flex-direction: column;
  }
  .child {
    width: 48%;
  }
}
<div class="parent">
  <div class="child">A</div>
  <div class="child">B</div>
  <div class="child">C</div>
  <div class="child">D</div>
  <div class="child">E</div>
  <div class="child">F</div>
  <div class="child">G</div>
  <div class="child">H</div>
  <div class="child">I</div>
  <div class="child">J</div>
</div>

See this demonstrated in the updated code below. I realize that this still loops over the elements 1.5 times. One could move the code to set the order style into the loop over the other elements in order to remove that excess loop.

(function() {
  var flexItems = document.getElementsByClassName("child"),
    flexItemsCount = flexItems.length,
    reverseAt = flexItems.length / 2,
    breakPoint = 480;
  document.addEventListener('DOMContentLoaded', function() {
    for (var i = reverseAt; i < flexItemsCount; i++) {
      flexItems[i].style.order = flexItemsCount - i;
    }
    reverseBoxes();
  });
  var reverseBoxes = function() {
    let height = (100 / flexItemsCount) * 2 - 2 + "%";
    let width = "auto";
    if (window.innerWidth > breakPoint) {
      width = height;    //use value calculated above
      height = "auto";    //then set this to "auto"
    }
    for (const item of flexItems) {
      item.style.width = width;
      item.style.height = height;
    }
  }
  window.addEventListener("resize", reverseBoxes);
})();
body {
  font-family: Arial, sans-serif;
  font-size: 18px;
  margin: 0;
  padding: 0;
}

.parent {
  display: flex;
  flex-wrap: wrap;
  list-style-type: none;
  padding: 0;
  height: 100vh;
}

.child {
  margin: 1%;
  text-align: center;
  background: #069;
  color: #fff;
  display: flex;
  align-items: center;
  justify-content: center;
}

@media only screen and (max-width: 480px) {
  .parent {
    flex-direction: column;
  }
  .child {
    width: 48%;
  }
}
<div class="parent">
  <div class="child">A</div>
  <div class="child">B</div>
  <div class="child">C</div>
  <div class="child">D</div>
  <div class="child">E</div>
  <div class="child">F</div>
  <div class="child">G</div>
  <div class="child">H</div>
  <div class="child">I</div>
  <div class="child">J</div>
</div>

See this demonstrated in the updated code below. The code to set the order style will only run when those styles are not yet set so those won't get updated each time the function runs. That functionality could also be run when the page loads.

(function() { //IIFE to keep scope of vars limited
  var flexItems = document.getElementsByClassName("child"),
    flexItemsCount = flexItems.length,
    reverseAt = flexItems.length / 2,
    breakPoint = 480;
  var reverseBoxes = function() {
    let height = (100 / flexItemsCount) * 2 - 2 + "%";
    let width = "auto";
    let i = 0;
    if (window.innerWidth > breakPoint) {
      width = height;    //use value calculated above
      height = "auto";    //then set this to "auto"
    }
    for (const item of flexItems) {
      item.style.width = width;
      item.style.height = height;
      if (i++ >= reverseAt && !item.style.order) {
        item.style.order = flexItemsCount - i;
      }
    }
  }
  window.addEventListener("resize", reverseBoxes);
  document.addEventListener("DOMContentLoaded", reverseBoxes);
})();
body {
  font-family: Arial, sans-serif;
  font-size: 18px;
  margin: 0;
  padding: 0;
}

.parent {
  display: flex;
  flex-wrap: wrap;
  list-style-type: none;
  padding: 0;
  height: 100vh;
}

.child {
  margin: 1%;
  text-align: center;
  background: #069;
  color: #fff;
  display: flex;
  align-items: center;
  justify-content: center;
}

@media only screen and (max-width: 480px) {
  .parent {
    flex-direction: column;
  }
  .child {
    width: 48%;
  }
}
<div class="parent">
  <div class="child">A</div>
  <div class="child">B</div>
  <div class="child">C</div>
  <div class="child">D</div>
  <div class="child">E</div>
  <div class="child">F</div>
  <div class="child">G</div>
  <div class="child">H</div>
  <div class="child">I</div>
  <div class="child">J</div>
</div>
update explanation
Source Link
Sᴀᴍ Onᴇᴌᴀ ♦
Sᴀᴍ Onᴇᴌᴀ ♦
  • 29.6k
  • 16
  • 46
  • 203
  • selecting child elements by class name with document.getElementsByClassName() and only do this once instead of each time the function runs. Generally document.getElementsByClassName will be quicker than document.querySelectorAll (see this post for more information) and the former also returns a live HTMLCollection so it wouldn't need to be queried each time.
  • only setting the order style on the items once, since that never changes between calls to reverseBoxes()
  • calculate the percentage height or width once instead of in each iteration of looping through the elements
  • selecting child elements by class name with document.getElementsByClassName() and only do this once instead of each time the function runs
  • only setting the order style on the items once, since that never changes between calls to reverseBoxes()
  • calculate the percentage height or width once instead of in each iteration of looping through the elements
  • selecting child elements by class name with document.getElementsByClassName() and only do this once instead of each time the function runs. Generally document.getElementsByClassName will be quicker than document.querySelectorAll (see this post for more information) and the former also returns a live HTMLCollection so it wouldn't need to be queried each time.
  • only setting the order style on the items once, since that never changes between calls to reverseBoxes()
  • calculate the percentage height or width once instead of in each iteration of looping through the elements
Source Link
Sᴀᴍ Onᴇᴌᴀ ♦
Sᴀᴍ Onᴇᴌᴀ ♦
  • 29.6k
  • 16
  • 46
  • 203

This code could be optimized by:

  • selecting child elements by class name with document.getElementsByClassName() and only do this once instead of each time the function runs
  • only setting the order style on the items once, since that never changes between calls to reverseBoxes()
  • calculate the percentage height or width once instead of in each iteration of looping through the elements

See this demonstrated in the updated code below. I realize that this still loops over the elements 1.5 times. One could move the code to set the order style into the loop over the other elements in order to remove that excess loop.

(function() {
  var flexItems = document.getElementsByClassName("child"),
    flexItemsCount = flexItems.length,
    reverseAt = flexItems.length / 2,
    breakPoint = 480;
  document.addEventListener('DOMContentLoaded', function() {
    for (var i = reverseAt; i < flexItemsCount; i++) {
      flexItems[i].style.order = flexItemsCount - i;
    }
    reverseBoxes();
  });
  var reverseBoxes = function() {
    let height = (100 / flexItemsCount) * 2 - 2 + "%";
    let width = "auto";
    if (window.innerWidth > breakPoint) {
      width = height;    //use value calculated above
      height = "auto";    //then set this to "auto"
    }
    for (const item of flexItems) {
      item.style.width = width;
      item.style.height = height;
    }
  }
  window.addEventListener("resize", reverseBoxes);
})();
body {
  font-family: Arial, sans-serif;
  font-size: 18px;
  margin: 0;
  padding: 0;
}

.parent {
  display: flex;
  flex-wrap: wrap;
  list-style-type: none;
  padding: 0;
  height: 100vh;
}

.child {
  margin: 1%;
  text-align: center;
  background: #069;
  color: #fff;
  display: flex;
  align-items: center;
  justify-content: center;
}

@media only screen and (max-width: 480px) {
  .parent {
    flex-direction: column;
  }
  .child {
    width: 48%;
  }
}
<div class="parent">
  <div class="child">A</div>
  <div class="child">B</div>
  <div class="child">C</div>
  <div class="child">D</div>
  <div class="child">E</div>
  <div class="child">F</div>
  <div class="child">G</div>
  <div class="child">H</div>
  <div class="child">I</div>
  <div class="child">J</div>
</div>