NoriginMedia / react-spatial-navigation

DEPRECATED. HOC-based Spatial Navigation. NEW Hooks version is available here: https://github.com/NoriginMedia/norigin-spatial-navigation
MIT License
226 stars 64 forks source link

Components wrapped by withFocusable cannot use setFocus during componentDidMount. #31

Closed shirakaba closed 2 years ago

shirakaba commented 5 years ago

Describe the bug

I have a class component, DismissButton, which needs to steal focus as soon as it mounts. The use-case is an error popup that appears and steals the spatial navigation focus to one of its buttons (such as "Dismiss" or "Retry"). Here is how I've written it:

import { withFocusable } from "@noriginmedia/react-spatial-navigation";
import * as React from "react";
import { Text, TouchableOpacity } from "react-native";

class DismissButton extends React.PureComponent {
    ref = React.createRef();

    componentDidMount(): void {
        const { stealFocusOnAppearance, focusable, focusKey, setFocus } = this.props;

        if (stealFocusOnAppearance && focusable) {
            console.log(`[DismissButton.componentDidMount] stealing focus for: "${focusKey}"`);
            setFocus(); // Fails because addFocusable(/* ... */) has not occurred yet
        }
    }

    render() {
        const { text = "Dismiss", focused, onClick } = this.props;

        return (
            <TouchableOpacity ref={this.ref} onPress={onClick}>
                <Text style={{ backgroundColor: focused ? "yellow" : "white" }}>{text}</Text>
            </TouchableOpacity>
        );
    }
}

const DismissButtonFocusable = withFocusable({})(DismissButton);

As you can see, DismissButton has its own componentDidMount() method in which it calls this.props.stealFocus(), but at this moment, it appears that DismissButton has not been registered in focusableComponents. Registration (by addFocusable()) happens only during the componentDidMount() lifecycle method added to the DismissButtonFocusable HOC by the recompose library.

To my understanding, DismissButton mounts before the HOC, and thus the order is:

DismissButton.componentDidMount() // In which we call setFocus()
DismissButtonFocusable.componentDidMount() // In which addFocusable() runs

If I wrap DismissButton.componentDidMount()'s call to setFocus() in a timeout, then it succeeds.

For some reason, there is no such problem in the "Menu" class component in the repo's sample app, though:

https://github.com/NoriginMedia/react-spatial-navigation/blob/master/src/App.js#L145

I'm wondering if I'm missing something.

asgvard commented 5 years ago

Hi,

hmm, to me it looks like you might use older version (or branch) where this is not present. This happens because your wrapped component gets mounted before its withFocusable HOC is able to call addFocusable. In order to compensate for this we have a small hack that allows you to set focus on non-existent components here, and then this components will be picked up here after it is mounted. That was the main reason why we decided to allow manual setFocus on any component even if it doesn't exist or has focusable=false.

asgvard commented 2 years ago

Closing due to inactivity. We have also released a new hook based version of this library here: https://github.com/NoriginMedia/Norigin-Spatial-Navigation