Skip to content

Conversation

dfaure
Copy link
Contributor

@dfaure dfaure commented Aug 16, 2025

Successfully tested with this code

forward-focus: my-key-handler;
my-key-handler := FocusScope {
    key-pressed(event) => {
        if (event.text == Key.Back) {
            root.close();
        }
        accept
    }
}

but also without (that was the hard part: letting Android terminate the activity on Back if it's not accepted by slint, which no longer happened after returning Some(...) in key_codes.rs)

@dfaure dfaure force-pushed the wip/dfaure/android_back_key branch 2 times, most recently from 2b212c6 to fcc484b Compare August 16, 2025 00:55
@dfaure dfaure force-pushed the wip/dfaure/android_back_key branch 3 times, most recently from 9b644ef to f26459f Compare August 16, 2025 12:20
Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.

@dfaure
Copy link
Contributor Author

dfaure commented Aug 16, 2025

widgets_combobox_qt seems to fail reliably in CI after these changes, surprisingly.

However I don't manage to run that test locally:

 SLINT_TEST_FILTER=combo cargo test -p test-driver-rust --features build-time

   Compiling test-driver-rust v1.13.0 (/d/slint/tests/driver/rust)
error[E0433]: failed to resolve: could not find `NativeComboBox` in `sp`
 --> /d/slint/target/debug/build/test-driver-rust-510c5e5728f3f3d2/out/widgets_combobox_qt.rs:1:22529
  |
1 | ...* & InnerComboBox_root_4 :: FIELD_OFFSETS . r#native_6 } + sp :: r#NativeComboBox :: FIELD_OFFSETS . r#current_value) . apply_pin (_self) , ({ InnerComboBox_root_4 :: FIELD_OFFSETS . r#b...
  |                                                                     ^^^^^^^^^^^^^^^^ could not find `NativeComboBox` in `sp`

(and more similar errors)
i-slint-backend-qt builds just fine.

@ogoffart
Copy link
Member

could not find NativeComboBox in sp

I have seen that before. I think it's a bug in the build script which somehow think Qt feature is active despite it is not (maybe because confused with other build)
Can add --features=slint/backend-qt to the cargo line

@tronical
Copy link
Member

It’s weird how another PR I made earlier today passed nicely, but this one somehow triggers the QComboBox test (not build) failure.

There are warnings about threading that are also worrisome (not caused by this patch).

Successfully tested with this code

    forward-focus: my-key-handler;
    my-key-handler := FocusScope {
        key-pressed(event) => {
            if (event.text == Key.Back) {
                root.close();
            }
            accept
        }
    }

but also without (that was the hard part: letting Android
terminate the activity on Back if it's not accepted by slint,
which no longer happened after returning Some(...) in key_codes.rs)
@ogoffart
Copy link
Member

Wierd indeed. Maybe related to the change of run_change_handlers, although i can't see a difference.

@tronical
Copy link
Member

Same here, I've been staring at the diff but can't see the difference. Somehow the defer! macro isn't producing the same behaviour.

@dfaure dfaure force-pushed the wip/dfaure/android_back_key branch from f26459f to ac737c2 Compare August 20, 2025 08:06
Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dfaure
Copy link
Contributor Author

dfaure commented Aug 21, 2025

what's the next step? (this isn't like Qt, I can't merge myself)
@tronical needs to approve too?

@ogoffart ogoffart merged commit e2053ee into slint-ui:master Aug 21, 2025
40 checks passed
@ogoffart
Copy link
Member

Sorry, I wanted to merge yesterday but forgot.

dfaure added a commit to dfaure/videofinder that referenced this pull request Aug 23, 2025
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.

3 participants