Shopify / flash-list

A better list for React Native
https://shopify.github.io/flash-list/
MIT License
5.4k stars 277 forks source link

onViewableItemsChanged not firing if items change without scroll 😔 #614

Open mfbx9da4 opened 1 year ago

mfbx9da4 commented 1 year ago

I've seen a couple of issues mentioning onViewableItemsChanged not working but thought I should open a new one because they don't provide reproducible examples and I found them a bit unclear

Current behavior

Adding new items to the FlashList does not cause the onViewableItemsChanged to be fired.

To Reproduce

Try tapping the button once loaded, the handler does not fire.

import { FlashList } from '@shopify/flash-list'
import React, { useState } from 'react'
import { Button, Text, ViewabilityConfig } from 'react-native'

const viewabilityConfig: ViewabilityConfig = {
  itemVisiblePercentThreshold: 0,
}

function handleViewableItemsChanged() {
  console.log('handleViewableItemsChanged')
}

export function ExampleFlashList() {
  const [data, setData] = useState(() => Array.from({ length: 100 }, (_, i) => i))
  return (
    <FlashList
      data={data}
      inverted
      ListHeaderComponent={
        <Button
          title="Add more items"
          onPress={() => {
            setData(x => [...Array.from({ length: 10 }, (_, i) => x[0] - (10 - i)), ...x])
          }}
        />
      }
      onViewableItemsChanged={handleViewableItemsChanged}
      renderItem={renderItem}
      estimatedItemSize={40}
      viewabilityConfig={viewabilityConfig}
      keyboardDismissMode="interactive"
      keyboardShouldPersistTaps="handled"
      onEndReachedThreshold={0.5}
    />
  )
}

function renderItem({ item }: { item: number }) {
  return <Text>Item: {item}</Text>
}

Platform:

Environment

1.2.2

bfricka commented 1 year ago

This is the only FlashList issue that I haven't found a good solution to. If the FlashList data changes, then onViewableItemsChanged should fire if the new data affects the viewable items in any way. Currently, changes to data will re-render the list as expected, but won't fire onViewableItemsChanged, even if the viewable items do change.

This issue means that we can't do anything in response to new data without either throwing away some efficient algorithms or doing some really crazy tracking of offsets.

For example, I have a list of Foo. When these are displayed, I need to download and decrypt, the data. To make this efficient, I use onViewableItemsChanged to filter what I need, and then send a batch of info through the bridge, which can do all of this efficiently. Since, onViewableItemsChanged isn't firing when a new Foo comes in, I need to either:

  1. Get rid of the batching, which means letting the renderItem component make the call for its own data.
  2. Make the bridge call whenever a new Foo comes in, regardless of whether it's viewable.

For now, I'll probably do 1. and create a debounced batch queue so it can collect what I need after items have been rendered for a small delay (and to avoid rendering stuff that's just scrolling by).

Still, it seems like this should be fix, because the workarounds are a real PITA.

Edit: I implemented solution 1, which works, but not ideally, since FlashList renders a bunch of non-viewable items as well. Back to the drawing board.

hirbod commented 1 year ago

@mfbx9da4 have you tried to add a keyExtractor to it? We had similar issues and using a keyExtractor only with index was enough for our use case to fix this issue.

bfricka commented 1 year ago

@hirbod Not OP, but my setup implements:

Anyways, looking at the underlying code, it's relatively clear that the issue for me is the fact that FlashList sort of conflates "visible indices" with "visible items". Frequently, new items can be added, without the visible indices changing at all.

It's very likely, therefore, that the reason I would occasionally see this fire, is because adding a new item would change the layout in such a way that visible indices changes (probably because I'm using numColumns).

Note that I have not verified this. I just looked at the code and the fact that FlashList relies on the underlying onVisibleIndicesChanged from recycler. I can go into it and figure out exactly what is going on, and probably fix it, if the maintainers are interested.

On my end, I already found a workaround, and that is keeping a reference to the visible indices. If something new comes in and it's within the visible range, I make sure the extra data is loaded.

Looks something like:

type ViewRange = [startIdx: number, endIdx: number]
const DEFAULT_VIEW_RANGE: ViewRange = [0, 1]
// List
const viewRangeRef = useRef(DEFAULT_VIEW_RANGE)

const handleViewableItemsChanged = ({ viewableItems }: FooViewabilityInfo) => {
  viewRangeRef.current[0] = viewableItems[0]?.index || 0
  viewRangeRef.current[1] = viewableItems[viewableItems.length - 1]?.index || 1
}

Not ideal, but it'll work until this is fixed.

eeshankeni commented 1 year ago

Same problem here, but it appears to be happening only on android. onviewableItemsChanged does fire as expected. However, viewableItems seems to not exist for specific items but does for others.

I have tried using the json data id and the index as the keyextractor prop. Both result in the same issue.

This pretty much makes it impossible to use flashlist where we need to keep track of which items are inside the viewport.

Edit: Setting itemVisiblePercentThreshold to 50 instead of 100 fixed my problem!

mfbx9da4 commented 1 year ago

@bfricka

On my end, I already found a workaround, and that is keeping a reference to the visible indices. If something new comes in and it's within the visible range, I make sure the extra data is loaded.

How do you get notified if the new item is being rendered though? What is handleViewableItemsChanged bound to in your pseudocode? It can't be onViewableItemsChanged, as that is the whole problem that it doesn't fire correctly? You hinted that you will use renderItem as your signal but just because the item is rendered doesn't mean it's actually visible?

mfbx9da4 commented 1 year ago

@hirbod I tried adding a key extractor didn't help. Have you tried the example?

@eeshankeni I tried playing with itemVisiblePercentThreshold no joy!

mfbx9da4 commented 1 year ago

From a brief read of the code it looks like the ViewabilityManger is only tracking indices so fundamentally it will not work as expected

I wonder if this comes down to the fact that recycler list view does not support onViewableItemsChanged? https://github.com/Flipkart/recyclerlistview/issues/713

mfbx9da4 commented 1 year ago

For my particular use case all I care about is if the first visible item changed I built a hook for this inspired from @bfricka's comment. A similar strategy could be used to fully fix FlashList

import { FlashList, ViewToken } from '@shopify/flash-list'
import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'
import { Button, Text, ViewabilityConfig } from 'react-native'

function isTruthy<T>(x: T): x is Exclude<T, Falsy> {
  return x !== null && x !== undefined && (x as any) !== false
}

const viewabilityConfig: ViewabilityConfig = {
  itemVisiblePercentThreshold: 50,
}

export function ExampleFlashList() {
  const [data, setData] = useState(() => Array.from({ length: 100 }, (_, i) => i))
  console.log('=================')
  const ref = useRef<FlashList<any>>(null)

  const notifyVisibleItemsChanged = useFirstVisibleItemChanged(data, item => {
    console.log('newly visible first item', item)
  })

  const handleViewableItemsChanged = useCallback(
    (info: { viewableItems: ViewToken[]; changed: ViewToken[] }) => {
      notifyVisibleItemsChanged(info)
      console.log('handleViewableItemsChanged')
    },
    [notifyVisibleItemsChanged]
  )

  return (
    <FlashList
      ref={ref}
      data={data}
      inverted
      ListHeaderComponent={
        <Button
          title="Add more items"
          onPress={() => {
            setData(x => [...Array.from({ length: 10 }, (_, i) => x[0] - (10 - i)), ...x])
          }}
        />
      }
      onViewableItemsChanged={handleViewableItemsChanged}
      renderItem={renderItem}
      estimatedItemSize={40}
      viewabilityConfig={viewabilityConfig}
      keyboardDismissMode="interactive"
      keyboardShouldPersistTaps="handled"
      onEndReachedThreshold={0.5}
    />
  )
}

function useFirstVisibleItemChanged<T>(
  data: T[],
  onFirstVisibleItemChanged: (item: { item: T; index: number }) => void
) {
  const viewableIndicesRef = useRef<number[]>([])

  const callbackRef = useRef(onFirstVisibleItemChanged)
  callbackRef.current = onFirstVisibleItemChanged

  const firstVisibleItemRef = useRef<T | null>(null)
  const firstVisibleItem = data[viewableIndicesRef.current[0]]
  useLayoutEffect(() => {
    if (firstVisibleItem !== firstVisibleItemRef.current) {
      callbackRef.current({ item: firstVisibleItem, index: viewableIndicesRef.current[0] })
      firstVisibleItemRef.current = firstVisibleItem
    }
  }, [firstVisibleItem])

  const trackVisibleIndices = useCallback(
    (info: { viewableItems: ViewToken[]; changed: ViewToken[] }) => {
      viewableIndicesRef.current = info.viewableItems.map(v => v.index).filter(isTruthy)
    },
    []
  )
  return trackVisibleIndices
}

function renderItem({ item }: { item: number }) {
  return <Text>Item: {item}</Text>
}
brunolcarlos commented 1 year ago

The problem its not if is firing or not, the problem is because the Component still static and nothing change. Try to create a tiktok feed and make a videos auto play, will not work

v-x2 commented 1 year ago

Still not fix?

zispidd commented 1 year ago

same problem

arnabadhikari2009 commented 1 month ago

onViewableItemsChanged not fire every time even after change in viewabilityConfig.I used Flatlist as a pager in class componant where I want the current visible item index. onViewableItemsChanged is ony fire at the last index . How coud I get the Index .. please help..