Skip to content
This repository has been archived by the owner on Jul 4, 2019. It is now read-only.

Feature: Throttled resize callback #4

Merged
merged 2 commits into from Jul 24, 2015

Conversation

TSMMark
Copy link
Contributor

@TSMMark TSMMark commented Jul 9, 2015

... since the Grid component listens to window resize events, this probably has some performance issues.

@jxnblk If you don't mind the lodash requirement, here's a simple throttling solution to improve the window resize listening.

@jxnblk
Copy link
Owner

jxnblk commented Jul 15, 2015

Thanks, I briefly tested throttling locally, but didn't seem to make a huge perceptual difference (which, admittedly, seemed odd). I tried to research using throttling in React components, and couldn't figure out the best place to handle that. Was wondering if you have any performance metrics or know of any best practices related to this.

@@ -1,5 +1,6 @@

import React from 'react'
import _ from 'lodash'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be import { throttle } from 'lodash', right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. Still an ES6 noob, I'm sorry to say.

Also clean up event listening.
@TSMMark
Copy link
Contributor Author

TSMMark commented Jul 15, 2015

@jxnblk I believe the lack of a huge perceptual difference is due to the Cells use of percent width. The Cells will appear to resize as the window resizes, but it's only the browser accounting for the existing inline % width style, and not React re-rendering. However, the percentage recalculation (updateWidth) will not occur until the throttled function is called. So all the Cell child components will not get render-spammed during the resize.

In my opinion, this is the best place to do the throttling, since this is where the event binding occurs.

As for benchmarking, I'm not sure of the best practices. I would have tried to whip something up if there was an existing test suite, but alas, there is not.

Edit: I'll mention that we're using the same window resize throttling technique for a few React components in production at Vydia.

@jxnblk
Copy link
Owner

jxnblk commented Jul 24, 2015

Thanks for clarifying. That makes a lot more sense now.

jxnblk added a commit that referenced this pull request Jul 24, 2015
@jxnblk jxnblk merged commit 5846509 into jxnblk:master Jul 24, 2015
@TSMMark TSMMark deleted the feature/throttle-window-resizing branch July 28, 2015 00:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants