Downshift: broken keyboard navigation in safari

[httpete-ire]

• downshift version: 1.22.1
• node version: n/a
• npm (or yarn) version: n/a
• chrome version: 62.0.3202.94
• safari version: 11.0.1

What you did:

In the dropdown example here, in safari when the dropdown is clicked the menu opens but trying to navigate the items with the keyboard fails to work, in the other examples it works fine. Short video of the issue.

I am also seeing this behaviour in a dropdown I built with downshift.

What happened:

Nothing happened. Should have update the highlighted index

[kentcdodds]

Thanks for reporting @httpete-ire! I was able to reproduce it.

I don't have time to work on this issue myself, but if you or anyone else would like to debug this and discover what the core issue is that would be awesome!

[httpete-ire]

@kentcdodds looked into this. I dont think its an issue with downshift.

I could reproduce the issue with just react and vanilla js. Keydown gets fired on both of these examples in chrome, they both dont work in safari. I wonder if safari is ignoring the keydown listener on a button element.

[kentcdodds]

Thanks for digging @httpete-ire. Do you think we could come up with a workaround? We could either document how to implement the workaround or maybe even do the workaround within downshift itself.

[philipyoungg]

Have the same problem. My workaround, while not ideal, is to focus on the button when it's clicked.

[httpete-ire]

@philipyoungg could you please expand on your workaround 👍

[philipyoungg]

@httpete-ire

const Comp = props =>
  <Downshift>
    {
      ({isOpen, getButtonProps}) => {
        return 
          <div>
            <button 
              ref={node => this._button = node} 
              onClick={() => this._button.focus()} 
              {…getButtonProps()} 
            />
            {isOpen => <Content />}
          </div>
      }
    }
  </Downshift>

[httpete-ire]

@philipyoungg Thanks for the example. However spreading getButtonProps will override your onClick handler. I fixed it by doing this 👇 not ideal but at least it works. Thanks for the tip of focusing the button.

<button
  ref={node => (this._button = node)}
  {...getButtonProps()}
  onClick={(e) => {
    this._button.focus();
    getButtonProps().onClick(e);
  }}
>

https://codesandbox.io/s/o96643xm5z

[kentcdodds]

Actually @httpete-ire and @philipyoungg, you run into some problems doing that (watch this today for free before it's pro-only tomorrow).

Instead you should do:

<button
  ref={node => (this._button = node)}
  {...getButtonProps({
    onClick: () => this._button.focus()
  })}
>

It'd be awesome if someone could make a PR to the README to add/update an example :)

[kentcdodds]

Or maybe we could make this the default behavior 🤔

[httpete-ire]

@kentcdodds I think it should be default behavior because at the moment it doesnt work in the latest Firefox without adding the focus to the button.

[kentcdodds]

I think I know what's up. By default we focus on the input when you click that button but if there is no input rendered then we shouldn't do that. Should be a fairly simple fix.

[philipyoungg]

Yeah. Kent comment is the right way to do. I wrote the code snippet above in hurry. Sorry for the wrong guidance!

[kentcdodds]

But now you shouldn't have to do it at all because the library should handle it for you thanks to #278 :)

[philipyoungg]

Oh sorry! I meant we should've adding all additional props inside the 'getButtonProps' 😉