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
178 stars 15 forks source link

Fix wrong focus direction. #110

Closed fulminant closed 2 months ago

fulminant commented 2 months ago

Fixed issue with incorrect focus direction when node appears conditionally. Also, added a separate page inside the example project to demonstrate the problem and its solution. Fixes https://github.com/bamlab/react-tv-space-navigation/issues/109#issue-2250291473

pierpo commented 2 months ago

Love the idea of having it in the example. It makes it so much more visible. I'll review properly later, as I said in the issue I think it's better not to use the index as a solution 😁

Doesn't mean we can't expose it, though!

Thanks 👍

pierpo commented 2 months ago

I don't think we'll need to expose the index yet. I'm not sure we want to give that ability, as it will only pollute the API and lead to unexpected behaviours 😁 We'll see later if we really do need this, but there was a cleaner solution to your problem so I guess nothing urgent 😎

Thank you so much for the contribution!

fulminant commented 2 months ago

Maybe you're right. For me, setting the index property is a clearer way to control this behavior, but it won't work for large-scale applications.

Thanks for the update.

pierpo commented 2 months ago

I'm still thinking this through. If that's clearer for you, maybe we can add this feature as experimental 😁 Would you rather have the index exposed than the solution I proposed?

In my opinion, it feels a little clunky to add index to all elements, that's what I don't like with the approach. But if people prefer the index option, I can't argue with that 😁