Add onStartReached and onStartReachedThreshold to VirtualizedList #35321

janicduplessis · 2022-11-13T01:44:48Z

Summary

Add onStartReached and onStartReachedThreshold to VirtualizedList. This allows implementing bidirectional paging.

Changelog

[General] [Added] - Add onStartReached and onStartReachedThreshold to VirtualizedList

Test Plan

Tested in the new RN tester example that the callback is triggered when close to the start of the list.

janicduplessis · 2022-11-13T01:45:36Z

Libraries/Lists/FlatList.d.ts

@@ -146,19 +146,6 @@ export interface FlatListProps<ItemT> extends VirtualizedListProps<ItemT> {
   */
  numColumns?: number | undefined;

-  /**


Those are already defined in VirtualizedList

analysis-bot · 2022-11-13T02:03:43Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 6003e70
Branch: main

analysis-bot · 2022-11-13T02:07:15Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,101,383	+3,547
android	hermes	armeabi-v7a	6,470,016	+3,061
android	hermes	x86	7,519,216	+3,168
android	hermes	x86_64	7,378,045	+3,676
android	jsc	arm64-v8a	8,965,371	+2,359
android	jsc	armeabi-v7a	7,696,762	+2,048
android	jsc	x86	9,027,866	+2,145
android	jsc	x86_64	9,506,006	+2,853

Base commit: 6003e70
Branch: main

pull-bot · 2022-11-13T02:14:41Z

PR build artifact for 8f7f6ad is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

pull-bot · 2022-11-13T02:18:36Z

PR build artifact for 8f7f6ad is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

Libraries/Lists/VirtualizedList.d.ts

NickGerleman · 2022-11-14T07:01:45Z

packages/rn-tester/js/examples/FlatList/FlatList-onStartReached.js

+export function FlatList_onStartReached(): React.Node {
+  const [output, setOutput] = React.useState('');
+  const exampleProps = {
+    onStartReached: (info: {distanceFromStart: number, ...}) =>


Is this called when a new list is mounted at zero position?

Yes it will, I think both behaviour seem ok to me.

NickGerleman · 2022-11-14T07:02:47Z

Overall lgtm but please add unit test coverage to the existing suite.

janicduplessis · 2022-11-30T18:39:01Z

@NickGerleman Thanks for the review, I think I addressed everything.

pull-bot · 2022-11-30T19:46:06Z

PR build artifact for 0295f84 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

pull-bot · 2022-11-30T19:47:30Z

PR build artifact for 0295f84 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

facebook-github-bot · 2022-12-01T15:05:43Z

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

NickGerleman · 2022-12-25T21:57:39Z

Libraries/Lists/VirtualizedList.js

    // TODO: T121172172 Look into why we're "defaulting" to a threshold of 2 when oERT is not present
-    const threshold =
-      onEndReachedThreshold != null ? onEndReachedThreshold * visibleLength : 2;
+    const startThreshold =
+      onStartReachedThresholdOrDefault(onStartReachedThreshold) * visibleLength;


Hey, I finally am getting around to some internal feedback on the change (just minor nits), but then I noticed that I think this line is a behavior change. Before, if onEndReachedThreshold wasn't set, the threshold was 2 pixels. But now it is 2 viewports, which would be a decent shift.

Would you mind if I change that back when landing?

NickGerleman · 2022-12-25T22:22:14Z

Libraries/Lists/VirtualizedList.js

    // TODO: T121172172 Look into why we're "defaulting" to a threshold of 2 when oERT is not present
-    const threshold =
-      onEndReachedThreshold != null ? onEndReachedThreshold * visibleLength : 2;
+    const startThreshold =
+      onStartReachedThresholdOrDefault(onStartReachedThreshold) * visibleLength;
+    const endThreshold =
+      onEndReachedThresholdOrDefault(onEndReachedThreshold) * visibleLength;
+    const isWithinStartThreshold = distanceFromStart <= startThreshold;
+    const isWithinEndThreshold = distanceFromEnd <= endThreshold;


Suggested change

// TODO: T121172172 Look into why we're "defaulting" to a threshold of 2 when oERT is not present

const threshold =

onEndReachedThreshold != null ? onEndReachedThreshold * visibleLength : 2;

const startThreshold =

onStartReachedThresholdOrDefault(onStartReachedThreshold) * visibleLength;

const endThreshold =

onEndReachedThresholdOrDefault(onEndReachedThreshold) * visibleLength;

const isWithinStartThreshold = distanceFromStart <= startThreshold;

const isWithinEndThreshold = distanceFromEnd <= endThreshold;

// TODO: T121172172 Look into why we're "defaulting" to a threshold of 2px

// when oERT is not present (different from 2 viewports used elsewhere)

const DEFAULT_THRESHOLD_PX = 2;

const startThreshold =

onStartReachedThreshold != null

? onStartReachedThreshold * visibleLength

: DEFAULT_THRESHOLD_PX;

const endThreshold =

onEndReachedThreshold != null

? onEndReachedThreshold * visibleLength

: DEFAULT_THRESHOLD_PX;

const isWithinStartThreshold = distanceFromStart <= startThreshold;

const isWithinEndThreshold = distanceFromEnd <= endThreshold;

This is the patch I have on the incoming diff. The code here... definitely seems incorrect. But also if it has always been like this I'm sure there is product code which might work incorrectly if the callback is called two viewports away from where it was before.

Really should probably just make it zero everywhere or at least two everywhere instead of 2px here and 2 viewports elsewhere and implied by the functions.

facebook-github-bot · 2023-01-03T23:02:03Z

@NickGerleman merged this pull request in 7683713.

janicduplessis · 2023-01-04T19:45:11Z

Hey thanks for merging this, didn't have the time to address your feedback, I assume you did it yourself?

NickGerleman · 2023-01-06T06:25:08Z

Hey thanks for merging this, didn't have the time to address your feedback, I assume you did it yourself?

Yep! I added in the patch in the most recent comment.

…cebook#35321) Summary: Add `onStartReached` and `onStartReachedThreshold` to `VirtualizedList`. This allows implementing bidirectional paging. ## Changelog [General] [Added] - Add onStartReached and onStartReachedThreshold to VirtualizedList Pull Request resolved: facebook#35321 Test Plan: Tested in the new RN tester example that the callback is triggered when close to the start of the list. Reviewed By: yungsters Differential Revision: D41653054 Pulled By: NickGerleman fbshipit-source-id: 368b357fa0d83a43afb52a3f8df84a2fbbedc132

Add onStartReached and onStartReachedThreshold props

8f7f6ad

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. labels Nov 13, 2022

janicduplessis commented Nov 13, 2022

View reviewed changes

NickGerleman reviewed Nov 14, 2022

View reviewed changes

react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Nov 22, 2022

janicduplessis added 3 commits November 30, 2022 13:31

Add tests

b16830e

Update props doc

c4a9373

Fix comment

0295f84

NickGerleman approved these changes Dec 1, 2022

View reviewed changes

NickGerleman reviewed Dec 25, 2022

View reviewed changes

facebook-github-bot closed this in 7683713 Jan 3, 2023

facebook-github-bot added the Merged This PR has been merged. label Jan 3, 2023

janicduplessis deleted the @janic/start-reached branch January 4, 2023 19:45

Simek mentioned this pull request May 17, 2023

Add onStartReached and sync onEndReached doc facebook/react-native-website#3721

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add onStartReached and onStartReachedThreshold to VirtualizedList #35321

Add onStartReached and onStartReachedThreshold to VirtualizedList #35321

janicduplessis commented Nov 13, 2022

janicduplessis Nov 13, 2022

analysis-bot commented Nov 13, 2022 •

edited

Loading

analysis-bot commented Nov 13, 2022 •

edited

Loading

pull-bot commented Nov 13, 2022

pull-bot commented Nov 13, 2022

NickGerleman Nov 14, 2022

janicduplessis Nov 14, 2022

NickGerleman commented Nov 14, 2022

janicduplessis commented Nov 30, 2022

pull-bot commented Nov 30, 2022

pull-bot commented Nov 30, 2022

facebook-github-bot commented Dec 1, 2022

NickGerleman Dec 25, 2022 •

edited

Loading

NickGerleman Dec 25, 2022 •

edited

Loading

facebook-github-bot commented Jan 3, 2023

janicduplessis commented Jan 4, 2023

NickGerleman commented Jan 6, 2023

Add onStartReached and onStartReachedThreshold to VirtualizedList #35321

Add onStartReached and onStartReachedThreshold to VirtualizedList #35321

Conversation

janicduplessis commented Nov 13, 2022

Summary

Changelog

Test Plan

janicduplessis Nov 13, 2022

Choose a reason for hiding this comment

analysis-bot commented Nov 13, 2022 • edited Loading

analysis-bot commented Nov 13, 2022 • edited Loading

pull-bot commented Nov 13, 2022

pull-bot commented Nov 13, 2022

NickGerleman Nov 14, 2022

Choose a reason for hiding this comment

janicduplessis Nov 14, 2022

Choose a reason for hiding this comment

NickGerleman commented Nov 14, 2022

janicduplessis commented Nov 30, 2022

pull-bot commented Nov 30, 2022

pull-bot commented Nov 30, 2022

facebook-github-bot commented Dec 1, 2022

NickGerleman Dec 25, 2022 • edited Loading

Choose a reason for hiding this comment

NickGerleman Dec 25, 2022 • edited Loading

Choose a reason for hiding this comment

facebook-github-bot commented Jan 3, 2023

janicduplessis commented Jan 4, 2023

NickGerleman commented Jan 6, 2023

analysis-bot commented Nov 13, 2022 •

edited

Loading

analysis-bot commented Nov 13, 2022 •

edited

Loading

NickGerleman Dec 25, 2022 •

edited

Loading

NickGerleman Dec 25, 2022 •

edited

Loading