-
Notifications
You must be signed in to change notification settings - Fork 256
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
Mobile web virtual keyboard support #279
Conversation
eframe was doing weird things with canvas resizing that didn't seem strictly necessary but I also didn't have a lot of different kinds of test devices. It seems to work as is and I'm not sure when I'll have the effort to really clean it up (the egui textEdit interaction and highlighting on mobile are still not very good as a base too). This also followed the weird eframe approach where a double tap is needed to open up the keyboard. |
@mvlabat any chance of a review? |
@v-kat hi, thank you for the PR! And apologies for neglecting it for so long. I've just tested it on my Android: for some reason, the keyboard disappears immediately after opening it. It seems like there's another click/touch event that follows immediately after that causes this behaviour. |
src/text_agent.rs
Outdated
} | ||
}) as Box<dyn FnMut(_)>); | ||
document.add_event_listener_with_callback("keydown", closure.as_ref().unchecked_ref())?; | ||
closure.forget(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of forgetting the closure, can we register it in the SubscribedEvents
resource? (Feel free to move it to lib.rs
as it resides in the web_clipboard
module atm.)
This way users will be able to unsubscribe manually in case they'll want to have some sort of multi-app setup on their page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 5985a6a I did multiple different SubscribedEvents
due to some annoyances with types but could switch things.
Are you seeing the same behavior on https://vpetmon.com I think there's maybe a conflict with the copy paste feature and duplicate events but need to double check |
There seems to be some bugs after I merged in 0.14 I'll investigate and try to update later. |
I tested it and seemed to be working on 0.14 still looking at using |
Maybe emilk/egui#4500 will eventually fix some of the issues. Need to fix the CI. |
https://blog.rust-lang.org/2024/07/25/Rust-1.80.0.html has LazyLock but I didn't want to try it. The TextEdit cursor focus issues on egui are pretty annoying and I might look at emilk/egui#4500 later to make this way less janky. |
I've just tested it on both Android and iOS, and I'm seeing the same issue: the keyboard closes immediately after opening it |
Can you tell me how you tested it and if it's the same on https://vpetmon.com I have at least 2 android users and an iOS user that are using it currently although the TextEdit focus is still buggy. Getting focus correctly still usually requires multiple clicks and you have to see the cursor to be able to use the keyboard emilk/egui#4855 might address it and I'll bring in anything else they do. I still have no clue what your actual issue is or how to reproduce it so I can't do anything going forward minus updating this PR to fix focus issues as egui fixes them. |
I test your PR with running the The focus behaviour is indeed consistent with what you're saying, it does require 2 taps to bring the keyboard up. Yet in the I just tested https://vpetmon.com/ on my Android (Google Chrome browser): the keyboard is indeed brought up, it doesn't disappear immediately like it does in the |
Apparently The other issue you're describing is the bad focus behavior with egui TextEdits that's getting fixed/examined upstream (I need to fix it so the keyboard only appears after focus as it's too hard to focus correctly currently). If you click until you see a cursor or flashing bar in the text field the keyboard should respond. I'm not sure if having this working without I'm still waiting for upstream changes to improve the focus behavior. I don't see much else I can do otherwise for the focus. 6eaa8e0 Maybe I could directly proxy the keyboard events in a handler like eframe is doing too if the the default event handling is off. I rather focus on other things for now.... I'll look at https://github.com/emilk/egui/blob/master/crates/eframe/src/web/events.rs#L139 again eventually and/or exposing more of this behavior generically from egui. |
Yeah, I can confirm that setting the flag back to true works around the focus issue. Unfortunately, I still can't get any text entered in the input field. It works the same way on vpetmon for me (I can't get any text entered there either). Screen_Recording_20240804_004445_Chrome.mp4 |
There might be code in emilk/egui#4848 that could maybe help and exposed more generically. (I haven't tried the pointer and focus changes) I'll check on the 0.28 release and see what else changed I think I need to steal some of the other updates as the main site demo seems better but I'm not sure if it's running off head or not. emilk/egui#4569 has been partly fixed since before this PR started and I'll see if there's anything else I can grab but the issue you showed is still seemingly possible but less likely. |
Testing using |
Good work on this, this branch works for the most part. However, when I click into the text box, the page zoom and scroll is reset, which means that I cannot double click on it to start typing. How could I help to fix this? |
Are you using the egui branch I mentioned and Android and the same example testbed? |
I am not using the same testbed, as I am using it in my own game. On modifying the egui branch, do you mean putting it in the bevy_egui Cargo.toml? That made it take one click instead of 2, but it still repositions the website on every key input. |
I haven't seen any zoom or canvas behavior like you're describing on Android with the UI example. I'm mostly finished with this so please close it if it's a big issue. I'm waiting for egui upstream changes and not actively working on this. |
If you give me more debug info I can try to help. I'm using it in prod on https://vpetmon.com with the egui branch I referenced and it seems okay with: DefaultPlugins
.set(WindowPlugin {
primary_window: Some(Window {
title: "Games VPetmon".to_string(),
fit_canvas_to_parent: true,
ime_enabled: true,
// prevent_default_event_handling: false, // to enable copy paste
// mode: WindowMode::BorderlessFullscreen,
..default()
}),
..default()
})
.set(AssetPlugin {
meta_check: AssetMetaCheck::Never,
..default()
}), [patch.crates-io]
egui = { git = "https://github.com/micmonay/egui", branch = "fix-keyboard" } |
The main change I have is that the canvas does not take up the entire screen, while the given example does. An example is at 139.144.52.74:12345, it may be a while until I can update it though. I will update this comment when it is live with the updated version. It appears that the current branch of bevy_egui has a bug. I am not sure where to put that patch so I will not be able to update the example given. |
da67a52
to
89a608a
Compare
@mvlabat the tests looked like they are failing with a build error. I'm still ideally waiting for a PR to be merged emilk/egui#4855 so that this works better but think it works decently. I didn't see a good way to fix 2 touches being needed to focus on an input on iOS. Even when trying to thread through the context it would need to be accessed repeatedly in the I'd like to see this merged soon and dunno how long the egui PR and version update will take to happen. Hopefully it's good enough. |
Yeah I was even thinking about merging and releasing it yesterday, considering that it'll likely work correctly once the linked PR is merged into Egui itself. But I decided to wait for a new Egui version anyway, just to be able test it with the final code that lands into Egui and be more confident that it works as we want. And thank you for keeping this updated! I do hope we merge it before the next release of beyv_egui :) |
@mvlabat any chance you could test this with the new egui version before merging #313 |
Already doing that :) I'll definitely merge it before the release, might push some changes to the branch as well |
Co-authored-by: v-kat <[email protected]>
Co-authored-by: Ivy <[email protected]>
Co-authored-by: v-kat <[email protected]>
I just merged to main. Thank you again! I had to finish it in a separate branch and then, when I squashed it, I realised it only attributed me as an author. I tried to trick github into linking the commit to your profile as well but it wouldn't work because of the fake email (that's why you see all these force-pushes to |
Co-authored-by: v-kat <[email protected]>
Ok, I just found the way lol. Butchering the repo history was totally worth it 😄 |
Fixes #29
Mostly stolen code from https://github.com/emilk/egui/blob/master/crates/eframe/src/web/text_agent.rs and https://github.com/oscrim/bevy_egui/tree/gui-touch-event still has the same issue as emilk/egui#4403 but seems to be mostly working. I had to use
once_cell
because iOS safari doesn't let you set an input focus unless the code is directly executed in the input callback. I also didn't proxy all of the key press events and am only doing backspace due to another iOS safari issue but could change that later.... Seems to work about as well as the egui mobile web virtual keyboard in eframe but there might be some issues.