Skip to content

Expose FocusEventReason to .slint as an argument in focus-event-changed and add focus-gained and focus-lost callbacks #8569

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

codeshaunted
Copy link
Contributor

@codeshaunted codeshaunted commented May 28, 2025

This exposes FocusEventReason to .slint, and adds it as an argument to focus-event-changed callback on FocusScope to close #8387. It also adds two new callbacks, focus-gained and focus-lost, which are identical to focus-event-changed but are only invoked on focus gain or loss respectively.

In addition to this, it removes the FocusEventReason::AccessKit variant, replacing it with the mouse variant to hopefully make AccessKit more compatible with any Slint code that will use FocusEventReason.

Finally, I added two tests based on focus_change_event.slint, one for testing the FocusEventReason argument and another for testing the new callbacks.

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 for the patch. Looks great!

Since this adds public API, we'll have to bikeshed the naming a bit :-)
I added suggestion, but these are just suggestions, i'm not sure yet what's the best name for these.

@@ -282,6 +282,9 @@ pub fn duration_until_next_timer_update() -> Option<core::time::Duration> {
pub use crate::input::key_codes::Key;
pub use crate::input::PointerEventButton;

// reexport focus reason enum to the public api
pub use crate::input::FocusEventReason;
Copy link
Member

Choose a reason for hiding this comment

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

I think it doesn't need to be in the public API.

The test can keep using the private API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, still getting acquainted with the project structure, how would I access it via the private API in the test?

Copy link
Member

Choose a reason for hiding this comment

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

The test can use slint::private_unstable_api::re_exports::FocusEventReason

@@ -131,6 +131,20 @@ macro_rules! for_each_enums {
Action,
}

/// This enum describes the different reasons for a FocusEvent
enum FocusEventReason {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be named FocusReason? Or FocusChangeReason ?

/// This enum describes the different reasons for a FocusEvent
enum FocusEventReason {
/// The keyboard caused the event (tabbing)
Keyboard,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe KeyboardNavigation or KeyboardShortcut
In slint it would then be FocusReason.keyboard or FocusReason.keyboard-navigation or FocusReason.keyboard-shortcut

Or maybe TabKey ?

/// The keyboard caused the event (tabbing)
Keyboard,
/// The mouse caused the event
Mouse,
Copy link
Member

Choose a reason for hiding this comment

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

Or Clicked

Comment on lines 142 to 143
/// A built-in function caused the event (set-focus-item, clear-focus-item)
BuiltinFunction,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could be Programmatic

Comment on lines 144 to 145
/// The window manager changed the active window and caused the event
ActiveWindow,
Copy link
Member

Choose a reason for hiding this comment

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

WindowActivation?

/// The mouse caused the event
Mouse,
/// A popup caused the event
Popup,
Copy link
Member

Choose a reason for hiding this comment

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

PopupActivation?

@codeshaunted codeshaunted force-pushed the avery/focus-reason-callback branch from 70b44c6 to 432ab67 Compare May 30, 2025 13:15
@codeshaunted codeshaunted force-pushed the avery/focus-reason-callback branch from 432ab67 to 237b9f4 Compare May 30, 2025 13:20
@codeshaunted
Copy link
Contributor Author

I've renamed the the variants and the enum based on your suggestions, I also had a bit of a git mishap and had to force push everything again here. Sorry about that 😅

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.

FocusScope: Distinguish between keyboard focus and click focus
2 participants