Skip to content

Add modal window treatment to macOS#116

Merged
dacap merged 1 commit into
aseprite:mainfrom
Gasparoken:fix-modal-window
Dec 9, 2024
Merged

Add modal window treatment to macOS#116
dacap merged 1 commit into
aseprite:mainfrom
Gasparoken:fix-modal-window

Conversation

@Gasparoken

Copy link
Copy Markdown
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
Copy Markdown
Collaborator

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

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

Copy link
Copy Markdown
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
Copy Markdown
Collaborator

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

Comment thread os/osx/window.mm Outdated
}

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

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

@Gasparoken Gasparoken removed their assignment Dec 5, 2024
@dacap

dacap commented Dec 9, 2024

Copy link
Copy Markdown
Member

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
@Gasparoken

Copy link
Copy Markdown
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