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

Error when trying to assign ref #29

Closed mgenev closed 5 years ago

mgenev commented 5 years ago

Hi, guys, brilliant work on this library. The only problem I've found so far is here:

Describe the bug When I create myself a component, then wrap it with the focusable HOC, if I try to assign a ref it breaks...

To Reproduce

const Li = styled.li`
  grid-column-end: span ${props => props.span};
  border: 3px solid ${props => props.focused ? 'red': 'yellow'};
`

const GridItemFocusable = withFocusable()(Li);
...

  constructor(props) {
    super(props);
    this.wrapperRef = React.createRef();
  }
...
<GridItemFocusable              
              ref={this.wrapperRef}
              onBecameFocused={this.onGridItemFocused}
              {...this.props}
            >

TypeError: Cannot read property 'scrollIntoView' of null
> 23 |   this.wrapperRef.current.scrollIntoView()

Expected behavior I expect to be able to use refs so I don't have to use a redundant span in this case like this:

<GridItemFocusable              
              onBecameFocused={this.onGridItemFocused}
              {...this.props}
            >
              <span ref={this.wrapperRef}>                 
                {this.props.children}
              </span>
            </GridItemFocusable>

Styled Components supports the ref, but it seems to break after the wrap with the HOC

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

asgvard commented 5 years ago

This is because we use recompose HOC in withFocusable and it doesn't support ref. However there is a similar issue that is solved with a helper HOC: https://github.com/acdlite/recompose/issues/640#issuecomment-441109620 Does this solve your problem?

asgvard commented 5 years ago

Closing since this issue cam be solved with another HOCs. Also we will perhaps add a possibility to provide ref when we migrate to hooks instead recompose.