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

Fix floating window disappears when another app gains focus #113

Closed
wants to merge 1 commit into from

Conversation

Gasparoken
Copy link
Member

This is the solution for issue aseprite/aseprite#4265

@aseprite-bot
Copy link
Collaborator

clang-tidy review says "All clean, LGTM! 👍"

@dacap
Copy link
Member

dacap commented Dec 4, 2024

I've just rebased the commit and fix conflicts in https://github.com/dacap/laf/tree/fix-float-window (changed minor things): a1498f0

Anyway testing the floating_windows example (the same happens on aseprite) I noticed a little flickering and sometimes the floating windows goes back to the main window:

Screen.Recording.2024-12-04.at.8.51.04.AM.mov

@dacap dacap assigned Gasparoken and unassigned dacap Dec 4, 2024
@aseprite-bot
Copy link
Collaborator

Hi there!

One or more of the commit messages in this PR do not match our code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

@aseprite-bot
Copy link
Collaborator

clang-tidy review says "All clean, LGTM! 👍"

@Gasparoken Gasparoken assigned dacap and unassigned Gasparoken Dec 5, 2024
@dacap
Copy link
Member

dacap commented Dec 9, 2024

The floating window is going to the back always:

Screen.Recording.2024-12-09.at.12.45.52.PM.mov

@dacap dacap assigned Gasparoken and unassigned dacap Dec 9, 2024
@Gasparoken
Copy link
Member Author

Referred to #113 (comment)

It turns out that the behavior we are looking for:

  • Floating windows that do not hide when our application loses focus.
  • But that remain under other active applications.
    This is not standard behavior on macOS. Typically, apps on macOS have floating windows that hide when you lose focus.

The flickering observed in a1498f0 would seem to be unavoidable with the tools available in Cocoa.

It was verified that another program with floating windows with the desired behavior has the same problem of flickering when the application gains focus.

Therefore, I think that the a1498f0 option would be adequate until the Cocoa maintainers add a more adequate way to handle floating windows.

I was not able to reproduce what happens in the video with a1498f0, I never observed the floating window go to the background. I'll update/rebase this branch with the latest modifications in case there is any interference with new commit additions.

@Gasparoken
Copy link
Member Author

I updated aseprite/laf and aseprite/aseprite, but I still can't get the main window to cover the floating window (my macOS version is: Ventura 13.0.1).

Screen.Recording.2024-12-13.at.11.23.23.mov

@dacap Did you use a1498f0 to make the video at #113 (comment)? What's your macOS version?

@aseprite-bot
Copy link
Collaborator

Hi there!

One or more of the commit messages in this PR do not match our code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

@aseprite-bot
Copy link
Collaborator

clang-tidy review says "All clean, LGTM! 👍"

@aseprite-bot
Copy link
Collaborator

Hi there!

One or more of the commit messages in this PR do not match our code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

@aseprite-bot
Copy link
Collaborator

clang-tidy review says "All clean, LGTM! 👍"

@Gasparoken
Copy link
Member Author

@dacap Did you use a1498f0 to make the video at #113 (comment)? What's your macOS version?

I updated my macOS to Sequoia 15.2. I can now see the behavior you reported.

Copy link
Member

@martincapello martincapello left a comment

Choose a reason for hiding this comment

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

I've suggested a change to fix the PR. In the same comment I also explain a couple of things I went through when trying to find a different fix (because it wasn't working when running on Sonoma).

Oh, I couldn't remove the flickering when the app is activated either...

Comment on lines +96 to +98
for (NSWindow* floatingWindow in self.floatingWindows)
floatingWindow.level = NSNormalWindowLevel;

Copy link
Member

@martincapello martincapello Jan 2, 2025

Choose a reason for hiding this comment

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

I've been working on ticket aseprite/aseprite#4774 which basically is a duplicate of aseprite/aseprite#4265 so I took a look at this PR, tried it, and then tried to find a new approach (because it wasn't working on all cases) using the following method:

- (void) addChildWindow:(NSWindow*) childWin 
                ordered:(NSWindowOrderingMode) place;

This method almost does what we need for free...BUT, it has a catch...when you move the parent window, the child window is moved along it.
So, finally I made a new approach based on this one. That work made me realize that by applying the following suggestion makes this PR work:

Suggested change
for (NSWindow* floatingWindow in self.floatingWindows)
floatingWindow.level = NSNormalWindowLevel;
for (NSWindow* floatingWindow in self.floatingWindows) {
floatingWindow.level = NSNormalWindowLevel;
[floatingWindow orderWindow:NSWindowAbove relativeTo:0];
}

The new approach is on this other PR, which basically uses a different way to search for floating windows but does pretty much the same as this PR.

@Gasparoken
Copy link
Member Author

Thanks @martincapello! @dacap you can close this PR and apply Martin's PR.

@Gasparoken Gasparoken assigned dacap and unassigned Gasparoken Jan 3, 2025
@dacap dacap closed this Jan 3, 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.

4 participants