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

Inverted Flatlist(VirtualizedList) with specific conditions has rendering problem. #1579

Closed
shine784 opened this issue Apr 9, 2020 · 15 comments
Labels
bug: react-native Bug associated with upstream React Native vendor code project:react-native-web Issue associated with react-native-web

Comments

@shine784
Copy link

shine784 commented Apr 9, 2020

The problem
Flatlist(VirtualizedList) has problem specific conditions in Web
This list rendered the wrong range of index items, Sometimes It is rendered again constantly

How to reproduce
Simplified test case: https://snack.expo.io/@kunhee_lee/kunhee_lee_test

Steps to reproduce:

  1. Just scroll up the list
  2. If you scroll up to near by 70~80 , you can see the malfunction
  3. this problem is happend when renderitem has numberic height (ex: maybe upper than 40)

Expected behavior
react-native-web/packages/react-native-web/src/vendor/reactnative/VirtualizeUtils/VirtualizeUtils.js

...
const leadFactor = 0.5; // Math.max(0, Math.min(1, velocity / 25 + 0.5));

  const fillPreference =
    velocity > 1 ? 'after' : velocity < -1 ? 'before' : 'none';

  const overscanBegin = Math.max(0,visibleBegin - (1 - leadFactor) * overscanLength, );
  const overscanEnd = Math.max(0, visibleEnd + leadFactor * overscanLength);

  const lastItemOffset = getFrameMetricsApprox(itemCount - 1).offset;
  if (lastItemOffset < overscanBegin) {
    // Entire list is before our overscan window
    return {
      first: Math.max(0, itemCount - 1 - maxToRenderPerBatch),
      last: itemCount - 1,
    };
  }
...

leadFactor(0.5) is not suitable value with some conditions
this makes the overscanBegin value bigger than 0
so It cause the wrong retern with wrong first, last
If you fix this value leadFactor, this matter would is solved
I have tried to make correct formula for leadFactor

Environment (include versions). Did this work in previous versions?

  • React Native for Web (version): 0.12.2
  • React (version): 16.9.0
  • Browser: Chrome
@Sharcoux
Copy link

Sharcoux commented Aug 4, 2020

Probably a duplicate of this: #1254

@azesmway
Copy link

azesmway commented Aug 29, 2020

I solved this problem as follows

"react-native-web": "^0.13.9",
node_modules/react-native-web/dist/vendor/react-native/VirtualizedList/index.js

Added a second object on line 349
_this._scrollMetrics2 = { contentLength: 0, dOffset: 0, dt: 10, offset: 0, timestamp: 0, velocity: 0, visibleLength: 0 };

in line 508 add
_this._scrollMetrics = { contentLength: contentLength, timestamp: timestamp, velocity: velocity, visibleLength: visibleLength }; _this._scrollMetrics2 = { contentLength: contentLength, dt: dt, dOffset: dOffset, offset: offset, timestamp: timestamp, velocity: velocity, visibleLength: visibleLength };

and line 1265
var _this$_scrollMetrics2 = this._scrollMetrics2,

and line 1278
this._sentEndForContentLength = this._scrollMetrics2.contentLength;

@awmiklovic
Copy link

@azesmway This looks promising, but isn't working for me. I'm not sure that I'm implementing properly. Would you be able to supply a diff for the file?

@azesmway
Copy link

@awmiklovic Hi! No problem, here's the whole file, you can compare.

"react-native-web": "^0.13.13"
PATH: node_modules/react-native-web/dist/vendor/react-native/VirtualizedList/index.js

index.js.zip

@awmiklovic
Copy link

Awesome, thank you! Here's the diff in case anyone else needs it:

index 8659501..da00709 100644
--- a/node_modules/react-native-web/dist/vendor/react-native/VirtualizedList/index.js
+++ b/node_modules/react-native-web/dist/vendor/react-native/VirtualizedList/index.js
@@ -346,6 +346,15 @@ function (_React$PureComponent) {
       velocity: 0,
       visibleLength: 0
     };
+    _this._scrollMetrics2 = {
+      contentLength: 0,
+      dOffset: 0,
+      dt: 10,
+      offset: 0,
+      timestamp: 0,
+      velocity: 0,
+      visibleLength: 0
+    };
     _this._scrollRef = null;
     _this._sentEndForContentLength = 0;
     _this._totalCellLength = 0;
@@ -506,6 +515,13 @@ function (_React$PureComponent) {
       }
 
       _this._scrollMetrics = {
+        contentLength: contentLength,
+        timestamp: timestamp,
+        velocity: velocity,
+        visibleLength: visibleLength
+      };
+
+      _this._scrollMetrics2 = {
         contentLength: contentLength,
         dt: dt,
         dOffset: dOffset,
@@ -1255,7 +1271,7 @@ function (_React$PureComponent) {
         getItemCount = _this$props10.getItemCount,
         onEndReached = _this$props10.onEndReached,
         onEndReachedThreshold = _this$props10.onEndReachedThreshold;
-    var _this$_scrollMetrics2 = this._scrollMetrics,
+    var _this$_scrollMetrics2 = this._scrollMetrics2,
         contentLength = _this$_scrollMetrics2.contentLength,
         visibleLength = _this$_scrollMetrics2.visibleLength,
         offset = _this$_scrollMetrics2.offset;
@@ -1268,7 +1284,7 @@ function (_React$PureComponent) {
     distanceFromEnd < onEndReachedThreshold * visibleLength && (this._hasDataChangedSinceEndReached || this._scrollMetrics.contentLength !== this._sentEndForContentLength)) {
       // Only call onEndReached once for a given dataset + content length.
       this._hasDataChangedSinceEndReached = false;
-      this._sentEndForContentLength = this._scrollMetrics.contentLength;
+      this._sentEndForContentLength = this._scrollMetrics2.contentLength;
       onEndReached({
         distanceFromEnd: distanceFromEnd
       });

@azesmway
Copy link

@awmiklovic Did it work for you?

@awmiklovic
Copy link

@azesmway Yes, nice work!

@Sharcoux
Copy link

Do you think this could solve this issue too? #1254

Here is the sandbox

@awmiklovic
Copy link

awmiklovic commented Sep 23, 2020

@Sharcoux I believe so. That looks exactly like the bug I was running into.

@necolas
Copy link
Owner

necolas commented Sep 23, 2020

You should probably upstream this to react native as the code here is copy-paste from there

@tswicegood
Copy link

Looks like this was originally reported in facebook/react-native#28556 and the React Native team pointed @shine784 to react-native-web to report this. Not sure if they're open to a fix at that level, @necolas.

@necolas
Copy link
Owner

necolas commented Oct 12, 2020

I reopened that issue. If someone writes a PR for RN that improves compatibility on web without negatively effecting RN, I will help get the review prioritized

@CoinCoderBuffalo
Copy link

@azesmway have you updated this to work with "react-native-web": "0.17.1"? Looks like these changes are already there, but I'm seeing the same issue still.

@azesmway
Copy link

azesmway commented Jan 12, 2022

@azesmway have you updated this to work with "react-native-web": "0.17.1"? Looks like these changes are already there, but I'm seeing the same issue still.

Hi
I found solution easier

node_modules/react-native-web/dist/vendor/react-native/VirtualizedList/index.js

string 653
newState = computeWindowedRenderLimits(_this.props, state, _this._getFrameMetricsApprox, _this._scrollMetrics);
newState.first = 0; // <--- this change

@necolas
Copy link
Owner

necolas commented Mar 27, 2023

Closing this old issue because VirtualizedList will be developed exclusively out of the RN monorepo in the future. There may still be an issue in RNW, so feel free to create a new issue with latest info if needed.

@necolas necolas closed this as completed Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: react-native Bug associated with upstream React Native vendor code project:react-native-web Issue associated with react-native-web
Projects
None yet
Development

No branches or pull requests

7 participants