bbc / lrud

Left, Right, Up, Down. A spatial navigation library for devices with input via directional controls.
Apache License 2.0
98 stars 21 forks source link

Question about forceRefocus #35

Closed jfrux closed 4 years ago

jfrux commented 5 years ago

Why is it that we use "assignFocus" if something is unregistered instead of like... setting the activeChild but not moving focus to it?

If I want to focus on something, but have some things "unregister" in the background...

It's stealing focus away from my intended focus because I'm unregistering some things.

That being said, I'm aware that I can set forceRefocus to false... but what I'd really prefer is to simply setActiveChild.

I wonder if there is a way we could simply setActiveChild and optionally refocus if that parent is actually already focused... somehow?

jordanholt commented 5 years ago

Hey @jfrux, we could modify the forceRefocus option to update the active child of the parent without refocusing. Would this meet your use case?

However the situation you describe with regards to it stealing focus when unregistering a node that is in a different subtree to the subtree containing your focus sounds like a more significant bug. Could you provide a sample of a tree and options that reproduce this behaviour?

jfrux commented 5 years ago

The complexity of my app being fully cross-platform makes things especially trying. I'm sure there is probably a solve for this.

I'll tell you this, for whatever reason changing it from assignFocus to setActiveChild(parent,child) did solve my particular use case... I do feel like it could be a problem for some depending on the way their apps are working.

To be honest, I'm using registerOverride way more than its intended and I do wish for less complex interactions at some point but I don't think that's a problem with lrud rather than my particular implementation.

jfrux commented 4 years ago

@jordanholt Yeah I think it makes more sense to set the set the active child.

I'm currently forcing focus on the video player, but since I switched playlists... the playlist category below the player updated too and it pulled focus. It's kind of tricky... I don't know how this would affect everyones codebase, but it doesn't seem to affect mine when I swap them out.

jfrux commented 4 years ago

After reviewing recalculateFocus, it does appear that if I'm setting forceRefocus: false it shouldn't be recalculating focus... hmm.

jfrux commented 4 years ago

Sorry... we good... I just turned it off for all my nodes and my app is good.

Thanks and sorry for so many comments.

jordanholt commented 4 years ago

No problem! Glad to hear it's sorted ☺️