Storybook: No throttling on resize event handlers.

[danigulyas]

Issue

When you resize windows fast, everything is laggy, and you feel like:

---

My take on this:

Cause

The resize event handler is not throttled.

Solution

import { throttle } from 'lodash'; // You have it as dependency already.

  {/* ... component stuff */}
   componentDidMount() {
    window.addEventListener('resize', this.onResize);
  }

  componentWillUnmount() {
    window.removeEventListener('resize', this.onResize);
  }

  onResize = throttle(this.handleOnResize, 200, { trailing: true });

  {/* onResize renamed to handleOnResize */}
  {/* ... more component stuff */}

Apart from that

_Thank you for making this!_

<3
-Dan

[shilman]

@danigulyas care to issue a PR for this? it looks like a very reasonable solution, though I haven't run into the problem myself AFAIK so it's hard for me to tell!

[Hypnosphi]

I'd prefer to use requestAnimationFrame throttling rather than timer-based one

[danigulyas]

@shilman sure thing, somewhere on the weekend.
@Hypnosphi good idea, will check up on that!

[danigulyas]

So i dug into this more, did a little timeline action, turns out the resize is kind of slow in general, i've tested the same html inside and outside the context of Storybook, it was similarly slow, which is in a way understandable for a while (this browser stuff is still good for free!).

However, after taking a second look on onResize, it seems to me that it couldn't even react to events when passed in like this (its just a higher order function, should've seen that before opening the issue).

See, onResize is registered as an event handler here, but it is actually being used for something different and it returns a function which is ought to handle some event with size as a parameter.

@Hypnosphi: I couldn't find requestAnimationFrame or something similar in the lib which the sidebar is implemented with, but that's not really a performance bottleneck in my opinion (you can check the timeline of rerenders with rect_perf and decide for yourself).

@shilman: So yup, this issue i guess is solved, given that it didn't exist to begin with, what do you think?

[wuweiweiwu]

@shilman @Hypnosphi @danielduan I just took a peek at the code and im 99.99% sure the issue lies because we dispatch setState too many times here

And MDN even has a page dedicated to how to throttle resize events.

Also im 99.99% sure this code is not resizing, adding onResize as an event listener doesnt do anything. Every thing is done in the onChange handler that calls onResize's return function

  componentDidMount() {
    window.addEventListener('resize', this.onResize);
  }

  componentWillUnmount() {
    window.removeEventListener('resize', this.onResize);
  }

But I'll have a PR out for this in the next couple of days when I have time :)

[danielduan]

In that case, a throttle probably makes more sense than requestAnimationFrame?

we shouldn't be running setState 60x / sec even if the computer is fast enough to support it. Looking forward to your PR, thanks @wuweiweiwu

[Hypnosphi]

we shouldn't be running setState 60x / sec

Why? I'd like the resizing to be smooth

UPD: OK, it's not about resizing itself, it's about updating dimension numbers. Then throttling on ~200 ms sounds reasonable

[wuweiweiwu]

@Hypnosphi @danielduan Done! I took a slightly different approach and delayed state update until the user stops dragging for 50ms using setTimeout and clearTimeout so we don't call setState a billion times :)

[Hypnosphi]

Released as 3.4.0-rc.1