react-autocomplete icon indicating copy to clipboard operation
react-autocomplete copied to clipboard

fix #287 - checks itemNode is not null in maybeScrollItemIntoView

Open SkinyMonkey opened this issue 8 years ago • 7 comments

When an async function is loading the data:

The itemNode is sometime null in this function : https://github.com/reactjs/react-autocomplete/blob/master/lib/Autocomplete.js#L245.

In the ref function of the renderMenu member function (https://github.com/reactjs/react-autocomplete/blob/master/lib/Autocomplete.js#L437), the ref parameter is null too and sets the refs[item- + index] to null

I tried to not set the key in the ref function if the value was null but it didn't help.

So ultimately I decided to protect the call to scrollIntoView to avoid the exception.

I hope it can fix the problem until you find a better solution if there is one.

PS : sorry, i forgot to read the contribution rules before commiting. My commit is prefixed by fix though.

SkinyMonkey avatar Nov 02 '17 05:11 SkinyMonkey

Same as #281

eduduardo avatar Nov 07 '17 17:11 eduduardo

I'll close it as soon as the other pull request is merged : it's been needed for a long time, a little reminder for the admins/maintainers doesn't hurt.

SkinyMonkey avatar Nov 07 '17 17:11 SkinyMonkey

That's right 😄

eduduardo avatar Nov 07 '17 19:11 eduduardo

What about merge this PR? I'm using 1.5.5 version of react-autocomplete with rendering menu to body (due to issues with overflow: hidden blocks) and everything is working great. But from 1.5.7 I get error with nullable itemNode on first render with highlighted item (e. g. arrow down press). I need an update because of renderInput is useful feature for me.

/cc @CMTegner

nikmilson avatar Nov 20 '17 07:11 nikmilson

I am still having issues with this always being null, so quickly i monkey-patched this function, by using inheritance ( fast and dirty). checkout the component scroll-into-view, it's pretty sweet.

I also don't show scrollbars in my app because they are king of unnecessary, this stops the outer window form bouncing.

maybeScrollItemIntoView() {
   const stopMainContentFromScrolling = () => {
     const mainContent = window.document.getElementsByClassName('main_content')[0]
     const contentY = mainContent.scrollTop
     if (mainContent.style) {
       mainContent.style.overflowY = 'hidden'
       mainContent.style.top = contentY
       setTimeout(() => {
         mainContent.style['overflow-y'] = 'auto'
         mainContent.style.top = 0
       }, 1000)
     }
   }

   if (this.isOpen() && this.state.highlightedIndex !== null) {
     const { className } = this.props
     const { highlightedIndex } = this.state
     const elements = window.document.getElementsByClassName(className)
     const itemNode = elements[highlightedIndex]
     stopMainContentFromScrolling()
     scrollIntoView(itemNode)
   }
 }

reduxdj avatar Dec 26 '17 23:12 reduxdj

As I've mentioned before, this is most likely due to a mismatch between what you are passing in as props.items and the render-tree that your renderMenu method is returning.

This is documented:

Ensure the returned tree includes every entry in items or else the highlight order and keyboard navigation logic will break.

If you think that this is not the case I'd be happy to look over a reduced test case to confirm that there is in fact a different bug in play.

CMTegner avatar Feb 04 '18 09:02 CMTegner

I got this error (and several others) when I was trying to use a functional component in the render* prop. I did not know that this library uses refs. As you know, functional components do not support refs. I think it should be mentioned in the documentation.

Example (react 15.6.1): https://codesandbox.io/s/32lz0wmmm1 Example (react 16.2.0): https://codesandbox.io/s/8xxq2l5910

With react 16 there is additional warning:

Warning: Stateless function components cannot be given refs. Attempts to access this ref will fail.

kaskazurek avatar Mar 08 '18 20:03 kaskazurek