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

onViewableItemsChanged not firing if items change without scroll 😔 #614

Open
1 task done
mfbx9da4 opened this issue Sep 26, 2022 · 12 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@mfbx9da4
Copy link

mfbx9da4 commented Sep 26, 2022

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:

  • iOS
    Android (untested)

Environment

1.2.2

@mfbx9da4 mfbx9da4 added the bug Something isn't working label Sep 26, 2022
@bfricka
Copy link

bfricka commented Sep 27, 2022

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
Copy link
Contributor

hirbod commented Sep 27, 2022

@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
Copy link

bfricka commented Sep 27, 2022

@hirbod Not OP, but my setup implements:

  • disableAutoLayout
  • estimatedItemSize
  • estimatedListSize
  • getItemType
  • keyExtractor
  • numColumns
  • onBlankArea
  • onViewableItemsChanged
  • overrideItemLayout
  • viewabilityConfig (tried various tweaks here, and even calling flashRef.recordInteraction() imperatively).

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
Copy link

eeshankeni commented Sep 28, 2022

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
Copy link
Author

mfbx9da4 commented Oct 20, 2022

@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
Copy link
Author

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

@eeshankeni I tried playing with itemVisiblePercentThreshold no joy!

@mfbx9da4 mfbx9da4 changed the title onViewableItemsChanged not firing onViewableItemsChanged not firing if items change without scroll 😔 Oct 20, 2022
@mfbx9da4
Copy link
Author

mfbx9da4 commented Oct 20, 2022

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

  • If the viewable indices being rendered changes => onViewableItemsChanged will fire correctly ✅
  • If the viewable indices being rendered does not change but the actual items being rendered does change => onViewableItemsChanged will not fire ❌

I wonder if this comes down to the fact that recycler list view does not support onViewableItemsChanged? Flipkart/recyclerlistview#713

@mfbx9da4
Copy link
Author

mfbx9da4 commented Oct 20, 2022

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
Copy link

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
Copy link

v-x2 commented Aug 3, 2023

Still not fix?

@zispidd
Copy link

zispidd commented Sep 2, 2023

same problem

@arnabadhikari2009
Copy link

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..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants