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.

Sign up for GitHub

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

Jump to bottom

VirtualizedList onViewableItemsChanged won't trigger if first item in data evaluate to false #35280

Closed
samchan0221 opened this issue Nov 9, 2022 · 2 comments
Closed

VirtualizedList onViewableItemsChanged won't trigger if first item in data evaluate to false #35280

samchan0221 opened this issue Nov 9, 2022 · 2 comments
Labels
Component: VirtualizedList Needs: Triage 🔍 Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@samchan0221
Copy link
Contributor

Description

VirtualizedList onViewableItemsChanged won't trigger if first item in data evaluate to false

Version

0.66.4, 0.70.4

Output of npx react-native info

System:
    OS: macOS 12.6
    CPU: (8) arm64 Apple M1
    Memory: 140.41 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node
    Yarn: 1.22.15 - ~/.nvm/versions/node/v16.14.2/bin/yarn
    npm: 8.5.0 - ~/.nvm/versions/node/v16.14.2/bin/npm
    Watchman: 2022.10.31.00 - /opt/homebrew/bin/watchman
  Managers:
    CocoaPods: 1.11.3 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 16.0, macOS 12.3, tvOS 16.0, watchOS 9.0
    Android SDK: Not Found
  IDEs:
    Android Studio: 2021.3 AI-213.7172.25.2113.9123335
    Xcode: 14.0/14A309 - /usr/bin/xcodebuild
  Languages:
    Java: 11.0.17 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 17.0.2 => 17.0.2
    react-native: 0.66.4 => 0.66.4
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to reproduce

import * as React from 'react';
import { View, FlatList } from 'react-native';

export default function App() {
  const renderItem = item => {
    return (
      <View style={{height: 200, marginVertical: 20, backgroundColor: "red"}} />
    );
  };

  const onViewableItemsChanged = () => {
    console.log("onViewableItemsChanged triggered");
  };

  return (
    <FlatList
      style={{flex: 1, backgroundColor: "blue"}}
      contentContainerStyle={{padding: 20}}
      data={[0, 1, 0, 2, 3, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18]}
      renderItem={renderItem}
      onViewableItemsChanged={onViewableItemsChanged}
    />
  );
}

Snack, code example, screenshot, or link to a repository

snack:
https://snack.expo.dev/tmQo_R_3Y
samchan0221 added a commit to samchan0221/react-native that referenced this issue Nov 9, 2022
@samchan0221
Fix VirtualizedList onViewableItemsChanged not triggered if first ite…
f79aade 
…m in data evaluate to false facebook#35280
@samchan0221 samchan0221 mentioned this issue Nov 9, 2022
Closed
samchan0221 added a commit to samchan0221/react-native that referenced this issue Nov 9, 2022
@samchan0221
Fix VirtualizedList onViewableItemsChanged not triggered if first ite…
bc2dbbb 
…m in data evaluate to false facebook#35280
samchan0221 added a commit to samchan0221/react-native that referenced this issue Nov 9, 2022
@samchan0221
Fix VirtualizedList onViewableItemsChanged not triggered if first ite…
bb88eae 
…m in data evaluate to false facebook#35280
facebook-github-bot pushed a commit that referenced this issue Nov 9, 2022
@samchan0221 @facebook-github-bot
VirtualizedList onViewableItemsChanged won't trigger if first item in…
1f0c2c2 
… data evaluate to false #35280 (#35282)

Summary:
VirtualizedList onViewableItemsChanged won't trigger if first item in data evaluate to false #35280

Described in #35280

## Changelog

- [General] [Fixed] Fix VirtualizedList onViewableItemsChanged won't trigger if first item in data evaluate to false

Pull Request resolved: #35282

Test Plan:
this snack will be able to log `onViewableItemsChanged triggered` after the fix:
https://snack.expo.dev/tmQo_R_3Y

Reviewed By: jacdebug

Differential Revision: D41158485

Pulled By: NickGerleman

fbshipit-source-id: 47434890155abe009c2560b658adc4e067c31027
facebook-github-bot referenced this issue Feb 6, 2023
@gauravroy1995 @facebook-github-bot
fix(35280): added a fix to trigger on viewable items changed when nul…
011ea33 
…l or 0 is passed in first element (#36009)

Summary:
Since currently the check was for null , and that too not === check. So added a check , only for item !== undefined, since null is an assigned value, and we can have null as values in the array for flatlist,
 undefined is in absence of any data, hence if its only undefined we should assign falsy to frame variable, since null is an assigned value, sometimes null can be passed to data in the dataset

 Hence added a check on top of Sam's previous commit to fix it

 UPDATE:

 Now after discussing with NickGerleman , removed the check for item with nullish/undefined.
 Now directly frames value is being controlled by _keyExtractor function

 This is already an issue [https://github.com/facebook/react-native/issues/35280](url)

Currently in my project, even [0,1] -> didnt trigger onViewableItemsChanged since, it was considered as falsy value,
went to check the node modules code for flatlist, debugged this.
When pulled latest main branch, saw it was partially fixed , but for null as values it wasnt fixed. So added that check .

## Changelog

[General] [Fixed] Fix VirtualizedList onViewableItemsChanged won't trigger if first item in data  is null

```
const frame =
      item !== undefined ? this._frames[this._keyExtractor(item, index, props)]
        : undefined;
```

      in place of existing which is

```
const frame =
      item != null ? this._frames[this._keyExtractor(item, index, props)]
        : undefined;
```

Update:

`const frame = this._frames[this._keyExtractor(item, index, props)]`

Finally this is the one used for getting frames value

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

Pull Request resolved: #36009

Test Plan:
Checked out in my local , wrt changes , will share video if required

Update:
After the new changes too, checked in local, working fine

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Reviewed By: NickGerleman

Differential Revision: D42934757

Pulled By: skinsshark

fbshipit-source-id: cb5622a79523bccbdfbc15470baf84422f635b33
@github-actions
Copy link

github-actions bot commented May 8, 2023

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label May 8, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
@samchan0221 @OlimpiaZurek
VirtualizedList onViewableItemsChanged won't trigger if first item in…
9b4e1e3 
… data evaluate to false facebook#35280 (facebook#35282)

Summary:
VirtualizedList onViewableItemsChanged won't trigger if first item in data evaluate to false facebook#35280

Described in facebook#35280

## Changelog

- [General] [Fixed] Fix VirtualizedList onViewableItemsChanged won't trigger if first item in data evaluate to false

Pull Request resolved: facebook#35282

Test Plan:
this snack will be able to log `onViewableItemsChanged triggered` after the fix:
https://snack.expo.dev/tmQo_R_3Y

Reviewed By: jacdebug

Differential Revision: D41158485

Pulled By: NickGerleman

fbshipit-source-id: 47434890155abe009c2560b658adc4e067c31027
OlimpiaZurek referenced this issue in OlimpiaZurek/react-native May 22, 2023
@gauravroy1995 @OlimpiaZurek
fix(35280): added a fix to trigger on viewable items changed when nul…
6139ba5 
…l or 0 is passed in first element (facebook#36009)

Summary:
Since currently the check was for null , and that too not === check. So added a check , only for item !== undefined, since null is an assigned value, and we can have null as values in the array for flatlist,
 undefined is in absence of any data, hence if its only undefined we should assign falsy to frame variable, since null is an assigned value, sometimes null can be passed to data in the dataset

 Hence added a check on top of Sam's previous commit to fix it

 UPDATE:

 Now after discussing with NickGerleman , removed the check for item with nullish/undefined.
 Now directly frames value is being controlled by _keyExtractor function

 This is already an issue [https://github.com/facebook/react-native/issues/35280](url)

Currently in my project, even [0,1] -> didnt trigger onViewableItemsChanged since, it was considered as falsy value,
went to check the node modules code for flatlist, debugged this.
When pulled latest main branch, saw it was partially fixed , but for null as values it wasnt fixed. So added that check .

## Changelog

[General] [Fixed] Fix VirtualizedList onViewableItemsChanged won't trigger if first item in data  is null

```
const frame =
      item !== undefined ? this._frames[this._keyExtractor(item, index, props)]
        : undefined;
```

      in place of existing which is

```
const frame =
      item != null ? this._frames[this._keyExtractor(item, index, props)]
        : undefined;
```

Update:

`const frame = this._frames[this._keyExtractor(item, index, props)]`

Finally this is the one used for getting frames value

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

Pull Request resolved: facebook#36009

Test Plan:
Checked out in my local , wrt changes , will share video if required

Update:
After the new changes too, checked in local, working fine

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Reviewed By: NickGerleman

Differential Revision: D42934757

Pulled By: skinsshark

fbshipit-source-id: cb5622a79523bccbdfbc15470baf84422f635b33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: VirtualizedList Needs: Triage 🔍 Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@react-native-bot @samchan0221