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

bugfix(android): Allow Volume* keys to be passed to the user #3636

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

MarijnS95
Copy link
Member

@MarijnS95 MarijnS95 commented Apr 18, 2024

It seems that discussions around #2748 (cc @Hoodad) being a poor workaround that should rather be implemented properly/differently drowned out the fact that the workaround was implemented incorrectly to begin with. By having the if inside the match arm, rather than /on/ the match arm the keycode => path is /never/ used for Volume* keys and this event is thus never passed on to the user, defeating the purpose of calling handle_volume_keys(). Only the Handled/Unhandled state is affected by the ignore_volume_keys workaround, as set by EventLoopBuilderExtAndroid::handle_volume_keys().

Fix this by moving the if workaround on the match arm, so that disabling it via EventLoopBuilderExtAndroid::handle_volume_keys() causes Volume* events to be delivered to user code once again.

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Missing a changelog entry.

@Hoodad
Copy link
Contributor

Hoodad commented Apr 18, 2024

Took me a while to see, but yes this should allow the users to react to input 👍 Thanks for the fix!

@daxpedda daxpedda added this to the Version 0.30.0 milestone Apr 19, 2024
@MarijnS95 MarijnS95 requested a review from kchibisov April 22, 2024 12:57
@MarijnS95 MarijnS95 force-pushed the android-fix-volume-keys branch from eb6ddb7 to 789063f Compare April 22, 2024 12:57
It seems that discussions around rust-windowing#2748 being a poor workaround that
should rather be implemented properly/differently drowned out the fact
that the workaround was implemented incorrectly to begin with.  By
having the `if` inside the match arm, rather than /on/ the match arm
the `keycode =>` path is /never/ used for `Volume*` keys and this
event is thus never passed on to the user, defeating the purpose
of calling `handle_volume_keys()`.  Only the `Handled`/`Unhandled`
state is affected by the `ignore_volume_keys` workaround, as set by
`EventLoopBuilderExtAndroid::handle_volume_keys()`.

Fix this by moving the `if` workaround on the match arm, so that
disabling it via `EventLoopBuilderExtAndroid::handle_volume_keys()`
causes `Volume*` events to be delivered to user code once again.
@MarijnS95 MarijnS95 force-pushed the android-fix-volume-keys branch from 15bea12 to 28034a4 Compare April 23, 2024 17:21
@kchibisov kchibisov merged commit 7006c7c into rust-windowing:master Apr 23, 2024
52 checks passed
@MarijnS95 MarijnS95 deleted the android-fix-volume-keys branch April 23, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants