Developer should have ability to override or compose default onKeyDown functionality
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 is to have ability to override default behaviour for specific event Keys.
Possible use case:
Space key and on Enter key drop down should close and blur outconst onKeyDown = (event, originalFn) => {
switch(event.key) {
case 'Enter':
submitChangesAndFocusOut();
brake;
case 'Space':
selectFocusedItem();
brake;
default:
originalFn(event);
}
}
<Autocomplete {...props} onKeyDown={onKeyDown}>
@ivan-jelev Interesting, we have seen a similar concern raised in #19500. What do you think of my proposal at https://github.com/mui-org/material-ui/issues/19500#issuecomment-580867606? I think that we could move forward with it :).
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: https://github.com/downshift-js/downshift/pull/358.
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.
@oliviertassinari You want to cover only the first two cases?
@marcosvega91 For the third case, I think that it should only concern the hook API, which support it.
@oliviertassinari It means that you need to pass a parameter to Autocomplete component to understand if you want to override the default behavior ?
@marcosvega91 I don't understand, what do you mean?
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 馃コ
@marcosvega91 I think that that second and third cases should be identical.
Yes right as i said in the previous comment i didn't read the case well.
So i can create a PR for this :)
@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.
Most helpful comment
@ivan-jelev Interesting, we have seen a similar concern raised in #19500. What do you think of my proposal at https://github.com/mui-org/material-ui/issues/19500#issuecomment-580867606? I think that we could move forward with it :).