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

Mobile web virtual keyboard support #279

Merged
merged 0 commits into from
Oct 4, 2024

Conversation

v-kat
Copy link
Contributor

@v-kat v-kat commented Apr 26, 2024

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.

@v-kat v-kat marked this pull request as ready for review April 26, 2024 23:56
@v-kat
Copy link
Contributor Author

v-kat commented Apr 29, 2024

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.

@v-kat
Copy link
Contributor Author

v-kat commented Jul 6, 2024

@mvlabat any chance of a review?

@mvlabat
Copy link
Owner

mvlabat commented Jul 7, 2024

@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.

}
}) as Box<dyn FnMut(_)>);
document.add_event_listener_with_callback("keydown", closure.as_ref().unchecked_ref())?;
closure.forget();
Copy link
Owner

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.

Copy link
Contributor Author

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.

src/text_agent.rs Outdated Show resolved Hide resolved
@v-kat
Copy link
Contributor Author

v-kat commented Jul 7, 2024

@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.

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

@v-kat
Copy link
Contributor Author

v-kat commented Jul 8, 2024

There seems to be some bugs after I merged in 0.14 I'll investigate and try to update later.

@v-kat
Copy link
Contributor Author

v-kat commented Jul 14, 2024

I tested it and seemed to be working on 0.14 still looking at using SubscribedEvents with some minor type annoyances with the EventClosure closure. There's some jank with the TextEdit cursor not always easily appearing on touch and then the keyboard input not doing anything until the input actually has focus with the cursor.

@v-kat
Copy link
Contributor Author

v-kat commented Jul 15, 2024

Maybe emilk/egui#4500 will eventually fix some of the issues. Need to fix the CI.

@v-kat
Copy link
Contributor Author

v-kat commented Jul 25, 2024

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.

@mvlabat
Copy link
Owner

mvlabat commented Jul 30, 2024

I've just tested it on both Android and iOS, and I'm seeing the same issue: the keyboard closes immediately after opening it

@v-kat
Copy link
Contributor Author

v-kat commented Jul 30, 2024

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.

@mvlabat
Copy link
Owner

mvlabat commented Jul 31, 2024

I test your PR with running the ui example in web: cargo run-wasm --example ui --host 0.0.0.0.

The focus behaviour is indeed consistent with what you're saying, it does require 2 taps to bring the keyboard up. Yet in the ui example the keyboard appears and is closed immediately on the second tap.

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 ui example, but no text appears in the input when I press keyboard buttons.

@v-kat
Copy link
Contributor Author

v-kat commented Aug 1, 2024

Apparently prevent_default_event_handling: false, set on the WindowPlugin causes the keyboard not to work at least in your example.

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 prevent_default_event_handling is possible but I can look more.

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.

@mvlabat
Copy link
Owner

mvlabat commented Aug 3, 2024

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

@v-kat
Copy link
Contributor Author

v-kat commented Aug 3, 2024

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. (Below is just for reference)

It works the same way on vpetmon for me (I can't get any text entered there either).

Screen_Recording_20240804_004445_Chrome.mp4

You have to click multiple times until there's a blinking cursor indicating the TextEdit can receive input
Screenshot_2024-08-03-14-59-53-82_3aea4af51f236e4932235fdada7d1643
I need to tweak the logic a tiny bit to not do spurious opens but getting always correct focus is an open egui issue.

@v-kat
Copy link
Contributor Author

v-kat commented Aug 4, 2024

lame_example
It's not very good and kind of hard to do successfully given the small sizes of TextEdits but does work.... emilk/egui#4855 will hopefully help and I'll also steal any other changes. LMK if you want a different prevent_default_event_handling fix. I kind of want to put this down and/or just get it merged despite the jank but am fine if you want to wait or something else.

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.

@v-kat
Copy link
Contributor Author

v-kat commented Aug 9, 2024

Testing using egui = { git = "https://github.com/micmonay/egui", default-features = false, branch = "fix-keyboard", features = ["bytemuck"] } for better TextEdit focus handling. Might maybe steal the PointerEvent change in emilk/egui#4848 but for now android always works on 1 touch for the input (as long as using emilk/egui#4855) and safari requires a double click due to ordering with the process_input_system firing after the iOS native safari event that's only needed due to browser quirks (I couldn't think of a better way to fix this minus maybe passing in context_params.contexts). Maybe still some other issues but seems pretty good when using the crates/egui/src/widgets/text_edit/builder.rs change in 4855.

@opfromthestart
Copy link

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?

@v-kat
Copy link
Contributor Author

v-kat commented Aug 9, 2024

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?

@opfromthestart
Copy link

opfromthestart commented Aug 9, 2024

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.
Also, when the canvas is maximized/made fullscreen, the keyboard no longer appears when clicking on a text input.
I cannot test if these are due to egui as the egui demo is in full screen and repositions elements to be on-screen when the keyboard appears.

@v-kat
Copy link
Contributor Author

v-kat commented Aug 9, 2024

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.

@v-kat
Copy link
Contributor Author

v-kat commented Aug 12, 2024

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. Also, when the canvas is maximized/made fullscreen, the keyboard no longer appears when clicking on a text input. I cannot test if these are due to egui as the egui demo is in full screen and repositions elements to be on-screen when the keyboard appears.

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" }

@opfromthestart
Copy link

opfromthestart commented Aug 17, 2024

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.

@v-kat
Copy link
Contributor Author

v-kat commented Aug 19, 2024

@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 touchend handler due to iOS security on setting focus which brought up 'static lifetime issues.

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.

@mvlabat
Copy link
Owner

mvlabat commented Aug 19, 2024

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 :)

@v-kat
Copy link
Contributor Author

v-kat commented Oct 3, 2024

@mvlabat any chance you could test this with the new egui version before merging #313

@mvlabat
Copy link
Owner

mvlabat commented Oct 4, 2024

Already doing that :) I'll definitely merge it before the release, might push some changes to the branch as well

mvlabat added a commit that referenced this pull request Oct 4, 2024
Co-authored-by: v-kat <fake@email.com>
@mvlabat mvlabat changed the base branch from main to test_branch October 4, 2024 15:04
@mvlabat mvlabat merged commit 1b0fc8a into mvlabat:test_branch Oct 4, 2024
39 checks passed
mvlabat added a commit that referenced this pull request Oct 4, 2024
Co-authored-by: Ivy <fake@email.com>
mvlabat added a commit that referenced this pull request Oct 4, 2024
Co-authored-by: v-kat <fake@email.com>
@mvlabat
Copy link
Owner

mvlabat commented Oct 4, 2024

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 main :D)

mvlabat added a commit that referenced this pull request Oct 4, 2024
Co-authored-by: v-kat <1297926+v-kat@users.noreply.github.com>
@mvlabat
Copy link
Owner

mvlabat commented Oct 4, 2024

Ok, I just found the way lol. Butchering the repo history was totally worth it 😄

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.

Text input doesn't bring up virtual keyboard on mobile
3 participants