Fix combo box lists do not expand in the Preference dialog on MacOS#181
Fix combo box lists do not expand in the Preference dialog on MacOS#181Gasparoken wants to merge 1 commit into
Conversation
Gasparoken
force-pushed
the
fix-modal-macos-2
branch
2 times, most recently
from
c46867c to
ffd31dc
Compare
martincapello
left a comment
martincapello
left a comment
There was a problem hiding this comment.
Works great and looks good to me!
Gasparoken
force-pushed
the
fix-modal-macos-2
branch
from
ffd31dc to
39df0e3
Compare
aseprite-bot
left a comment
aseprite-bot
left a comment
There was a problem hiding this comment.
clang-tidy made some suggestions
| m_impl = nullptr; | ||
| } | ||
|
|
||
| - (void)windowDidBecomeKey:(NSNotification*)notification; |
There was a problem hiding this comment.
warning: expected ')' [clang-diagnostic-error]
- (void)windowDidBecomeKey:(NSNotification*)notification;
^Additional context
os/osx/window_delegate.h:31: to match this '('
- (void)windowDidBecomeKey:(NSNotification*)notification;
^| m_impl = nullptr; | ||
| } | ||
|
|
||
| - (void)windowDidBecomeKey:(NSNotification*)notification; |
There was a problem hiding this comment.
warning: expected external declaration [clang-diagnostic-error]
- (void)windowDidBecomeKey:(NSNotification*)notification;
^| m_impl = nullptr; | ||
| } | ||
|
|
||
| - (void)windowDidBecomeKey:(NSNotification*)notification; |
There was a problem hiding this comment.
warning: expected unqualified-id [clang-diagnostic-error]
- (void)windowDidBecomeKey:(NSNotification*)notification;
^| } | ||
|
|
||
| - (void)windowDidBecomeKey:(NSNotification*)notification; | ||
| { |
There was a problem hiding this comment.
warning: expected unqualified-id [clang-diagnostic-error]
{
^
|
I only corrected one comment after the revision. |
| [[NSNotificationCenter defaultCenter] addObserver:self | ||
| selector:@selector(appDidBecomeActive:) | ||
| name:NSApplicationDidBecomeActiveNotification | ||
| object:nil]; |
There was a problem hiding this comment.
Instead of using the NSNotificationCenter, probably we could use the applicationDidBecomeActive from our own AppDelegate to avoid adding a new observer for each window. And then we can re-order the windows there (or call a WindowOSX function to do the appDidBecomeActive job).
(Our applicationDidBecomeActive is already doing something for floating windows)
There was a problem hiding this comment.
Seeing our current applicationWillResignActive:
- (void)applicationWillResignActive:(NSNotification*)notification
{
for (id window : [NSApp windows]) {
if ([window isKindOfClass:[WindowOSXObjc class]] && [window isFloating]) {
[window setLevel:NSNormalWindowLevel];
[window orderWindow:NSWindowAbove relativeTo:0];
}
}
NSEvent* event = [NSApp currentEvent];
if (event != nil)
[ViewOSX updateKeyFlags:event];
}Wouldn't be possible to reorder children just starting from [NSApp windows]? (without a our auxiliary m_savedChildOrder array?)
There was a problem hiding this comment.
Instead of using the NSNotificationCenter, probably we could use the applicationDidBecomeActive from our own AppDelegate to avoid adding a new observer for each window. And then we can re-order the windows there (or call a WindowOSX function to do the appDidBecomeActive job).
Yes, you're right! I'll explore that option.
Wouldn't be possible to reorder children just starting from [NSApp windows]? (without a our auxiliary m_savedChildOrder array?)
No, '[NSApp windows]' this array do not guarantee the correct windows order (docs). On the other hand '[NSApp orderedWindows]' is an array of window objects arranged according to their front-to-back ordering on the screen, but even with orderedWindows, we need to remember the array when Aseprite loses focus, since I empirically verified that the z-order of child windows is lost. This makes me think it might be a Cocoa/Appkit bug, but I'm not sure.
| if (spec->modal()) | ||
| self.level = NSModalPanelWindowLevel; | ||
|
|
||
| m_savedChildOrder = [NSMutableArray array]; |
There was a problem hiding this comment.
We could keep m_savedChildOrder = nullptr until the first child window is added.
dacap
added
the
release blocker
Gasparoken
force-pushed
the
fix-modal-macos-2
branch
from
39df0e3 to
d4a902c
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
Gasparoken
force-pushed
the
fix-modal-macos-2
branch
from
d4a902c to
1caf621
Compare
Before this fix, on MacOS, combo box lists expanded behind the Preferences dialog when the 'UI with multiple windows' option was enabled. Now, if a window has a new child window (like a combo box list), it must be brought above its parent's z-order so it always appears on top, even when the parent is clicked/activated. This fixes a regression inserted in the commit aseprite/aseprite@d9785e5: 'Fix inactive window passes events to window below (aseprite/aseprite#4561, aseprite/aseprite#4835)'
|
clang-tidy review says "All clean, LGTM! 👍" |
Gasparoken
force-pushed
the
fix-modal-macos-2
branch
from
1caf621 to
7f70199
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
4 participants
This fixes a regression inserted in the commit aseprite/aseprite@d9785e5: PR: aseprite/aseprite#5813