From 442e84562953c3dcb31f8bbd9b18c213bdd8a038 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 29 Jul 2021 15:14:52 +0100 Subject: [PATCH] Merge pull request #6188 from AvaloniaUI/fixes/6007-child-window-activation OSX: Fix child window activation # Conflicts: # native/Avalonia.Native/src/OSX/window.mm # src/Avalonia.Controls/ApiCompatBaseline.txt # tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs --- native/Avalonia.Native/src/OSX/window.h | 1 + native/Avalonia.Native/src/OSX/window.mm | 76 ++++++------- src/Avalonia.Controls/ApiCompatBaseline.txt | 5 +- .../Platform/IWindowBaseImpl.cs | 4 +- src/Avalonia.Controls/Window.cs | 4 +- src/Avalonia.Controls/WindowBase.cs | 2 +- .../Remote/PreviewerWindowImpl.cs | 6 +- src/Avalonia.DesignerSupport/Remote/Stubs.cs | 2 +- src/Avalonia.Headless/HeadlessWindowImpl.cs | 7 +- src/Avalonia.Native/PopupImpl.cs | 4 +- src/Avalonia.Native/WindowImplBase.cs | 4 +- src/Avalonia.Native/avn.idl | 2 +- src/Avalonia.X11/X11Window.cs | 2 +- src/Windows/Avalonia.Win32/PopupImpl.cs | 2 +- src/Windows/Avalonia.Win32/Win32Platform.cs | 2 +- src/Windows/Avalonia.Win32/WindowImpl.cs | 4 +- .../ContextMenuTests.cs | 101 ++++++++++++++++-- .../WindowBaseTests.cs | 2 +- .../MockWindowingPlatform.cs | 2 +- 19 files changed, 159 insertions(+), 73 deletions(-) diff --git a/native/Avalonia.Native/src/OSX/window.h b/native/Avalonia.Native/src/OSX/window.h index b1f64bca88..e2f69c8359 100644 --- a/native/Avalonia.Native/src/OSX/window.h +++ b/native/Avalonia.Native/src/OSX/window.h @@ -34,6 +34,7 @@ class WindowBaseImpl; -(double) getScaling; -(double) getExtendedTitleBarHeight; -(void) setIsExtended:(bool)value; +-(bool) isDialog; @end struct INSWindowHolder diff --git a/native/Avalonia.Native/src/OSX/window.mm b/native/Avalonia.Native/src/OSX/window.mm index e76d8c4a7e..da9cffb12f 100644 --- a/native/Avalonia.Native/src/OSX/window.mm +++ b/native/Avalonia.Native/src/OSX/window.mm @@ -105,7 +105,7 @@ public: return Window; } - virtual HRESULT Show(bool activate) override + virtual HRESULT Show(bool activate, bool isDialog) override { @autoreleasepool { @@ -488,6 +488,11 @@ public: return S_OK; } + virtual bool IsDialog() + { + return false; + } + protected: virtual NSWindowStyleMask GetStyle() { @@ -518,6 +523,7 @@ private: NSRect _preZoomSize; bool _transitioningWindowState; bool _isClientAreaExtended; + bool _isDialog; AvnExtendClientAreaChromeHints _extendClientHints; FORWARD_IUNKNOWN() @@ -573,11 +579,12 @@ private: } } - virtual HRESULT Show (bool activate) override + virtual HRESULT Show (bool activate, bool isDialog) override { @autoreleasepool - { - WindowBaseImpl::Show(activate); + { + _isDialog = isDialog; + WindowBaseImpl::Show(activate, isDialog); HideOrShowTrafficLights(); @@ -1070,6 +1077,11 @@ private: } } + virtual bool IsDialog() override + { + return _isDialog; + } + protected: virtual NSWindowStyleMask GetStyle() override { @@ -1858,6 +1870,11 @@ NSArray* AllLoopModes = [NSArray arrayWithObjects: NSDefaultRunLoopMode, NSEvent _isExtended = value; } +-(bool) isDialog +{ + return _parent->IsDialog(); +} + -(double) getScaling { return _lastScaling; @@ -2036,7 +2053,22 @@ NSArray* AllLoopModes = [NSArray arrayWithObjects: NSDefaultRunLoopMode, NSEvent -(BOOL)canBecomeKeyWindow { - return _canBecomeKeyAndMain; + if (_canBecomeKeyAndMain) + { + // If the window has a child window being shown as a dialog then don't allow it to become the key window. + for(NSWindow* uch in [self childWindows]) + { + auto ch = objc_cast(uch); + if(ch == nil) + continue; + if (ch.isDialog) + return false; + } + + return true; + } + + return false; } -(BOOL)canBecomeMainWindow @@ -2044,22 +2076,6 @@ NSArray* AllLoopModes = [NSArray arrayWithObjects: NSDefaultRunLoopMode, NSEvent return _canBecomeKeyAndMain; } --(bool) activateAppropriateChild: (bool)activating -{ - for(NSWindow* uch in [self childWindows]) - { - auto ch = objc_cast(uch); - if(ch == nil) - continue; - [ch activateAppropriateChild:false]; - return FALSE; - } - - if(!activating) - [self makeKeyAndOrderFront:self]; - return TRUE; -} - -(bool)shouldTryToHandleEvents { return _isEnabled; @@ -2070,26 +2086,15 @@ NSArray* AllLoopModes = [NSArray arrayWithObjects: NSDefaultRunLoopMode, NSEvent _isEnabled = enable; } --(void)makeKeyWindow -{ - if([self activateAppropriateChild: true]) - { - [super makeKeyWindow]; - } -} - -(void)becomeKeyWindow { [self showWindowMenuWithAppMenu]; - if([self activateAppropriateChild: true]) + if(_parent != nullptr) { - if(_parent != nullptr) - { - _parent->BaseEvents->Activated(); - } + _parent->BaseEvents->Activated(); } - + [super becomeKeyWindow]; } @@ -2099,7 +2104,6 @@ NSArray* AllLoopModes = [NSArray arrayWithObjects: NSDefaultRunLoopMode, NSEvent if(parent != nil) { [parent removeChildWindow:self]; - [parent activateAppropriateChild: false]; } } diff --git a/src/Avalonia.Controls/ApiCompatBaseline.txt b/src/Avalonia.Controls/ApiCompatBaseline.txt index 11b1f4a833..bac569d563 100644 --- a/src/Avalonia.Controls/ApiCompatBaseline.txt +++ b/src/Avalonia.Controls/ApiCompatBaseline.txt @@ -10,4 +10,7 @@ EnumValuesMustMatch : Enum value 'Avalonia.Platform.ExtendClientAreaChromeHints InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Platform.ITopLevelImpl.SetCursor(Avalonia.Platform.ICursorImpl)' is present in the implementation but not in the contract. InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Platform.ITopLevelImpl.SetCursor(Avalonia.Platform.IPlatformHandle)' is present in the contract but not in the implementation. MembersMustExist : Member 'public void Avalonia.Platform.ITopLevelImpl.SetCursor(Avalonia.Platform.IPlatformHandle)' does not exist in the implementation but it does exist in the contract. -Total Issues: 11 +InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Platform.IWindowBaseImpl.Show(System.Boolean)' is present in the contract but not in the implementation. +MembersMustExist : Member 'public void Avalonia.Platform.IWindowBaseImpl.Show(System.Boolean)' does not exist in the implementation but it does exist in the contract. +InterfacesShouldHaveSameMembers : Interface member 'public void Avalonia.Platform.IWindowBaseImpl.Show(System.Boolean, System.Boolean)' is present in the implementation but not in the contract. +Total Issues: 14 diff --git a/src/Avalonia.Controls/Platform/IWindowBaseImpl.cs b/src/Avalonia.Controls/Platform/IWindowBaseImpl.cs index 0d303a6666..5172569726 100644 --- a/src/Avalonia.Controls/Platform/IWindowBaseImpl.cs +++ b/src/Avalonia.Controls/Platform/IWindowBaseImpl.cs @@ -7,7 +7,9 @@ namespace Avalonia.Platform /// /// Shows the window. /// - void Show(bool activate); + /// Whether to activate the shown window. + /// Whether the window is being shown as a dialog. + void Show(bool activate, bool isDialog); /// /// Hides the window. diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index 700c3d9bad..f5a86b8b21 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -671,7 +671,7 @@ namespace Avalonia.Controls SetWindowStartupLocation(Owner?.PlatformImpl); - PlatformImpl?.Show(ShowActivated); + PlatformImpl?.Show(ShowActivated, false); Renderer?.Start(); } OnOpened(EventArgs.Empty); @@ -743,7 +743,7 @@ namespace Avalonia.Controls SetWindowStartupLocation(owner.PlatformImpl); - PlatformImpl?.Show(ShowActivated); + PlatformImpl?.Show(ShowActivated, true); Renderer?.Start(); diff --git a/src/Avalonia.Controls/WindowBase.cs b/src/Avalonia.Controls/WindowBase.cs index cdcb499e98..3080def895 100644 --- a/src/Avalonia.Controls/WindowBase.cs +++ b/src/Avalonia.Controls/WindowBase.cs @@ -162,7 +162,7 @@ namespace Avalonia.Controls LayoutManager.ExecuteInitialLayoutPass(); _hasExecutedInitialLayoutPass = true; } - PlatformImpl?.Show(true); + PlatformImpl?.Show(true, false); Renderer?.Start(); OnOpened(EventArgs.Empty); } diff --git a/src/Avalonia.DesignerSupport/Remote/PreviewerWindowImpl.cs b/src/Avalonia.DesignerSupport/Remote/PreviewerWindowImpl.cs index 787f44887f..e82ec0fb0f 100644 --- a/src/Avalonia.DesignerSupport/Remote/PreviewerWindowImpl.cs +++ b/src/Avalonia.DesignerSupport/Remote/PreviewerWindowImpl.cs @@ -20,7 +20,7 @@ namespace Avalonia.DesignerSupport.Remote ClientSize = new Size(1, 1); } - public void Show(bool activate) + public void Show(bool activate, bool isDialog) { } @@ -99,10 +99,6 @@ namespace Avalonia.DesignerSupport.Remote { } - public void ShowDialog(IWindowImpl parent) - { - } - public void SetSystemDecorations(SystemDecorations enabled) { } diff --git a/src/Avalonia.DesignerSupport/Remote/Stubs.cs b/src/Avalonia.DesignerSupport/Remote/Stubs.cs index eedfc52d9d..e5fd5a91e6 100644 --- a/src/Avalonia.DesignerSupport/Remote/Stubs.cs +++ b/src/Avalonia.DesignerSupport/Remote/Stubs.cs @@ -77,7 +77,7 @@ namespace Avalonia.DesignerSupport.Remote { } - public void Show(bool activate) + public void Show(bool activate, bool isDialog) { } diff --git a/src/Avalonia.Headless/HeadlessWindowImpl.cs b/src/Avalonia.Headless/HeadlessWindowImpl.cs index af522f3e36..3c0dc5f6e2 100644 --- a/src/Avalonia.Headless/HeadlessWindowImpl.cs +++ b/src/Avalonia.Headless/HeadlessWindowImpl.cs @@ -75,7 +75,7 @@ namespace Avalonia.Headless public Action Closed { get; set; } public IMouseDevice MouseDevice { get; } - public void Show(bool activate) + public void Show(bool activate, bool isDialog) { if (activate) Dispatcher.UIThread.Post(() => Activated?.Invoke(), DispatcherPriority.Input); @@ -147,11 +147,6 @@ namespace Avalonia.Headless } - public void ShowDialog(IWindowImpl parent) - { - Show(true); - } - public void SetSystemDecorations(bool enabled) { diff --git a/src/Avalonia.Native/PopupImpl.cs b/src/Avalonia.Native/PopupImpl.cs index c36675afcd..8740dd6f12 100644 --- a/src/Avalonia.Native/PopupImpl.cs +++ b/src/Avalonia.Native/PopupImpl.cs @@ -60,14 +60,14 @@ namespace Avalonia.Native } } - public override void Show(bool activate) + public override void Show(bool activate, bool isDialog) { var parent = _parent; while (parent is PopupImpl p) parent = p._parent; if (parent is WindowImpl w) w.Native.TakeFocusFromChildren(); - base.Show(false); + base.Show(false, isDialog); } public override IPopupImpl CreatePopup() => new PopupImpl(_factory, _opts, _glFeature, this); diff --git a/src/Avalonia.Native/WindowImplBase.cs b/src/Avalonia.Native/WindowImplBase.cs index f716464d14..0dba11af5a 100644 --- a/src/Avalonia.Native/WindowImplBase.cs +++ b/src/Avalonia.Native/WindowImplBase.cs @@ -351,9 +351,9 @@ namespace Avalonia.Native } - public virtual void Show(bool activate) + public virtual void Show(bool activate, bool isDialog) { - _native.Show(activate.AsComBool()); + _native.Show(activate.AsComBool(), isDialog.AsComBool()); } diff --git a/src/Avalonia.Native/avn.idl b/src/Avalonia.Native/avn.idl index 3071333c5a..078922c584 100644 --- a/src/Avalonia.Native/avn.idl +++ b/src/Avalonia.Native/avn.idl @@ -430,7 +430,7 @@ interface IAvnString : IUnknown [uuid(e5aca675-02b7-4129-aa79-d6e417210bda)] interface IAvnWindowBase : IUnknown { - HRESULT Show(bool activate); + HRESULT Show(bool activate, bool isDialog); HRESULT Hide(); HRESULT Close(); HRESULT Activate(); diff --git a/src/Avalonia.X11/X11Window.cs b/src/Avalonia.X11/X11Window.cs index 8764b7bbc1..c7f619ffe7 100644 --- a/src/Avalonia.X11/X11Window.cs +++ b/src/Avalonia.X11/X11Window.cs @@ -808,7 +808,7 @@ namespace Avalonia.X11 XSetTransientForHint(_x11.Display, _handle, parent.Handle.Handle); } - public void Show(bool activate) + public void Show(bool activate, bool isDialog) { _wasMappedAtLeastOnce = true; XMapWindow(_x11.Display, _handle); diff --git a/src/Windows/Avalonia.Win32/PopupImpl.cs b/src/Windows/Avalonia.Win32/PopupImpl.cs index dd3fd1342c..7b2d20a986 100644 --- a/src/Windows/Avalonia.Win32/PopupImpl.cs +++ b/src/Windows/Avalonia.Win32/PopupImpl.cs @@ -17,7 +17,7 @@ namespace Avalonia.Win32 [ThreadStatic] private static IntPtr s_parentHandle; - public override void Show(bool activate) + public override void Show(bool activate, bool isDialog) { // Popups are always shown non-activated. UnmanagedMethods.ShowWindow(Handle.Handle, UnmanagedMethods.ShowWindowCommand.ShowNoActivate); diff --git a/src/Windows/Avalonia.Win32/Win32Platform.cs b/src/Windows/Avalonia.Win32/Win32Platform.cs index d92bd08d01..e3b01e8071 100644 --- a/src/Windows/Avalonia.Win32/Win32Platform.cs +++ b/src/Windows/Avalonia.Win32/Win32Platform.cs @@ -246,7 +246,7 @@ namespace Avalonia.Win32 public IWindowImpl CreateEmbeddableWindow() { var embedded = new EmbeddedWindowImpl(); - embedded.Show(true); + embedded.Show(true, false); return embedded; } diff --git a/src/Windows/Avalonia.Win32/WindowImpl.cs b/src/Windows/Avalonia.Win32/WindowImpl.cs index 3a3342fd14..c46cbb3274 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.cs @@ -570,7 +570,7 @@ namespace Avalonia.Win32 _shown = false; } - public virtual void Show(bool activate) + public virtual void Show(bool activate, bool isDialog) { SetParent(_parent); ShowWindow(_showWindowState, activate); @@ -1120,7 +1120,7 @@ namespace Avalonia.Win32 SetParent(null); if (shown) - Show(activated); + Show(activated, false); } } else diff --git a/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs b/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs index f3a1316c7d..569326bff3 100644 --- a/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ContextMenuTests.cs @@ -123,12 +123,97 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Cancel_Light_Dismiss_Closing_Keeps_Flyout_Open() + { + using (Application()) + { + popupImpl.Setup(x => x.Show(true, false)).Verifiable(); + popupImpl.Setup(x => x.Hide()).Verifiable(); + + var window = PreparedWindow(); + window.Width = 100; + window.Height = 100; + + var button = new Button + { + Height = 10, + Width = 10, + HorizontalAlignment = Layout.HorizontalAlignment.Left, + VerticalAlignment = Layout.VerticalAlignment.Top + }; + window.Content = button; + + window.ApplyTemplate(); + window.Show(); + + var tracker = 0; + + var c = new ContextMenu(); + c.ContextMenuClosing += (s, e) => + { + tracker++; + e.Cancel = true; + }; + button.ContextMenu = c; + c.Open(button); + + var overlay = LightDismissOverlayLayer.GetLightDismissOverlayLayer(window); + _mouse.Down(overlay, MouseButton.Left, new Point(90, 90)); + _mouse.Up(button, MouseButton.Left, new Point(90, 90)); + + Assert.Equal(1, tracker); + Assert.True(c.IsOpen); + + popupImpl.Verify(x => x.Hide(), Times.Never); + popupImpl.Verify(x => x.Show(true, false), Times.Exactly(1)); + } + } + + [Fact] + public void Light_Dismiss_Closes_Flyout() + { + using (Application()) + { + popupImpl.Setup(x => x.Show(true, false)).Verifiable(); + popupImpl.Setup(x => x.Hide()).Verifiable(); + + var window = PreparedWindow(); + window.Width = 100; + window.Height = 100; + + var button = new Button + { + Height = 10, + Width = 10, + HorizontalAlignment = Layout.HorizontalAlignment.Left, + VerticalAlignment = Layout.VerticalAlignment.Top + }; + window.Content = button; + + window.ApplyTemplate(); + window.Show(); + + var c = new ContextMenu(); + c.PlacementMode = PlacementMode.Bottom; + c.Open(button); + + var overlay = LightDismissOverlayLayer.GetLightDismissOverlayLayer(window); + _mouse.Down(overlay, MouseButton.Left, new Point(90, 90)); + _mouse.Up(button, MouseButton.Left, new Point(90, 90)); + + Assert.False(c.IsOpen); + popupImpl.Verify(x => x.Hide(), Times.Exactly(1)); + popupImpl.Verify(x => x.Show(true, false), Times.Exactly(1)); + } + } + [Fact] public void Clicking_On_Control_Toggles_ContextMenu() { using (Application()) { - popupImpl.Setup(x => x.Show(true)).Verifiable(); + popupImpl.Setup(x => x.Show(true, false)).Verifiable(); popupImpl.Setup(x => x.Hide()).Verifiable(); var sut = new ContextMenu(); @@ -148,7 +233,7 @@ namespace Avalonia.Controls.UnitTests _mouse.Click(target); Assert.False(sut.IsOpen); - popupImpl.Verify(x => x.Show(true), Times.Once); + popupImpl.Verify(x => x.Show(true, false), Times.Once); popupImpl.Verify(x => x.Hide(), Times.Once); } } @@ -158,7 +243,7 @@ namespace Avalonia.Controls.UnitTests { using (Application()) { - popupImpl.Setup(x => x.Show(true)).Verifiable(); + popupImpl.Setup(x => x.Show(true, false)).Verifiable(); popupImpl.Setup(x => x.Hide()).Verifiable(); var sut = new ContextMenu(); @@ -179,7 +264,7 @@ namespace Avalonia.Controls.UnitTests Assert.True(sut.IsOpen); popupImpl.Verify(x => x.Hide(), Times.Once); - popupImpl.Verify(x => x.Show(true), Times.Exactly(2)); + popupImpl.Verify(x => x.Show(true, false), Times.Exactly(2)); } } @@ -226,7 +311,7 @@ namespace Avalonia.Controls.UnitTests { using (Application()) { - popupImpl.Setup(x => x.Show(true)).Verifiable(); + popupImpl.Setup(x => x.Show(true, false)).Verifiable(); bool eventCalled = false; var sut = new ContextMenu(); @@ -242,7 +327,7 @@ namespace Avalonia.Controls.UnitTests Assert.True(eventCalled); Assert.False(sut.IsOpen); - popupImpl.Verify(x => x.Show(true), Times.Never); + popupImpl.Verify(x => x.Show(true, false), Times.Never); } } @@ -346,7 +431,7 @@ namespace Avalonia.Controls.UnitTests { using (Application()) { - popupImpl.Setup(x => x.Show(true)).Verifiable(); + popupImpl.Setup(x => x.Show(true, false)).Verifiable(); popupImpl.Setup(x => x.Hide()).Verifiable(); bool eventCalled = false; @@ -370,7 +455,7 @@ namespace Avalonia.Controls.UnitTests Assert.True(eventCalled); Assert.True(sut.IsOpen); - popupImpl.Verify(x => x.Show(true), Times.Once()); + popupImpl.Verify(x => x.Show(true, false), Times.Once()); popupImpl.Verify(x => x.Hide(), Times.Never); } } diff --git a/tests/Avalonia.Controls.UnitTests/WindowBaseTests.cs b/tests/Avalonia.Controls.UnitTests/WindowBaseTests.cs index 8109b037c5..1b4214e0c7 100644 --- a/tests/Avalonia.Controls.UnitTests/WindowBaseTests.cs +++ b/tests/Avalonia.Controls.UnitTests/WindowBaseTests.cs @@ -137,7 +137,7 @@ namespace Avalonia.Controls.UnitTests var target = new TestWindowBase(windowImpl.Object); target.IsVisible = true; - windowImpl.Verify(x => x.Show(true)); + windowImpl.Verify(x => x.Show(true, false)); } } diff --git a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs index 8a24a8366f..713283aa2b 100644 --- a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs +++ b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs @@ -58,7 +58,7 @@ namespace Avalonia.UnitTests windowImpl.Object.Resized?.Invoke(clientSize); }); - windowImpl.Setup(x => x.Show(true)).Callback(() => + windowImpl.Setup(x => x.Show(true, It.IsAny())).Callback(() => { windowImpl.Object.Activated?.Invoke(); });