[go: nahoru, domu]

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

Optimized mouse up listener #31

Merged
merged 2 commits into from
Feb 10, 2019
Merged

Optimized mouse up listener #31

merged 2 commits into from
Feb 10, 2019

Conversation

gorhawk
Copy link
Contributor
@gorhawk gorhawk commented Dec 12, 2018

No description provided.

@zesik
Copy link
Owner
zesik commented Feb 9, 2019

Hi, why do we need this? If resizing is false before, there shouldn't be any re-rendering, right?

@gorhawk
Copy link
Contributor Author
gorhawk commented Feb 9, 2019

If you mean a DOM change by rendering, you are right. But the default implementation of shouldComponentUpdate just returns true. That means the lifecycle methods get called and eventually (most likely) React decides there should be no DOM change. You can help out React by not calling setState with the same state.

@gorhawk
Copy link
Contributor Author
gorhawk commented Feb 9, 2019

Yeah, found it in the docs: https://reactjs.org/docs/react-component.html#setstate

Sorry, I should've provided a description.

Edit: Oh, and it seems the React docs should also be updated too. In v16, you can return null from a setState callback. That's what I did in this commit. https://reactjs.org/blog/2017/09/26/react-v16.0.html

@zesik
Copy link
Owner
zesik commented Feb 10, 2019

Oh, you're talking about the life cycle methods.

Will merge it. Thanks for contribution.

@zesik zesik merged commit d3be447 into zesik:master Feb 10, 2019
@gorhawk
Copy link
Contributor Author
gorhawk commented Feb 10, 2019

Yes, including render :D

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

Successfully merging this pull request may close these issues.

2 participants