Skip to content

core: Use player notifications for virtual keyboard #19889

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

Closed

Conversation

kjarosh
Copy link
Member

@kjarosh kjarosh commented Mar 22, 2025

This refactor replaces open_virtual_keyboard/close_virtual_keyboard methods from UiBackend with OpenVirtualKeyboard/CloseVirtualKeybard player notifications.

It's a follow up to #19666 and a PoC of how notifications can be used for other purposes too.

This refactor replaces open_virtual_keyboard/close_virtual_keyboard
methods from UiBackend with OpenVirtualKeyboard/CloseVirtualKeybard
player notifications.
@kjarosh kjarosh added A-core Area: Core player, where no other category fits T-refactor Type: Refactor / Cleanup labels Mar 22, 2025
@kjarosh kjarosh requested a review from evilpie March 22, 2025 22:38
@kjarosh
Copy link
Member Author

kjarosh commented Mar 22, 2025

@torokati44 is this refactor okay for Android?

@kjarosh kjarosh requested a review from torokati44 March 22, 2025 22:38
@@ -208,15 +209,17 @@ impl<'gc> FocusTracker<'gc> {
}

fn update_virtual_keyboard(&self, context: &mut UpdateContext<'gc>) {
if let Some(text_field) = self.get_as_edit_text() {
let notification = if let Some(text_field) = self.get_as_edit_text() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let notification = if let Some(text_field) = self.get_as_edit_text() {
let notification = self.get_as_edit_text().is_some_and(|tf| tf.is_editable())

@n0samu
Copy link
Member

n0samu commented Mar 23, 2025

Has anyone tested this on iOS? I'd be happy to do so if someone puts up a demo build

@kjarosh
Copy link
Member Author

kjarosh commented Mar 23, 2025

Has anyone tested this on iOS? I'd be happy to do so if someone puts up a demo build

Oh dang, I forgot about iOS. It won't work there because notifications are asynchronous and it requires focus to be set synchronously.

@kjarosh kjarosh closed this Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core player, where no other category fits T-refactor Type: Refactor / Cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants