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

Add a new prop - onEndReachedThresholdRelative #695

Merged
merged 4 commits into from
Mar 10, 2022
Merged

Add a new prop - onEndReachedThresholdRelative #695

merged 4 commits into from
Mar 10, 2022

Conversation

ElviraBurchik
Copy link
Contributor

What

Adds a new prop onEndReachedThresholdRelative identical to the onEndReachedThreshold of FlatList.

It allows to specify how far from the end (in units of visible length of the list) the bottom edge of the list must be from the end of the content to trigger the onEndReached callback

if (windowBound - lastOffset <= Default.value<number>(this.props.onEndReachedThreshold, 0)) {
const threshold = windowBound - lastOffset

const triggerOnEndThresholdRelative = windowBound * Default.value<number>(this.props.onEndReachedThresholdRelative, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the bound here refers to maxScrollOffset so this compute can be wrong. Our check needs to be something like threshold <= this._layout.height * relative_threshold

const threshold = windowBound - lastOffset

const triggerOnEndThresholdRelative = windowBound * Default.value<number>(this.props.onEndReachedThresholdRelative, 0);
const triggerOnEndThreshold = windowBound * Default.value<number>(this.props.onEndReachedThreshold, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This multiplication is not needed

README.md Outdated
@@ -90,6 +90,7 @@ In case you cannot determine heights of items in advance just set `forceNonDeter
| externalScrollView | No | { new (props: ScrollViewDefaultProps): BaseScrollView } | Use this to pass your on implementation of BaseScrollView |
| onEndReached | No | () => void | Callback function executed when the end of the view is hit (minus onEndThreshold if defined) |
| onEndReachedThreshold | No | number | Specify how many pixels in advance for the onEndReached callback |
| onEndReachedThresholdRelative | No | number | Specify the percent of a view in advance for the onEndReached callback |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rephrase this IMO. It's a little difficult to understand right now.

@naqvitalha
Copy link
Collaborator

@ElviraBurchik you will need to look into the lint issues. CI checks are failing

- Update new prop description in readme
@ElviraBurchik
Copy link
Contributor Author

@naqvitalha I've addressed your comments and fix lint issues, but looks like I can't re-run CI check myself.

@naqvitalha naqvitalha closed this Mar 10, 2022
@naqvitalha naqvitalha reopened this Mar 10, 2022
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.

None yet

2 participants