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

Support Back button/KeyCode on Android #2304

Open
MarijnS95 opened this issue May 26, 2022 · 11 comments
Open

Support Back button/KeyCode on Android #2304

MarijnS95 opened this issue May 26, 2022 · 11 comments
Labels
C - needs discussion Direction must be ironed out DS - android

Comments

@MarijnS95
Copy link
Member

MarijnS95 commented May 26, 2022

Started playing about with VirtualKeyCode on Android, and it seems there's no mapping for KeyCode::Back which would be needed to handle a graceful app exit (or going to the previous page) when the user presses it. The implementation for VirtualKeyCode::Back reads:

winit/src/event.rs

Lines 962 to 964 in f04fa5d

/// The Backspace key, right over Enter.
// TODO: rename
Back,

So I don't think we should add a new Back VirtualKeyCode and rename existing uses to Backspace, as that's really hard to notice for end-users upgrading their winit. Perhaps we can piggyback NavigateBackward but android already supports that as KeyCode::NavigatePrevious:

Keycode::Back => Some(VirtualKeyCode::NavigateBackward),

How do you think we should go forward?

Originally posted by @MarijnS95 in #2226 (comment)

@kchibisov
Copy link
Member

@MarijnS95 is this still an issue?

@MarijnS95
Copy link
Member Author

@kchibisov I don't see Keycode::Back being handled anywhere in Android's platform_impl, so I guess not?

@MarijnS95
Copy link
Member Author

MarijnS95 commented Jul 31, 2023

@kchibisov note that while searching for VirtualKeyCode I could still find some references in src/platform_impl/linux/x11/events.rs even though it was renamed to Key in #2662. This module doesn't seem to be referenced anywhere, was the file not deleted while X11 was refactored?

EDIT: Turns out that refactor happened in #2662 :)

@kchibisov
Copy link
Member

I guess everything is fine, right?

@MarijnS95
Copy link
Member Author

@kchibisov it's not exactly fine as we cannot handle that keycode yet, it's not mapped.

What's worse, we don't have a way to optionally mark the keycode as used at runtime (e.g. given how inconsistently we're forced to handle volume keys now since #2748) as users would likely want to back out of some menu stack. Once they are at the bottom of the stack, they will set handled=false on Keycode::Back (probably via a KeyEvent extension trait) so that Android handles it instead and leaves or stops the app.

@kchibisov
Copy link
Member

But you can define a filter mask on event loop builder whether we want to accept or not? Like HashSet<Key>. Could use something else, but setting a filter mask for events user want to handle or not sounds 'sane'.

@MarijnS95
Copy link
Member Author

Was that built already? Your comment sounded like a suggestion but I'll check it out. Most important question: can we change the contents of the HashSet at runtime after building the event loop? As explained above that is vital to supporting these things.

@kchibisov
Copy link
Member

Well, it's not here, but nothing stops you from defining such a thing. You can change the event delivery at runtime, for example we have similar concept with listen_device_events, you can change it at runtime just fine.

@MarijnS95
Copy link
Member Author

Sure, I'll think about it but doubt I can find the time.

Am I right in assuming that you're suggesting this route because going the extension way on KeyEvent might be very hard to synchronize back to the platform implementation?

@kchibisov
Copy link
Member

Right, and because events are just data which could be send between threads.

@MarijnS95
Copy link
Member Author

MarijnS95 commented Jul 31, 2023

Exactly, we have to call finish_event(evt, handled: bool) at some point, and have to know the value of handled. That is impossible if the user holds on to a KeyEvent for who knows how long.

EDIT: I still haven't decided whether we should give the user the event if it is in the "don't mark this as handled" HashSet<Key>. Perhaps there are cases to be made where a user first inspects the event before marking it as handled, the API is designed like this for a reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out DS - android
Development

No branches or pull requests

2 participants