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

useKeyPress hook should support a way to filter out input, textarea, contenteditable #2364

Closed
websiddu opened this issue Nov 11, 2023 · 6 comments

Comments

@websiddu
Copy link

When implementing keyboard shortcuts for an App, we should filter keyboard events from input, textarea, contenteditable, select etc...

Please provide an option to filter out this element.

@BastKakrolot
Copy link

I think filtering is not needed, but the return method is used to control the opening and closing of the response, which can be handled more flexibly.
what you think?

@liuyib
Copy link
Collaborator

liuyib commented Nov 14, 2023

Hi @websiddu , please see: #2365, I think this is a more general way to solve your problem. Is that right?

@websiddu
Copy link
Author

In my code today,

export const isInputElement = (el: any) => {
  if (!el) return false;

  const tagName = el.tagName.toLowerCase();
  const inputTypes = ["input", "textarea", "select"];
  if (inputTypes.includes(tagName)) return true;

  return false;
};
useKeyPress("n", (e: any) => {
    if (isInputElement(e.target)) return;
    createNew();
  });
  

The above works for me.

But I feel the default behavior should be that, when focused on a text input it shouldn't bubble the events, to parents unless its explicitly said so.

And finally I'm curious on how will the observe prop going to help in my case?

@BastKakrolot
Copy link

I don't think this point of view is correct. For example, the editor has many shortcut keys. We should focus on the hooks themselves and should not bind them to the actual scene.

Therefore, I think we can deal with more scenarios by letting go of control and handing it over to users.

const Demo = () => {
    const [observe,{setFalse,setTrue}] = useBoolean(true)
    useKeyPress("n", callback, {
        observe
    });
    return <Input onFocus={setFalse} onBlur={setTrue} />;
};

@liuyib
Copy link
Collaborator

liuyib commented Nov 14, 2023

@websiddu In your case, your code is more suitable and simple, but it seems to be solving a specific problem.

I imagined a similar case: "The engineer wants to set [Shift + v] to paste in some textarea and [Shift + w] not to be fire in textarea", In this situation, it is inappropriate to activate or deactivate hotkeys within the textarea.

So I'm sorry, the logic for filtering input elements won't be built into ahooks here. Do you have any other ideas? Please feel free to discuss them here.


By the way, your code can be simpler to solve your problem:

useKeyPress("n", (e: any) => {
    if (document.querySelector(':focus')) return;
    createNew();
});

@websiddu
Copy link
Author

That is totally cool. Thanks for your work here.

if (document.querySelector(':focus')) return; this seems to be an elegant solution. I will try it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants