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

Add modal window treatment to macOS #116

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

Gasparoken
Copy link
Member

This fix is the first step to solve issue aseprite/aseprite#4561

@Gasparoken Gasparoken requested a review from dacap December 3, 2024 14:01
@Gasparoken Gasparoken self-assigned this Dec 3, 2024
@aseprite-bot
Copy link
Collaborator

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

os/osx/window.mm Outdated
Comment on lines 120 to 123
if (spec->floating()) {
self.level = NSFloatingWindowLevel;
self.hidesOnDeactivate = true;
if (spec->modal())
[self setLevel:NSModalPanelWindowLevel];
}
Copy link
Member

Choose a reason for hiding this comment

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

We're already setting the level property if it's floating, probably we can have a modal / not floating (this is not the case of Aseprite which sets the "floating" property for all non-main windows):

Suggested change
if (spec->floating()) {
self.level = NSFloatingWindowLevel;
self.hidesOnDeactivate = true;
if (spec->modal())
[self setLevel:NSModalPanelWindowLevel];
}
if (spec->floating()) {
self.level = NSFloatingWindowLevel;
self.hidesOnDeactivate = true;
}
if (spec->modal())
self.level = NSModalPanelWindowLevel;

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

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

os/osx/window.mm Outdated
@@ -122,6 +122,9 @@ - (WindowOSXObjc*)initWithImpl:(os::WindowOSX*)impl
self.hidesOnDeactivate = true;
}

if (spec->modal())
[self setLevel:NSModalPanelWindowLevel];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[self setLevel:NSModalPanelWindowLevel];
self.level = NSModalPanelWindowLevel;

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry!

Before this addition, when a modal window was active,
it was allowed to bring other windows to the front (which
should be prohibited).
This addition is part of the solution for the issue
aseprite/aseprite#4561 'Inactive windows in multiple-window UI mode
pass events to below windows'
@aseprite-bot
Copy link
Collaborator

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

@Gasparoken Gasparoken removed their assignment Dec 5, 2024
@dacap
Copy link
Member

dacap commented Dec 9, 2024

I'll merge this, it would be nice to block the whole hierarchy of parent's floating windows when a modal is displayed, but at the moment this works better than the current main to fix the bug.

@dacap dacap merged commit 4a3a28c into aseprite:main Dec 9, 2024
8 checks passed
@Gasparoken
Copy link
Member Author

I'll merge this, it would be nice to block the whole hierarchy of parent's floating windows when a modal is displayed, but at the moment this works better than the current main to fix the bug.

Great! Remember to approve aseprite/aseprite#4835 so that issue aseprite/aseprite#4561 is resolved in Aseprite.

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