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

Autoscroll after mounting in case preserveScrollTop is false #32

Merged
merged 1 commit into from
Feb 3, 2016
Merged

Autoscroll after mounting in case preserveScrollTop is false #32

merged 1 commit into from
Feb 3, 2016

Conversation

zalmoxisus
Copy link
Collaborator

this.scrollDown = false;
}
}
componentDidUpdate = this.scroll;
Copy link
Owner

Choose a reason for hiding this comment

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

This looks a tiny bit surprising, let's turn this into a regular declaration instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure :)
By regular declaration you mean componentDidUpdate() { this.scroll(); }?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If so, agree, in my case we pass prevProps and prevState to this.scroll, which is unnecessary.

Copy link
Owner

Choose a reason for hiding this comment

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

First question: yes.
Second question: not sure I understand. Can it just take no arguments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, sorry, I wasn't clear enough. I meant that one disadvantage of componentDidUpdate = this.scroll (as I did before) is that it creates an impression that this.scroll is called as this.scroll(prevProps, prevState) as componentDidUpdate passes those arguments.

gaearon added a commit that referenced this pull request Feb 3, 2016
Autoscroll after mounting in case `preserveScrollTop` is `false`
@gaearon gaearon merged commit e8924b1 into gaearon:master Feb 3, 2016
@gaearon
Copy link
Owner

gaearon commented Feb 3, 2016

Out in 1.0.4.

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