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

Fix beep sound, closes #799 #801

Merged
merged 2 commits into from
Dec 10, 2022
Merged

Fix beep sound, closes #799 #801

merged 2 commits into from
Dec 10, 2022

Conversation

probablykasper
Copy link
Member

@probablykasper probablykasper commented Dec 9, 2022

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

All I did was have the WryWebViewParent call performKeyEquivalent on the menu. As far as I can tell, everything works:

  • The frontend receives Escape keydown events
  • There's no beep sound. Built-in menus like Copy can still beep, but that's correct behavior - Safari acts the same. You can prevent the menu (and by extension, the beep) from being called using e.preventDefault() in the frontend.
  • Q and Ctrl+Shift+Q menus work
  • Menus only fire once

Closes #799

@wusyong Is there anything I'm missing?

@probablykasper probablykasper marked this pull request as ready for review December 9, 2022 23:24
@probablykasper probablykasper requested a review from a team as a code owner December 9, 2022 23:24
Copy link
Member

@wusyong wusyong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following bugs we encountered bugs before have tested:

  • no beep sound
  • keyboard input working including IME
  • menu accelerator works with: CMD+, SHIFT+, and single keyinput in menu example.
  • keyinput only reigger once using sample in [bug] Keydown event trigger twice tauri#5741
  • works with release build with this config:
[profile.release]
strip = true 
lto = true
opt-level = "z"

@wusyong wusyong merged commit 94256c3 into dev Dec 10, 2022
@wusyong wusyong deleted the k-beepfix branch December 10, 2022 00:07
@github-actions github-actions bot mentioned this pull request Dec 10, 2022
@wusyong
Copy link
Member

wusyong commented Dec 10, 2022

@probablykasper Thanks again! This really fix everything around key events I believe.

@probablykasper
Copy link
Member Author

That's awesome! Pure luck that I tried that haha

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 this pull request may close these issues.

Best approch to disable key error sound
2 participants