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

[Peek]Fix foreground window setting #26473

Conversation

jaimecbernardo
Copy link
Collaborator

Summary of the Pull Request

After #26364, the way to bring the Peek window to the foreground doesn't seem to be working correctly. It sometimes seems to bring keyboard input in but not mouse input, making the way that we try to detect loss of focus not work when the option to "Automatically close the Peek window after it loses focus" not work.

Changed the way we bring the Peek Window to the foreground mimic what we do on other PowerToys, which is to first send some mouse simulated mouse input and then bring the window to the foreground. This seems to be more stable.

PR Checklist

Validation Steps Performed

Tested switching files multiple times with both the "Automatically close the Peek window after it loses focus" on and off, both on the Desktop and on an Explorer window.

@jaimecbernardo jaimecbernardo added the Hot Fix Items we will product an out-of-band release for label May 30, 2023
Copy link
Contributor

@gokcekantarci gokcekantarci left a comment

Choose a reason for hiding this comment

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

I tested it and the code looks good

@davidegiacometti
Copy link
Collaborator

I was wondering if we can use single instance instead of this hack https://learn.microsoft.com/en-us/windows/apps/windows-app-sdk/applifecycle/applifecycle-instancing

@jaimecbernardo
Copy link
Collaborator Author

I was wondering if we can use single instance instead of this hack https://learn.microsoft.com/en-us/windows/apps/windows-app-sdk/applifecycle/applifecycle-instancing

What do you mean? Wouldn't that mean cold booting every time?

@jaimecbernardo jaimecbernardo merged commit 88b1203 into microsoft:main May 30, 2023
@davidegiacometti
Copy link
Collaborator

davidegiacometti commented May 30, 2023

I was wondering if we can use single instance instead of this hack https://learn.microsoft.com/en-us/windows/apps/windows-app-sdk/applifecycle/applifecycle-instancing

What do you mean? Wouldn't that mean cold booting every time?

The idea was to make it single-instance using WinAppSDK lifecycle APIs. When a second process is launched redirect the activation to the running instance. I have done a quick test with Hosts File Editor (we are already using this to make it single-instance) but I am not able to bring up the running window when a second process is launched. Never mind, the PR solution looks fine!

BLM16 pushed a commit to BLM16/PowerToys that referenced this pull request Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hot Fix Items we will product an out-of-band release for
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants