Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Autocomplete] Add ability to override/compose key down events in autocomplete #19887

Closed
1 task done
ivan-jelev opened this issue Feb 28, 2020 · 10 comments · Fixed by #23487
Closed
1 task done

[Autocomplete] Add ability to override/compose key down events in autocomplete #19887

ivan-jelev opened this issue Feb 28, 2020 · 10 comments · Fixed by #23487
Labels
component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@ivan-jelev
Copy link

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Developer should have ability to override or compose default onKeyDown functionality

Examples 🌈

Currently when i specify custom onKeyDown prop i'll get default functionality executed and only then the one i provided.

It makes not possible to override or compose default functionality for specific key pressed.

What i want to be able to do is:

<Autocomplete {...props} onKeyDown={(event, originalHandler) => {
  if (['Enter', 'ArrowLeft'].includes(event.key)) {
    customHandler(event);
  } else {
    originalHandler(event)
  }
}}>

Motivation 🔦

Motivation is to have ability to override default behaviour for specific event Keys.

Possible use case:

  • Need to be able to select options in dropdown with Space key and on Enter key drop down should close and blur out
const onKeyDown = (event, originalFn) => {
  switch(event.key) {
    case 'Enter':
       submitChangesAndFocusOut();
       brake;
    case 'Space':
      selectFocusedItem();
      brake;
     default:
        originalFn(event);
  }
}

<Autocomplete {...props} onKeyDown={onKeyDown}>
@oliviertassinari oliviertassinari added component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request labels Mar 1, 2020
@oliviertassinari
Copy link
Member

@ivan-jelev Interesting, we have seen a similar concern raised in #19500. What do you think of my proposal at #19500 (comment)? I think that we could move forward with it :).

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Mar 1, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 26, 2020

I had forgotten to link the previous benchmark I have run on the issue. Basically, I have been wondering about this very problem in the past for the potential touch handle conflicts between the components: once a component decides to handle an event, the other components shouldn't intervene. The idea for event.muiHandled?: boolean; started in https://github.com/mui-org/material-ui/pull/17941/files#diff-dfe2f9b3bcff520f425e96bfe824362cR312. We already use it in production.

Then, I have noticed that Downshift went down the same road: downshift-js/downshift#358.


Event Handlers

You can provide your own event handlers to Downshift which will be called before the default handlers:

const ui = (
  <Downshift>
    {({getInputProps}) => (
      <input
        {...getInputProps({
          onKeyDown: event => {
            // your handler code
          }
        })}
      />
    )}
  </Downshift>
)

If you would like to prevent the default handler behavior in some cases, you can set the event's preventDownshiftDefault property to false:

const ui = (
  <Downshift>
    {({getInputProps}) => (
      <input
        {...getInputProps({
          onKeyDown: event => {
            if (event.key === 'Enter') {
              // Prevent Downshift's default 'Enter' behavior.
              event.preventDownshiftDefault = false
              // your handler code
            }
          }
        })}
      />
    )}
  </Downshift>
)

If you would like to completely override Downshift's behavior for a handler, in favor of your own, you can bypass prop getters:

const ui = (
  <Downshift>
    {({getInputProps}) => (
      <input
        {...getInputProps()}
        onKeyDown={event => {
          // your handler code
        }}
      />
    )}
  </Downshift>
)

In this context, it seems like a straightforward direction to take.

@marcosvega91
Copy link
Contributor

@oliviertassinari You want to cover only the first two cases?

@oliviertassinari
Copy link
Member

@marcosvega91 For the third case, I think that it should only concern the hook API, which support it.

@marcosvega91
Copy link
Contributor

@oliviertassinari It means that you need to pass a parameter to Autocomplete component to understand if you want to override the default behavior ?

@oliviertassinari
Copy link
Member

@marcosvega91 I don't understand, what do you mean?

@marcosvega91
Copy link
Contributor

marcosvega91 commented Apr 26, 2020

To implement the third behavior we need a way fro the user to tell to Autocomplete component what it will have to do.
How Autocomplete will understand that it must call default behavior and then user one?

Now the onKeyDown is passed through useAutocomplete hook and there the callback is called at the end of onKeyDown function.

Maybe I didn't understand the case 🥳

@oliviertassinari
Copy link
Member

@marcosvega91 I think that that second and third cases should be identical.

@marcosvega91
Copy link
Contributor

Yes right as i said in the previous comment i didn't read the case well.

So i can create a PR for this :)

@marcosvega91
Copy link
Contributor

@oliviertassinari There is a test that now fails because of changing.
The test do the following on keyDown

onKeyDown={(event) => {
    if (!event.defaultPrevented && event.key === 'Enter') {
         handleSubmit();
     }
}}

Because of the user keydown function is on the top of keydown listener in useAutocomplete, defaultPrevented will never be true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants