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
205 stars 18 forks source link

SpatialNavigationScrollView should have a way to scroll to a specific item without focusing the view #106

Closed jonp-iversoft closed 5 months ago

jonp-iversoft commented 5 months ago

Is your feature request related to a problem? Please describe. There should be a way to scroll a SpatialNavigationScrollView to a specific item without focusing the SpatialNavigationScrollView itself

Describe the solution you'd like There is a way using a scrollItemRef.current.focus() to focus and scroll to an item in a SpatialNavigationScrollView, but there is no way to scroll to an item in a list without focusing the referenced item. This would be useful for a menu scrolling to a specific section of the content, but not focusing the content unless the content is navigated to. Such as a library of items with a letter based navigation to fast-forward to the items starting with a selected/hovered letter

image

So in this screenshot, when the letter e is selected, the SpatialNavigationScrollView should have a way to scroll to the items starting with the letter e without focusing the first item in the grid

pierpo commented 5 months ago

That's a very interesting feature indeed 😊

What API would you expect exactly? 😁

jonp-iversoft commented 5 months ago

I don't terribly mind the existing scrollItemRef.current.focus() 'api', maybe SpatialNavigationScrollViewRef.current.scrollTo(index) or SpatialNavigationScrollViewRef.current.scrollTo(scrollItemRef.current) - whatever is easier based on the existing logic to scroll when scrollItemRef.current.focus() is called

It'd be useful on the SpatialNavigationScrollView as well as the SpatialNavigationVirtualizedList and SpatialNavigationVirtualizedGrid

scrollItemRef.current.scrollTo() would be even better and bypass the need to interact/change the scroll lists at all

pierpo commented 5 months ago

I'm suggesting a ref method on focusable views and nodes called triggerScroll(). Does it work for you? See the PR #107 :)

jonp-iversoft commented 5 months ago

scrollItemRef.current.triggerScroll() works for me! Sounds like a simple yet effective solution!

pierpo commented 5 months ago

Hey @jonp-iversoft!

I'm having second thoughts about the API I suggested 🙈 I think I jumped on a solution without thinking it enough.

We could simply expose the ref of the ScrollView inside SpatialNavigationScrollView, right? Then you'd access the React Native scrollView imperative API and do whatever you want.

Do you agree that it is a better solution?

Thanks 🙌

jonp-iversoft commented 5 months ago

That could work, the solution would end up being something like this then?

const items = ['Item 1', 'Item 2', 'Item 3', 'Item 4', 'Item 5'];
const scrollViewRef = useRef(null);
const itemRefs = useRef(items.map(() => createRef()));

const scrollToItem = (index, direction) => {
  itemRefs.current[index].current.measureLayout(
    scrollViewRef.current,
    (x, y) => {
      scrollViewRef.current.scrollTo({
        x: direction === 'horizontal' ? x : 0,
        y: direction === 'vertical' ? y : 0,
        animated: true,
      });
    },
    (error) => {
      console.error('Failed to find element', error);
    },
  );
};

// Scroll to item 5
scrollToItem(5, 'vertical');

return (
  <SpatialNavigationScrollView scrollViewRef={scrollViewRef}>
    {items.map((item, index) => (
      <View key={index} ref={itemRefs.current[index]} style={styles.item}>
        <Text>{item}</Text>
      </View>
    ))}
  </SpatialNavigationScrollView>
);

That'd give a good amount of flexibility, at the 'cost' of a less integrated solution

Would the offsetFromStart need to also be applied to the scrollTo value? Not 100% sure how that's used by the library

pierpo commented 5 months ago

Yes definitely, would be something like this. Actually, I can say that it is something like this because I just published the change 😂 You'll find it in 3.4.0 🎉

<SpatialNavigationScrollView ref={scrollViewRef}>
  ...
</SpatialNavigationScrollView>

The ref is found under ref, as expected. You'd need more tweaks on your side indeed, but I don't think it is that common and that anyone would implement it the same way. So it's better to keep this part off the library for now, and keep it simple on the lib's side for now.

Regarding offsetFromStart, this might be subject to change in the future but for now it helps to have an offset when you scroll to an element. Otherwise, when you focus an item then the scrollview will align the item with the top of the screen. Usually you want it on the middle of the screen, and that's when you add an offset.

If you manually scroll to the scrollview, you'll need to mention the offset yourself (by adding it's value to your scroll logic), because we're exposing the raw scrollTo of react native 😊

pierpo commented 5 months ago

I'm closing as it is published, feel free to reopen if you have any other problem 😊

jonnypage commented 5 months ago

You're a legend! Thanks so much for getting that out.