6
\$\begingroup\$

I developed this CSS Masonry.

It works with a CSS grid with grid-auto-rows: 1px. There is a JS recalc script in there, which recalculates the grid-row-end values (with the span prefix). Apart from that, the layout is totally up to the Browser's engine.

(The --raster property is used to set vertical raster on or off. With the raster enanbled the heights of the items are calculated as multiples of the gaps, so the bottoms of the grid items align a little bit nicer).

On slow machines (esp. in Chrome) it might become a little bit laggy (not too dramatically), so I want to work on the performance a little bit.

I know, I could use a debounce on the resize event handler, but that's not the point here. I want to improve on the performace of the recalc function alone.

I used getElementsByClassName instead of querySelectorAll to improve performance. I needed to filter the resulting Elements by only direct descendants (simulate querySelectorAll(':scope > .item')). Don't know, if this is slower in the end than using querySelectorAll in the first place.

I used getBoundingClientRect() to calculate height of the items, so there doesn't seem to be any improvement there.

Is there anything else I could improve?

"use strict";
console.clear();

{
  const gridClass = 'grid';
  const itemClass = 'item';
  
  const grid = document.getElementsByClassName(gridClass);

  const recalc = () => {
    Array.from(grid).forEach(gr => {
      // const items = gr.querySelectorAll(`:scope > .${itemClass}`);
      const items = gr.getElementsByClassName(itemClass);
      Array.from(items)
        .filter(el => el.parentElement === gr)
        .forEach(it => {
          it.style.setProperty('grid-row-end', 'span 1');
          const h = parseInt(Math.ceil(it.children[0].getBoundingClientRect().height));
          const gap = parseInt(getComputedStyle(it).getPropertyValue('--gap'));
          const raster = !!parseInt(getComputedStyle(it).getPropertyValue('--raster'), 10)
          let span;
          if (raster) {
            span = parseInt(Math.ceil((h + gap) / gap) * gap);          
          } else {
            span = parseInt(Math.ceil(h + gap));          
          }
          it.style.setProperty('grid-row-end', `span ${span}`);
      })
    })
  }
  

  window.addEventListener('resize', recalc);
  window.addEventListener('load', recalc);
  recalc()
}
.grid {
  --cols: 12;
  --gap: 16;
  --raster: 1;
  display: grid;
  grid-template-columns: repeat(var(--cols, 12), calc((100% - 1px * var(--gap, 20) * (var(--cols, 12) - 1)) / var(--cols, 12)));
  grid-auto-rows: 1px;
  grid-gap: 0px calc(1px * var(--gap, 20));
  margin-top: calc(-1px * var(--gap, 20));
}
@media screen and (max-width: 900px) and (min-width: 600px) {
  .grid {
    --cols: 8;
  }
}
@media screen and (max-width: 599px) {
  .grid {
    --cols: 4;
  }
}
.grid > .item {
  grid-column-end: span 4;
  grid-row-end: span 400;
  margin-top: calc(1px * var(--gap, 20));
  display: flex;
  flex-direction: column;
  justify-items: stretch;
}

.grid > .item > div {
  background-color: gold;
  flex-basis: 100%;
}
<h1>CSS Grid Masonry with little help from JS</h1>
<div class="grid" style="--raster: 1;">
  <div class="item">
    <div><div>1<br>1<br></div></div>
  </div>
  <div class="item">
    <div><div>2<br>2<br>2<br>2<br>2<br>2<br>2<br></div></div>
  </div>
  <div class="item">
    <div>3<br>3<br>3<br>3<br></div>
  </div>
  <div class="item">
    <div>4<br>4<br>4<br></div>
  </div>
  <div class="item">
    <div>5<br>5<br>5<br>5<br>5<br></div>
  </div>

  <div class="item">
    <div>6<br>6<br>6<br></div>
  </div>
  <div class="item">
    <div>7<br>7<br>7<br>7<br>7<br></div>
  </div>
  <div class="item">
    <div>8<br>8<br>8<br>8<br>8<br><div class="item">item</div></div>
  </div>
  <div class="item">
    <div>9<br></div>
  </div>
  <div class="item">
    <div>10<br>10<br>10<br></div>
  </div>
  <div class="item">
    <div>11<br>11<br>11<br>11<br>11<br>11<br>11<br>11<br>11<br>11<br>11<br>11<br></div>
  </div>
  <div class="item">
    <div>12<br>12<br>12<br>12<br>12<br></div>
  </div>
  <div class="item">
    <div>13<br>13<br></div>
  </div>
  <div class="item">
    <div>14<br>14<br>14<br></div>
  </div>
  <div class="item">
    <div>15<br>15<br>15<br></div>
  </div>
  <div class="item">
    <div>16<br>16<br>16<br></div>
  </div>
  <div class="item">
    <div>17<br>17<br>17<br>17<br>17<br></div>
  </div>
  <div class="item">
    <div>18<br>18<br></div>
  </div>
  <div class="item">
    <div>19<br>19<br>19<br>19<br></div>
  </div>
  <div class="item">
    <div>20<br>20<br></div>
  </div>
  <div class="item">
    <div>21<br>21<br>21<br>21<br>21<br>21<br>21<br></div>
  </div>
  <div class="item">
    <div>22<br>22<br>22<br></div>
  </div>
  <div class="item">
    <div>23<br>23<br></div>
  </div>
  <div class="item">
    <div>24<br>24<br></div>
  </div>
  <div class="item">
    <div>25<br>25<br>25<br>25<br></div>
  </div>
</div>

\$\endgroup\$
1
\$\begingroup\$

Good things

  • variables are block scoped, declared with const unless re-assignment is necessary
  • indentation is consistent

Suggestions

Number of times recalc is called initially

Given the last two lines of the whole block:

 window.addEventListener('load', recalc);
 recalc()

Is the last line really necessary, given that the function would be called when the window is loaded? Perhaps instead of listening for the load event it should listen for the DOMContentLoaded event.

loops

It looks like the code does the following:

  • loops over elements with class name grid
    • loops over sub-elements with class item
      • filters out any item that isn't a child of the outer element with class grid, yet the inner loop doesn't appear to depend on the outer element with class grid

I'm not sure what would be quicker but I would either take one of these approaches:

  • use document.querySelectorAll('.grid > .item') (like you mentioned in the description) to find the elements to iterate over, thus allowing one loop instead of a loop inside a loop
  • instead of calling .filter().forEach() do one loop - instead of the .filter() have the .forEach() callback return if the parent is not gr.

spreading items into an array

Instead of calling Array.from() - e.g.

Array.from(grid)

the spread syntax can be used for the same result:

[...grid]

Redundant styles

The CSS for .grid contains --raster: 1

.grid {
  --cols: 12;
  --gap: 16;
  --raster: 1;

And yet the element with class grid has the --raster style in-line:

<div class="grid" style="--raster: 1;">

I see that the inline style is used by the JavaScript code to determine the calculation for span. It may be simpler to simply make a class name - e.g.

.raster1 { 
    --raster: 1
}

Then add that to the list of class names for that element and check for that class name in the JavaScript code instead of parsing the inline-style.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.