Skip to content

always set this.pageCount#5

Open
ksti wants to merge 3 commits intoldn0x7dc:masterfrom
ksti:master
Open

always set this.pageCount#5
ksti wants to merge 3 commits intoldn0x7dc:masterfrom
ksti:master

Conversation

@ksti
Copy link
Copy Markdown

@ksti ksti commented Sep 20, 2016

I should always set this.pageCount.
I used this in the project:ksti/react-native-photo-browser
Thanks for your awesome works!

@ksti ksti mentioned this pull request Sep 20, 2016
…ngth !== this.props.pageDataArray.length, maybe I deleted one photo\!
Copy link
Copy Markdown
Author

@ksti ksti left a comment

Choose a reason for hiding this comment

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

this should be necessary

setTimeout(this.scrollToPage.bind(this, this.props.initialPage, true), 0);
}
} else if (this.layoutChanged) {
} else if (this.layoutChanged || prevProps.pageDataArray.length !== this.props.pageDataArray.length) {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this is useful and necessary when pageDataArray changed , for instance , you deleted one photo!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this should be done in componentWillReceiveProps not in componentDidUpdate

}

componentWillReceiveProps(nextProps) {
if (nextProps.initialPage !== this.props.initialPage) {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

if initialPage changed , then scroll to nextProps.initialPage.
this code should be put in componentWillReceiveProps not in componentDidUpdate

@shercoder
Copy link
Copy Markdown

It would be great to get this PR merged.

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