bamlab / react-tv-space-navigation

A React Native module to handle spatial navigation for a TV application in a 100% cross-platform way
https://bamlab.github.io/react-tv-space-navigation/
MIT License
217 stars 19 forks source link

Support for using an external SpatialNavigationScrollView on SpatialNavigationVirtualizedGrid #46

Closed adheus closed 8 months ago

adheus commented 9 months ago

Is your feature request related to a problem? Please describe. I have a screen that have a header with details about a collection and a grid right below it with the content of this collection. When navigating through the items, I want the header to be scroll out together with the focus. Currently, we have two scroll animations, one when you leave focus from the header to the grid items and then the grid items scrollview, which will scroll on top of the header.

Describe the solution you'd like Maybe SpatialNavigationVirtualizedGrid can support different span sizes through rows or a simple header. Or they can detect that their are inside a nested ScrollView and make use of it if flagged.

Focus on header:

image

Focus on grid:

image

Code is somewhat like:

<SpatialNavigationScrollView>
      <Header />
      <SpatialNavigationVirtualizedGrid />
</SpatialNavigationScrollView>
pierpo commented 9 months ago

Hi!

Thanks, this is indeed a very common need. And it is very interesting to notice that it could have been anticipated on our side given the way we implemented it :joy:

"Scrolling" in our virtualized list is all fake: it's actually translations (as you probably noticed). I don't think it would make sense to use the parent scroll view. If you did that, then you should just have regular elements displayed as a grid (btw, I am just noticing that this isn't even supported as our only way to have grids today is by using SpatialNavigationVirtualizedGrid).

So, the "best" way to handle it is to indeed have your header... Inside the grid, as a first row that has a different size. Which is not supported today. To do it, I'm not even sure of the API we should expose.

Either a flexible but unclear one (as in the data array we'd mix rows and elements together...?):

<SpatialNavigationVirtualizedGrid
  data={[{ type: 'header' }, { type: 'element', id: 1, thumbnail: 'a.jpg' }, { type: 'element', id: 2, thumbnail: 'a.jpg' }, { type: 'element', id: 3, thumbnail: 'a.jpg' }, { type: 'element', id: 4, thumbnail: 'a.jpg' }]}
  getElementSize={({type}) => { if (type==='header') return { height: 400, width: 400 } else return { width: 100, height: 40 }  }}
  renderItem={(element) => {if (element.type === 'header') return <Header /> else return <Element />}}
/>

Either a less flexible one but way clearer and probably easier to handle. But closed for future changes / less extensible :thinking:

<SpatialNavigationVirtualizedGrid
  data={[{ type: 'element', id: 1, thumbnail: 'a.jpg' }, { type: 'element', id: 2, thumbnail: 'a.jpg' }, { type: 'element', id: 3, thumbnail: 'a.jpg' }, { type: 'element', id: 4, thumbnail: 'a.jpg' }]}
  header={<MyHeader />}
  headerHeight={400}
  renderItem={ /* etc. */ }
/>

One last and more achievable way would be to simply open up SpatialNavigationrNode so that it includes the grid option. It is supported by LRUD (the navigation lib under the hood), we only need to pass down the grid props. You will lose the benefits of SpatialNavigationVirtualizedGrid (which is performance and smoother scroll), but it would be way simpler to implement.

I think this should be the way to go. Regarding the implementation, maybe simply adding the isGrid option on SpatialNavigationNode that passes isIndexAlign down to LRUD would suffice.

pierpo commented 9 months ago

As a workaround, you can use a non virtualized grid for now 👍 It works if you don't have infinite data. Scrolling won't be as smooth, though (even with a few elements, as the virtualized grid is a CSS scroll and not a ScrollView scroll 😁).

Here's an example: https://github.com/bamlab/react-tv-space-navigation/pull/51

adheus commented 9 months ago

Thank you, @pierpo! I'll give it a try. When you say "you don't have infinite data", do you also mean pagination?

pierpo commented 9 months ago

Yes I meant pagination 😊

We're working on the virtualized grid with a header props anyways. We're going for this API:

<SpatialNavigationVirtualizedGrid
  data={[{ id: 1, thumbnail: 'a.jpg' }, { id: 2, thumbnail: 'a.jpg' }, { id: 3, thumbnail: 'a.jpg' }, { id: 4, thumbnail: 'a.jpg' }]}
  header={<MyHeader />}
  headerHeight={400}
  renderItem={ /* etc. */ }
/>
pierpo commented 9 months ago

You can check out the example here for a non virtualized grid with a header :)

https://bamlab.github.io/react-tv-space-navigation/

image
pierpo commented 9 months ago

Btw, we're still working on this. First step is to support different element sizes for our virtualized lists. Draft is here https://github.com/bamlab/react-tv-space-navigation/pull/61

The grid being a list of virtualized rows, we'll be able to add the header as a bigger row, and the scrolling will probably work as expected 👍

pierpo commented 8 months ago

Finally merged the header support 🎉 https://github.com/bamlab/react-tv-space-navigation/pull/64

You'll find the props in the newest version 1.5.0 😊