Why does this code need an unused expression to work?

JavaScript
1

I have a simple slideToggle component (taken from here).

You can see the code running in a CodePen here.

I understand the general principle of the code, but there are two lines (both the same) which have me scratching my head as to what they do.

Here is the code for the slideDown function:

const slideDown = (target, duration = 500) => {
  target.style.removeProperty("display");
  let display = window.getComputedStyle(target).display;

  if (display === "none") display = "block";

  target.style.display = display;
  const height = target.offsetHeight;
  target.style.overflow = "hidden";
  target.style.height = 0;
  target.style.paddingTop = 0;
  target.style.paddingBottom = 0;
  target.style.marginTop = 0;
  target.style.marginBottom = 0;

  // Commenting out this line causes the animation to break
  target.offsetHeight;

  target.style.boxSizing = "border-box";
  target.style.transitionProperty = "height, margin, padding";
  target.style.transitionDuration = duration + "ms";
  target.style.height = height + "px";
  target.style.removeProperty("padding-top");
  target.style.removeProperty("padding-bottom");
  target.style.removeProperty("margin-top");
  target.style.removeProperty("margin-bottom");

  window.setTimeout(() => {
    target.style.removeProperty("height");
    target.style.removeProperty("overflow");
    target.style.removeProperty("transition-duration");
    target.style.removeProperty("transition-property");
  }, duration);
};

Notice this line:

target.offsetHeight;

At first I thought it was a typo, but commenting it out stops the slideDown animation from working.

The same line occurs in the slideUp function, but the animation still seems to work if this is commented out.

Can anyone explain why this random expression which appears to do nothing, stops the slideDown animation from working if it is removed?

Here is the code in a web page in case anyone wants to run it on their machine:

<!DOCTYPE html>
<html lang="en" >
<head>
  <meta charset="utf-8">
  <title>SlideToggle</title>
  <style>
    #target {
      padding: 6em 1.5em;
      background-color: #ef1;
      margin-bottom: 1em;
      text-align: center;
      color: rgba(0, 0, 0, 0.65);
      font-weight: 700;
      border-radius: 5px;
    }

    #target h1 {
      max-width: 500px;
      font-size: 1.5;
      margin: 0 auto;
      line-height: 1.618;
      font-weight: regular;
    }

    .triggers {
      text-align: center;
    }
  </style>
</head>
<body>
  <div id="target">
    <h1>Pure JavaScript SlideToggle / SlideUp / slideDown</h1>
  </div>

  <div class="triggers">
    <button id="triggerUp">slideUp</button>
    <button id="triggerDown">slideDown</button>
    <button id="triggerToggle">slideToggle</button>
  </div>

  <script>
    const slideUp = (target, duration = 500) => {
      target.style.transitionProperty = "height, margin, padding";
      target.style.transitionDuration = duration + "ms";
      target.style.boxSizing = "border-box";
      target.style.height = target.offsetHeight + "px";
      // Seems you can get away with commenting out the next line
      target.offsetHeight;
      target.style.overflow = "hidden";
      target.style.height = 0;
      target.style.paddingTop = 0;
      target.style.paddingBottom = 0;
      target.style.marginTop = 0;
      target.style.marginBottom = 0;

      window.setTimeout(() => {
        target.style.display = "none";
        target.style.removeProperty("height");
        target.style.removeProperty("padding-top");
        target.style.removeProperty("padding-bottom");
        target.style.removeProperty("margin-top");
        target.style.removeProperty("margin-bottom");
        target.style.removeProperty("overflow");
        target.style.removeProperty("transition-duration");
        target.style.removeProperty("transition-property");
      }, duration);
    };

    const slideDown = (target, duration = 500) => {
      target.style.removeProperty("display");
      let display = window.getComputedStyle(target).display;

      if (display === "none") display = "block";

      target.style.display = display;
      const height = target.offsetHeight;
      target.style.overflow = "hidden";
      target.style.height = 0;
      target.style.paddingTop = 0;
      target.style.paddingBottom = 0;
      target.style.marginTop = 0;
      target.style.marginBottom = 0;
      // Commenting out the next line causes the animation to break
      target.offsetHeight;
      target.style.boxSizing = "border-box";
      target.style.transitionProperty = "height, margin, padding";
      target.style.transitionDuration = duration + "ms";
      target.style.height = height + "px";
      target.style.removeProperty("padding-top");
      target.style.removeProperty("padding-bottom");
      target.style.removeProperty("margin-top");
      target.style.removeProperty("margin-bottom");

      window.setTimeout(() => {
        target.style.removeProperty("height");
        target.style.removeProperty("overflow");
        target.style.removeProperty("transition-duration");
        target.style.removeProperty("transition-property");
      }, duration);
    };

    const slideToggle = (target, duration = 500) => {
      if (window.getComputedStyle(target).display === "none") {
        return slideDown(target, duration);
      }
      return slideUp(target, duration);
    };

    document.getElementById("triggerUp").addEventListener("click", function () {
      slideUp(document.getElementById("target"), 300);
    });

    document.getElementById("triggerDown").addEventListener("click", function () {
      slideDown(document.getElementById("target"), 300);
    });

    document.getElementById("triggerToggle").addEventListener("click", function () {
      slideToggle(document.getElementById("target"), 300);
    });
  </script>
</body>
</html>
3

Hi Jim,

I’m on a mobile at the moment but the offset-height (hack) was usually a way to restart an animation.

I’ll take a look when I’m back on the desktop but it looks like it may be related to the above.

2 Likes
4

Thanks, Paul. I’ll also give that article a read in the meantime. I’m also not 100% sure, but it seems like that could be what’s going on here.

1 Like
5

My first thought was that it had something to do with repaint/redraw. Funnily enough on googling ‘JS dom force repaint’, it came up with this link.

https://martinwolf.org/before-2018/blog/2014/06/force-repaint-of-an-element-with-javascript/

Code excerpt from that link.

/**
 * Force Repaint of Header because of
   OSX Safari Rendering Bug
 */
var siteHeader = document.getElementById('header');

siteHeader.style.display='none';
siteHeader.offsetHeight; // no need to store this anywhere, the reference is enough
siteHeader.style.display='block';

Would need to dig deeper to fully understand how it works in your script.

Edit: Started this message about 30 mins ago, but was called mid typing :slight_smile:

3 Likes
6

Yes I believe it’s to restart the transition again.

You can see it more easily in a simpler codepen.

This CSS Tricks article is about ten years old and is the place I first saw it being used,

2 Likes
7

Hey @James_Hibbard, yes for the CSS transition to kick in, the start properties must be set in the preceding frame… another, probably clearer solution would be to wrap setting the transition properties in a requestAnimationFrame() callback:

target.style.height = 0

window.requestAnimationFrame(() => {
  target.style.height = height + 'px'
  // ...
})
2 Likes
8

Thank you for the links and for the explanations. They have definitely shed some light on the issue.

The following comment from the first article puts it quite succinctly:

→ void element.offsetWidth;

This instructs the browser to recalculate an element’s width, thus causing a reflow in the DOM. The reflow event triggers the animation to run from the start.

The CSS Tricks article claims that if you don’t specify void as a return value, this won’t work in strict mode. That didn’t seem to be the case for me.

A couple of the articles talk about this method incurring a performance hit.

How bad would that be in this case compared to a pure JS solution? Negligible, right?

Or is this really some kind of bad practice?

I’m afraid I couldn’t get that to work. Using my original demo I tried:

const slideDown = (target, duration = 500) => {
  target.style.removeProperty("display");
  // ...
  target.style.marginBottom = 0;

  window.requestAnimationFrame(() => {
    target.style.boxSizing = "border-box";
    // ...
  });

  window.setTimeout(() => {
    target.style.removeProperty("height");
    // ...
  }, duration);
};

and:

const slideDown = (target, duration = 500) => {
  target.style.removeProperty("display");
  // ...
  target.style.marginBottom = 0;

  window.requestAnimationFrame(() => {
    target.style.boxSizing = "border-box";
    // ...

    window.setTimeout(() => {
      target.style.removeProperty("height");
      // ...
    }, duration);
  });
};

Can you give me a clue as to what I am doing wrong?

9

Hm no that’s what I meant… if the timeout is inside the requestAnimationFrame() callback shouldn’t matter, I’d just keep it outside to keep it flat. Here’s a forked pen with all property assignments wrapped that followed the target.offsetHeight lines:

10

Thanks for the Pen. That’s what I thought you meant (the first code block in #8), but I’m afraid it doesn’t work for me.

The slideUp function works ok, but AFAICT that didn’t seem to rely on using target.offsetHeight; in the first place.

The slideDown function is broken however — the div is shown, but with no animation.

…

Edit: Ah. I just tried it in Chromium where it works as expected. It doesn’t work in Firefox. Any thoughts?

11

Huh, this is weird… yes I was using a chromium based browser, and I can confirm it doesn’t work in FF (also tried with a most minimal setup that would just set the height to a fixed value). I have no idea what’s wrong here I’m afraid. It’s actually a pattern I’ve been using in the past, so good to know it’s broken in FF… oO I’m off and back to backend. :-D

PS: Okay these would work in FF as well:

window.setTimeout(() => {
  slide.style.height = height + 'px'
})

window.requestAnimationFrame(() => {
  window.requestAnimationFrame(() => {
    slide.style.height = height + 'px'
  })
})

… but this again doesn’t exactly make the code clearer. I would stick to the implicitly forced repaint then I guess.

2 Likes
12

Thanks, good to know I wasn’t missing something obvious. I think I’ll stick to the previous method then.

2 Likes
13

For the record though, I think I figured out what’s throwing off FF here: it’s setting display: block and height: 0 in the same frame. So if you just keep the element displayed throughout (and hidden via its height only) it will work with a single requestAnimationFrame(), as would first setting the display and then the height property:

window.requestAnimationFrame(() => {
  slide.style.display = 'block'

  window.requestAnimationFrame(() => {
    slide.style.height = height + 'px'
  })
})
3 Likes
14

And to go completely off topic I would toggle keyframes instead. :slight_smile:

Then you don’t need the hack. (The timeout is just to remove the class afterwards so that there is no fixed height and the page can scale.)

Although I do prefer this older version but you have to manage the padding in the css.

2 Likes
15

Just to add, the timeout here

window.setTimeout(() => {
  target.classList.remove("open");
}, delay + 100);

can be substituted for a transitionend

target.addEventListener(
  'transitionend',
  ({ target }) => target.classList.remove("open"),
  { once: true } // event listener is added once and after firing removed
)
2 Likes
16

Thanks, Paul. Is it me, or does the animation look slightly more fluid in my example? Like it’s using some kind of easing function? I dunno… is that even possible?

Other than that your code is much more concise and I’ll definitely take the time to look through it.

Another question: would browser compatibility be a factor here? Is either demo better supported than the other?

17

I used a linear easing as that seemed more suitable for a shutter type effect but that can just be changed in the css.

Your version would have ‘ease’ applied as that’s the default if nothing else is specified.

1 Like
18

Got it. Thanks.

19

There’s nothing ‘css wise’ that would be an issue. It’s all pretty standard these days.

I’m not sure I like the padding being animated though as the text will move. In my second demo the padding is on an inner element instead and the effect is smoother.

I guess your js was intended to work without too much setting up and I guess it does that job ok. :slight_smile:

1 Like
20

Where would that be placed?

I tried it in the same place as the timeout but it didn’t remove the open class. I must have put it in the wrong place :slight_smile:

21

@PaulOB ,

Just a straight swap line 27. :slight_smile: