-
Notifications
You must be signed in to change notification settings - Fork 64
Create React hook (e.g useFocusable
) that compliments the existing HOC.
#77
Comments
Hi! Yes, it's been on our roadmap for quite a long time (as you mentioned) :) |
You might want to check some of the similar issues discussed previously about alternatives to The main reason why it is tricky, is because when using 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 |
We are also aware that |
Thanks for your answers @asgvard ! I'll see if I can find some time soon to work on this and make a PR. |
@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 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 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. |
@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 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 Finally I think there is something nice about the fact |
@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 Here is a code sandbox to demonstrate: https://codesandbox.io/s/clever-einstein-442hj?file=/src/App.js:137-369 |
@GeraldHost The layers approach could have issues if you have multiple children under sibling parents, for example:
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
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 🤔 |
@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! |
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 think this might actually be related. Recompose still uses the legacy context API which seems to be convenient in 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 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. |
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 |
Is your feature request related to a problem? Please describe.
Hooks were introduced 2 years ago in React 16.8. With class-based components decreasing in popularity compared to functional components, it would make sense to create a hook as an alternative to the
withFocusable
HOC.Describe the solution you'd like
The hook could work like this:
Additional context
This is highly desireable, since many projects are not using HOCs anymore. In our project,
withFocusable
is the last remaining HOC.Please let me know if you'd like help with this.
The text was updated successfully, but these errors were encountered: