downshift: Enter event on form wrapped input should not have e.preventDefault()

Ok, great! It’s out and it’s actually downshift@1.29.0 (because this is a new feature 😅)

Thanks again!

Alright! So downshift@1.28.6 now supports event.preventDownshiftDefault = true. Much better IMO and will help us avoid this issue entirely 😃

Thanks everyone! Especially @ericedem for implementing it and those who contributed to the discussion in #353 👍 🎉

I’m glad that the solution seems to work for you! I’d love a PR. If you’d prefer to wait for your co-worker to weigh in that’s fine with me.

Hi @kentcdodds, my schedule’s been hectic but I’ll try to submit a PR for review this weekend 😸 !

Hi, @kentcdodds. Those are all super valid points, and I agree that it is not necessarily Downshift’s responsibility to implement (or even help provide a way to implement) all of the features for all of the different types of autocompletes, typeaheads, combo boxes, etc.—it could be a never-ending list of requests 😅

I knew I was missing some caveat with my one-off solution, haha. Thanks for explaining why it wouldn’t work. Your more generic solution of exposing the keydown handlers seems like a great idea for solving the form submission problem; I can’t wait to see it added to this library by @hzhu.

I am very grateful for how much is provided for free for all of us using OSS like this, and it’s even more pleasant that you (and other maintainers) are willing to actively maintain and engage in a discussion around the tools you release. Thanks again 💛

Ha! I knew that making the solution more generic was a good idea! This solution will also make the arrow key behavior easier to control as well (#336)

@hzhu I was working on this commit that seems to address the issue.

I’m not sure if it’s the most elegant. I had to control the isOpen and selectedItem of the Downshift component. I also had to pass the onSubmit function to the input element.

Something else that I tried playing with but couldn’t make it work: The Enter keydown event is prevented only when the menu is open (see code here). I had an idea to close the menu first, and then redispatch the Enter keydown event.

<Input
    {...getInputProps({
        placeholder: 'Enter color here',
        onKeyDown: e => {
            e.persist();
            // Use closeMenu action function that Downshift provides
            closeMenu(() => {
                // somehow redispatch the Synthetic event so that Downshift could take care of it
                // https://stackoverflow.com/questions/11974262/how-to-clone-or-re-dispatch-dom-events
                // this is where I failed 😟
            });      
        }
     })}
/>

I believe @hzhu was taking this one on

Why do you need to actually submit the form though? Why not just make your form a controlled component and let React handle the state of each element in the form?

Hi Austin, just to preface, I could be wrong as I’m pretty new to working with autocomplete UIs 🤐 . Some apps like Amazon.com implement their search autocomplete as a form like the <form> example in Downshift’s storybook. I think it would be standard behavior when a user hits ENTER that the <form>'s onSubmit handler is invoked. Note, the form doesn’t need to be submitted to a server. The page doesn’t need to refresh.

Thoughts? Anyone wanna try to take this on? We’ll need at least one or two tests and the keyDownHandlers prop will need prop types and a default value.

Hi Kent, I’d love to take this on as my first open source contribution to Downshift! I just tested your suggestion and it works. It seems non-invasive to the existing codebase. My teammate (who is helping me implement autocomplete at work using Downshift) has been working on a solution over the weekend. I’d love to see what he has in mind also!

What if we allowed you to do this:

const handlers = {
  Enter(event) {
    if (this.getState().isOpen) {
      this.selectHighlightedItem({
        type: Downshift.stateChangeTypes.keyDownEnter,
      })
    }
  }
}

const ui = <Downshift keyDownHandlers={handlers}>
 {() => <div>your ui</div>}
</Downshift>

This would be fairly trivial to implement. We’d just need to add a little bit of logic here. Probably change it to this:

input_handleKeyDown = event => {
  const handler = this.props.keyDownHandlers[event.key] || this.keyDownHandlers[event.key]
  if (handler) {
    handler.call(this, event)
  }
}

It would be an advanced feature and we should mention that in the docs to avoid using it because it exposes the downshift instance in a way that could lead to your code breaking if we decide to change a method you’re relying on…

Alternatively we could do: handler.call(this.getStateAndHelpers(), event) which would only allow you to access the same things you can access in the render prop… I think I like that better. It would require us to expose moveHighlightedIndex, but I’m cool with that.

Thoughts? Anyone wanna try to take this on? We’ll need at least one or two tests and the keyDownHandlers prop will need prop types and a default value.

The more I think about this the more I like it as a solution. It’s not the best, but it seems like a good way to get around this problem without complicating the codebase or the usage too much.

Hey @hzhu,

Thanks for the issue! I’m not sure of the best solution for this to be honest. I can think of several things you could do, but I’m not super fond of them.

I don’t think I want to add a prop especially for this case. I think we can come up with something else. I’d love people to give their input to solving this problem. Anyone have ideas?