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

Best approch to disable key error sound #799

Closed
wusyong opened this issue Dec 9, 2022 · 4 comments · Fixed by #801 or #802
Closed

Best approch to disable key error sound #799

wusyong opened this issue Dec 9, 2022 · 4 comments · Fixed by #801 or #802
Assignees
Labels

Comments

@wusyong
Copy link
Member

wusyong commented Dec 9, 2022

After #798, we remove all keyDown implementations. Because key events on macOS is too messy, it's not worth it to implement our own. This will bring back beep sound when pressing key.

The error sound will fire when the view doesn't handle it, eg. there's no menu accelerator for it, or users are not typing in the textarea. To remove it, we can offer a config to add keyDown back again with several workaround. But I think we should not handle any accelerator without Ctrl and Cmd this time. Because the key event chain is totally different without them, and I have struggled to satisfy every scenarios. See this slides for more information. Official Apple apps seem to follow this convention too. They don't have acceleartors without Ctrl and Cmd.

Do you think this is a good idea: add a option to disable error sound but menu accelerator won't work without Cmd and Ctrl. And should it be default?

@probablykasper
Copy link
Member

probablykasper commented Dec 9, 2022

Here are some uninformed suggestions that were probably already considered/tried:

A

  1. Make the webview be the first to receive keydown events using makeFirstResponder
  2. Afterwards, have a catchall keydown handler on NSView which runs performKeyEquivalent

B

  1. Make the webview be the first to receive keydown events using makeFirstResponder
  2. Have a catchall keydown handler in a subview

C

Something with interpretKeyEvents? not sure what it does

D

Put webview inside a subview inside the contentview, if that's any different from what was done before which removed resize/cursor events.

E

Override the webview's keydown handler itself

The missing events were the only functional issue with the subview approach, right? Did you figure out why exactly the events didn't work with that setup?

@wusyong
Copy link
Member Author

wusyong commented Dec 9, 2022

Yeah I have tried above but they all have something doesn't work. Like Cmd+* accelerator not working, or Shift+* not working.

The approach before #798 also have issues that accelerators without Cmd and Ctrl will fire twice. And I don't know why calling super.performKeyEquivalent can't trigger Esc key call. It has to be super.keyDown. And if we just call keyDown, the beep sound returns.

@probablykasper
Copy link
Member

Is Electron's usage of performKeyEquivalent helpful in any way?
https://github.com/search?q=repo%3Aelectron%2Felectron%20performKeyEquivalent&type=code

@probablykasper
Copy link
Member

probablykasper commented Dec 9, 2022

I think I might've actually managed to fix it 🤞

wusyong pushed a commit that referenced this issue Dec 10, 2022
* Beepfix attempt

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

Successfully merging a pull request may close this issue.

2 participants