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

Notify widgets of window focus change #2016

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ForLoveOfCats
Copy link
Collaborator

Not entirely sure if this is exactly the best way to go about this. An alternative could be to track window focus in WindowHandle but adding this as a new Event was by far and away the simplest (this patch is much smaller than I expected) and with the other window events there is existing precedence for approaching in this way.

The use case for this is to be able to perform actions, like saving content, on window focus lost, which is exactly what I needed this for

@ForLoveOfCats ForLoveOfCats added enhancement adds or requests a new feature S-needs-review waits for review labels Oct 21, 2021
@ForLoveOfCats
Copy link
Collaborator Author

Having now integrated this patch into my application I ran into an small sticking point of how widget focus change is a lifecycle event (and the fact that lifecycle event handling cannot mutate application outside submitting commands). As such it seems that there is probably more of a discussion to be had before deciding if notifying widgets of window focus change via an event is the right approach to expose window focus changes. If it is then there is the disconnect between it and widget focus handling which would be worth looking at as well. For the time being my application can continue to use this branch as I don't want to merge this as-is without some meaningful discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement adds or requests a new feature S-needs-review waits for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant