Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new(vx-responsive): add leading option to resize debounce #754

Merged

Conversation

Pringels
Copy link
Contributor

@Pringels Pringels commented Jun 18, 2020

🚀 Enhancements

💥 Breaking Changes

  • add leading calls to debounced resize function in all @vx/responsive HOCs and components. This prevents the initial render from blocking, increasing perceived page load speed.

Fixes #753

The gifs below represent a page refresh. I set the debounceTime to 1000ms for demonstration purposes. The Before image shows a 1 second delay before the chart gets rendered on reload. The After image immediately renders the component on reload.

Before:
vx-responsive-before
After:
vx-responsive-after

@williaster williaster added this to the 0.0.198 milestone Jun 18, 2020
Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

This is a great addition ⚡ thanks for the contribution @Pringels !

My only thought is that we could make this configurable with props, but we could merge for now and add that later if we get feedback that lib consumers don't want that for some reason (not sure when you wouldn't want this)

@williaster
Copy link
Collaborator

@Pringels after discussing more with @hshoff, and looking at the rendering implications (this will add an additional render which is likely fine), we think this probably should be configurable via props, could you make that change?

You can test the difference in behavior in this sandbox
image

@Pringels
Copy link
Contributor Author

@williaster I added the additionial key to props. I set the default value to true, as I suspect this would be the desired behaviour in most cases, but if someone really needs to fine-tune performance they can always opt out.

@williaster
Copy link
Collaborator

williaster commented Jun 22, 2020

Awesome, thanks @Pringels 🙏 true sounds like the right default 👍

This should be out in the 0.0.198 release 🔜 .

@williaster williaster merged commit 1689a53 into airbnb:master Jun 22, 2020
@williaster
Copy link
Collaborator

(marking this as breaking for the release since it will add new behavior, still below 1.0.0 so will be 0.0.198 as mentioned above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

withParentSize: add "leading" option to debounced resize handler
2 participants