Coder Social home page Coder Social logo

Comments (11)

asgvard avatar asgvard commented on June 2, 2024 7

Hello! Here is the new version of this library that is migrated to hooks + Typesript: https://github.com/NoriginMedia/Norigin-Spatial-Navigation

We have decided to go with the ref approach to link the DOM node with the hook, and we are using Context.Provider that will pass a value of the "parent component focus key" down to the next child using this context. Since React contexts can be nested, you can have multiple layers of parent components passing their focus keys down. Inspired by this article.

from react-spatial-navigation.

TrebuhD avatar TrebuhD commented on June 2, 2024 4

Thanks for your answers @asgvard ! I'll see if I can find some time soon to work on this and make a PR.

from react-spatial-navigation.

asgvard avatar asgvard commented on June 2, 2024 2

Hi!

Yes, it's been on our roadmap for quite a long time (as you mentioned) :)
Hopefully, soon we will get some dedicated time to update this library to the modern standards including TS and Hooks.
There are certain restrictions/complications with using hooks in terms of how we get the reference to the current component in order to pass it to the Spatial Navigation service. Currently we are using findDOMNode(this), where this is pointing to the current instance of the functional component, allowing React to find the corresponding DOM element for it. By using hooks you will need to somehow pass this reference from the parent, which would inherently add some boilerplate code.
So if you have time to try it out and port existing HOC into Hook with similar APIs, please do not hesitate to propose a PR! :)

from react-spatial-navigation.

asgvard avatar asgvard commented on June 2, 2024 1

We are also aware that findDOMNode has been deprecated in React Strict Mode so we will have to look for alternatives.

from react-spatial-navigation.

GeraldHost avatar GeraldHost commented on June 2, 2024 1

@asgvard An idea on how you could track the parent/child relationship could look something like this:

const useFocus = (props) => {
  const trackChildren = props?.trackChildren;
  const { layer, nextLayer } = useContext(Ctx);
  const thisLayer = React.useRef(layer.current);
  React.useMemo(() => {
    trackChildren && nextLayer();
  }, []);

  return { layer: thisLayer.current };
};


export default function App() {
  const layer = React.useRef(0);
  const nextLayer = () => (layer.current = layer.current + 1);

  return (
    <Ctx.Provider value={{ layer, nextLayer }}>
    </Ctx.Provider>
  );
}

Usage would look like:

const Component = () => {
  const { layer } = useFocus({ trackChildren });
  return ( ... );
};

What you will end up with is all siblings will have the same value for layer.

Here is a code sandbox to demonstrate: https://codesandbox.io/s/clever-einstein-442hj?file=/src/App.js:137-369

from react-spatial-navigation.

marnusw avatar marnusw commented on June 2, 2024 1

I've given this some thought and I can't come up with an idea how one might have a single context and create a tree structure without each new node being told what it's parent is either.

I would suggest that an update of the HOC implementation without using recompose would be nice! But that's not related haha!

I think this might actually be related. Recompose still uses the legacy context API which seems to be convenient in withFocusable, but is not sustainanble because it will eventually be removed. This means inevitably the implementation will have to move away from recompose and to the new context API, and that means, with everything else kept the same, a new Provider will have to be instantiated by each HOC to pass its focus key down as the parent key of its children. This seems unfortunate at first, but then, it is just the way Context works. It forces the implementation in the direction of hooks though, and that is a good thing.

Accepting that every parent will have to render a context provider still leaves the problem that all leaf nodes will be rendering providers unnecessarily. This could easily be solved with a prop/option like isLeaf which can have it render without an additional provider. But, at this point the leaf HOCs will pretty much just be registering with the navigator and pulling its own focus state from the context, preceisely the proposal for useFocusable.

Therefore, perhaps it's reasonable to consider that each parent in the navigation hierarchy will need to remain a HOC (although one could probably expose the provider etc. to set this up manually if users insist on not using HOCs), and that leave nodes would only need to use a hook.

from react-spatial-navigation.

asgvard avatar asgvard commented on June 2, 2024

You might want to check some of the similar issues discussed previously about alternatives to findDOMNode in Functional Components:
facebook/react#16699
reactjs/rfcs#97

The main reason why it is tricky, is because when using useRef you have to manually attach this ref to certain DOM element, so instead of:

const { focused, setFocus, stealFocus } = useFocusable({ 
  trackChildren: true,
  forgetLastFocusedChild: true,
});

It will be more like:

...
const myRef = useRef(null);
const { focused, setFocus, stealFocus } = useFocusable({ 
  trackChildren: true,
  forgetLastFocusedChild: true,
  node: myRef
});

return (<div ref={myRef}>...</div>);

Which is already a bit much compared to just withFocusable() call and from the first look we didn't see any easier ways to do this without increasing the amount of boilerplate code, which is the main reason this library has been created - to allow very quick wrapping of react components into "focusable" ones.

from react-spatial-navigation.

TrebuhD avatar TrebuhD commented on June 2, 2024

@asgvard I took a quick look and the code and I think we could move this to hooks using your API proposal (passing the ref). If we do that, we shouldn't need findDOMNode anymore either.

It would come with a tradeoff of making the usage of this library more verbose. I'm not aware of any way to automatically obtain a ref to the component in which the hook is used.

Then there's the question of passing parentFocusKey without Recompose's withContext. I haven't found a direct replacement for this. We can of course use useContext, but we'd have to find a way to create some global map of child -> parent key relationships and store those refs. This would likely mean we'd have to pass the parent key explicitly, which is rather ugly and complicates things:

const myRef = useRef(null);

const { focused, setFocus, stealFocus } = useFocusable({ 
  trackChildren: true,
  forgetLastFocusedChild: true,
  node: myRef,
  focusKey: "myComponent",
  parentFocusKey: "myParentComponent",
});

return (<div ref={myRef}>...</div>);

What's your view on these tradeoffs? This could probably be improved, but it does look like a considerable refactor so it's probably better to gather everyone's ideas before starting any actual work.

Edit: I'm aware this is not even close to how easy it is to use this library with the existing HOC. This post is meant as a starting point for a discussion about the main obstacles.

from react-spatial-navigation.

GeraldHost avatar GeraldHost commented on June 2, 2024

@asgvard @TrebuhD I think the proposal to have to manually assign to ref is an acceptable one. Another react library I use called react-hook-form does exactly this. It returns a register function which is essentially just a ref callback and gets used like this.

const { register } = useForm( ... );
...
<input ref={register} name="fname" />

In terms of developer experience I think it's pretty nice and doesn't really give any crazy cognitive overload! Also in terms of focus it would allow the user to define which element is the target rather than just assuming it's the root (or first child) element as findDomNode does. This could open up this library to some super useful behaviours.

Finally I think there is something nice about the fact useForm returns register instead of inputRef or similar. It seems to describe the fact that you are registering this input into your form and stops the user getting confused or concerned with the implementation.

from react-spatial-navigation.

asgvard avatar asgvard commented on June 2, 2024

@GeraldHost The layers approach could have issues if you have multiple children under sibling parents, for example:

-ParentA (layer1)
---Child1 (layer2)
---Child2 (layer2)
-ParentB (layer1)
---Child3 (layer2)
-ParentC (layer1)
---Child4 (layer2)
---Child5 (layer2)

All Child components here will have layer 2, but they are child of different parent components, so it's not enough to determine siblings properly. E.g. Child5 is not a sibling of Child1.

I was trying to think on how to replace withContext and unfortunately I don't have other ideas so far. Even if using useContext, we still need Context.Provider, which means you have to wrap every group of children components into separate provider. Even by doing this, you still don't know which Context to use later deep in the tree:

-FocusableParent (creating Context and wrapping the next component with <Context.Provider>)
---NonFocusableChild
------NonFocusableChild2
---------FocusableIChild1 (Even when using `useContext` here, we don't know how to get the Context created by FocusableParent above, as there could be many layers of such "parents" with their own contexts)

The idea of having a list of parent-child relationships is probably the most realistic one, however it might quickly become ugly as well, since if your FocusableChild component is many layers deep from the parents - you need to somehow pass the parentKey down many layers, resulting into prop drilling. The one beautiful thing about the current implementation is that in most cases you even need to specify the "focusKey" - the library will auto generate it, and all children will know it as well. While in case of passing down the parentKey - you have to know it somehow.

I think this parent-child issue is the most blocking right now for the migration to hooks. The approach with the "register" function to pass the ref into it is perfectly fine. I'd say it's even better than the current implementation since you can choose which "portion" of the component will be focusable. For example if you have a complex UI component and you only want certain "area" of it to be considered as focusable (including that area coordinates and size), then you can take the "ref" of that area and provide it to "register" method 👍

But yes, this parent-child thing is what is blocking us I think. I was thinking if it's possible to change the approach and completely get rid of the parent-child concept in this library, but this introduces other challenges especially in the scrolling lists, where you need to know the sibling components, and avoid the focus from jumping to the next "upper" layer focusable elements that are visually closer on the screen, but actually outside of the scrolling lists.

Brainstorming continues 🤔

from react-spatial-navigation.

GeraldHost avatar GeraldHost commented on June 2, 2024

@asgvard Yes you are exactly right! I left a comment on my PR regarding this. I also agree that without a way of passing the parent key down this hooks implementation would be awkward! I would suggest that an update of the HOC implementation without using recompose would be nice! But that's not related haha!

from react-spatial-navigation.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.