From 411b8f039843e3c3478569985bd1a701497ace9e Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 4 Mar 2021 17:22:40 +0000 Subject: [PATCH 1/8] enumerate child windows and find out if close will be cancelled before closing. --- src/Avalonia.Controls/Window.cs | 38 ++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index ab99cfad17..c40787209e 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -482,10 +482,9 @@ namespace Avalonia.Controls try { - if (!ignoreCancel && HandleClosing()) + if (!ignoreCancel && ShouldCancelClose()) { close = false; - return; } } finally @@ -497,11 +496,25 @@ namespace Avalonia.Controls } } + /// + /// Handles a closing notification from . + /// true if closing is cancelled. Otherwise false. + /// + protected virtual bool HandleClosing() + { + if (ShouldCancelClose()) + { + CloseInternal(); + return true; + } + + return false; + } + private void CloseInternal() { foreach (var (child, _) in _children.ToList()) { - // if we HandleClosing() before then there will be no children. child.CloseInternal(); } @@ -515,20 +528,13 @@ namespace Avalonia.Controls PlatformImpl?.Dispose(); } - /// - /// Handles a closing notification from . - /// - protected virtual bool HandleClosing() + private bool ShouldCancelClose() { bool canClose = true; - + foreach (var (child, _) in _children.ToList()) { - if (!child.HandleClosing()) - { - child.CloseInternal(); - } - else + if (child.ShouldCancelClose()) { canClose = false; } @@ -541,10 +547,8 @@ namespace Avalonia.Controls return args.Cancel; } - else - { - return !canClose; - } + + return true; } protected virtual void HandleWindowStateChanged(WindowState state) From 460a5ba5ae30d486ed384fa4b99e93b8a3bfe357 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 4 Mar 2021 17:28:07 +0000 Subject: [PATCH 2/8] whitespace. --- src/Avalonia.Controls/Window.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index c40787209e..e98945777d 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -4,10 +4,7 @@ using System.ComponentModel; using System.Linq; using System.Reactive.Linq; using System.Threading.Tasks; -using Avalonia.Controls.Chrome; using Avalonia.Controls.Platform; -using Avalonia.Controls.Primitives; -using Avalonia.Data; using Avalonia.Input; using Avalonia.Interactivity; using Avalonia.Layout; @@ -531,7 +528,7 @@ namespace Avalonia.Controls private bool ShouldCancelClose() { bool canClose = true; - + foreach (var (child, _) in _children.ToList()) { if (child.ShouldCancelClose()) From 19e5b277f25cca790e293c4d1e5ba0c22c3ef22a Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 4 Mar 2021 17:51:01 +0000 Subject: [PATCH 3/8] add unit tests. --- .../WindowTests.cs | 102 +++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Controls.UnitTests/WindowTests.cs b/tests/Avalonia.Controls.UnitTests/WindowTests.cs index ba29001cf3..6d9b319a3e 100644 --- a/tests/Avalonia.Controls.UnitTests/WindowTests.cs +++ b/tests/Avalonia.Controls.UnitTests/WindowTests.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; -using Avalonia.Layout; using Avalonia.Platform; using Avalonia.Rendering; using Avalonia.UnitTests; @@ -137,6 +136,107 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(1, count); } } + + [Fact] + public void Child_windows_should_be_closed_before_parent() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window(); + var child = new Window(); + + int count = 0; + int windowClosing = 0; + int childClosing = 0; + int windowClosed = 0; + int childClosed = 0; + + window.Closing += (sender, e) => + { + count++; + windowClosing = count; + }; + + child.Closing += (sender, e) => + { + count++; + childClosing = count; + }; + + window.Closed += (sender, e) => + { + count++; + windowClosed = count; + }; + + child.Closed += (sender, e) => + { + count++; + childClosed = count; + }; + + window.Show(); + child.Show(window); + + window.Close(); + + Assert.Equal(2, windowClosing); + Assert.Equal(1, childClosing); + Assert.Equal(4, windowClosed); + Assert.Equal(3, childClosed); + } + } + + [Fact] + public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window(); + var child = new Window(); + + int count = 0; + int windowClosing = 0; + int childClosing = 0; + int windowClosed = 0; + int childClosed = 0; + + window.Closing += (sender, e) => + { + count++; + windowClosing = count; + e.Cancel = true; + }; + + child.Closing += (sender, e) => + { + count++; + childClosing = count; + }; + + window.Closed += (sender, e) => + { + count++; + windowClosed = count; + }; + + child.Closed += (sender, e) => + { + count++; + childClosed = count; + }; + + window.Show(); + child.Show(window); + + window.Close(); + + Assert.Equal(2, windowClosing); + Assert.Equal(1, childClosing); + Assert.Equal(0, windowClosed); + Assert.Equal(0, childClosed); + } + } [Fact] public void Showing_Should_Start_Renderer() From 50a88ae2db60aba84b17a72aedf9256a44830881 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Fri, 5 Mar 2021 11:27:43 +0000 Subject: [PATCH 4/8] share cancel event args and ensure event is raised on all windows. --- src/Avalonia.Controls/Window.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index e98945777d..2aca1854ce 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -525,13 +525,18 @@ namespace Avalonia.Controls PlatformImpl?.Dispose(); } - private bool ShouldCancelClose() + private bool ShouldCancelClose(CancelEventArgs args = null) { + if (args is null) + { + args = new CancelEventArgs(); + } + bool canClose = true; foreach (var (child, _) in _children.ToList()) { - if (child.ShouldCancelClose()) + if (child.ShouldCancelClose(args)) { canClose = false; } @@ -539,7 +544,6 @@ namespace Avalonia.Controls if (canClose) { - var args = new CancelEventArgs(); OnClosing(args); return args.Cancel; From e85da082b0129d02d805ee6c5258d6510e2d993e Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Fri, 5 Mar 2021 15:02:12 +0000 Subject: [PATCH 5/8] fix handleclosing logic. --- src/Avalonia.Controls/Window.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index 2aca1854ce..f51cf8b300 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -499,7 +499,7 @@ namespace Avalonia.Controls /// protected virtual bool HandleClosing() { - if (ShouldCancelClose()) + if (!ShouldCancelClose()) { CloseInternal(); return true; From 8b2fb2711a7d07c5583cfcbbfe2d09852d60fb79 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Fri, 5 Mar 2021 15:26:17 +0000 Subject: [PATCH 6/8] fix return value. --- src/Avalonia.Controls/Window.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Window.cs b/src/Avalonia.Controls/Window.cs index f51cf8b300..98f4cadc13 100644 --- a/src/Avalonia.Controls/Window.cs +++ b/src/Avalonia.Controls/Window.cs @@ -502,10 +502,10 @@ namespace Avalonia.Controls if (!ShouldCancelClose()) { CloseInternal(); - return true; + return false; } - return false; + return true; } private void CloseInternal() From f829137f73b9c6f858449c44d315650ef1f53c06 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Fri, 5 Mar 2021 16:04:23 +0000 Subject: [PATCH 7/8] add unit tests for window close when os button is clicked --- .../WindowTests.cs | 109 +++++++++++++++++- 1 file changed, 107 insertions(+), 2 deletions(-) diff --git a/tests/Avalonia.Controls.UnitTests/WindowTests.cs b/tests/Avalonia.Controls.UnitTests/WindowTests.cs index 6d9b319a3e..4822c07fa1 100644 --- a/tests/Avalonia.Controls.UnitTests/WindowTests.cs +++ b/tests/Avalonia.Controls.UnitTests/WindowTests.cs @@ -138,7 +138,112 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void Child_windows_should_be_closed_before_parent() + public void Child_windows_should_be_closed_before_parent_OSCloseButton() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window(); + var child = new Window(); + + int count = 0; + int windowClosing = 0; + int childClosing = 0; + int windowClosed = 0; + int childClosed = 0; + + window.Closing += (sender, e) => + { + count++; + windowClosing = count; + }; + + child.Closing += (sender, e) => + { + count++; + childClosing = count; + }; + + window.Closed += (sender, e) => + { + count++; + windowClosed = count; + }; + + child.Closed += (sender, e) => + { + count++; + childClosed = count; + }; + + window.Show(); + child.Show(window); + + var cancel = window.PlatformImpl.Closing(); + + Assert.Equal(false, cancel); + + Assert.Equal(2, windowClosing); + Assert.Equal(1, childClosing); + Assert.Equal(4, windowClosed); + Assert.Equal(3, childClosed); + } + } + + [Fact] + public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel_OSCloseButton() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var window = new Window(); + var child = new Window(); + + int count = 0; + int windowClosing = 0; + int childClosing = 0; + int windowClosed = 0; + int childClosed = 0; + + window.Closing += (sender, e) => + { + count++; + windowClosing = count; + e.Cancel = true; + }; + + child.Closing += (sender, e) => + { + count++; + childClosing = count; + }; + + window.Closed += (sender, e) => + { + count++; + windowClosed = count; + }; + + child.Closed += (sender, e) => + { + count++; + childClosed = count; + }; + + window.Show(); + child.Show(window); + + var cancel = window.PlatformImpl.Closing(); + + Assert.Equal(true, cancel); + + Assert.Equal(2, windowClosing); + Assert.Equal(1, childClosing); + Assert.Equal(0, windowClosed); + Assert.Equal(0, childClosed); + } + } + + [Fact] + public void Child_windows_should_be_closed_before_parent_Programatically() { using (UnitTestApplication.Start(TestServices.StyledWindow)) { @@ -188,7 +293,7 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel() + public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel_Programatically() { using (UnitTestApplication.Start(TestServices.StyledWindow)) { From 69a149f0ca1e8749bfa6ccaefe002289f7bbf204 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Fri, 5 Mar 2021 16:12:34 +0000 Subject: [PATCH 8/8] use inline data. --- .../WindowTests.cs | 133 ++++-------------- 1 file changed, 25 insertions(+), 108 deletions(-) diff --git a/tests/Avalonia.Controls.UnitTests/WindowTests.cs b/tests/Avalonia.Controls.UnitTests/WindowTests.cs index 4822c07fa1..e8311b79ac 100644 --- a/tests/Avalonia.Controls.UnitTests/WindowTests.cs +++ b/tests/Avalonia.Controls.UnitTests/WindowTests.cs @@ -137,8 +137,10 @@ namespace Avalonia.Controls.UnitTests } } - [Fact] - public void Child_windows_should_be_closed_before_parent_OSCloseButton() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Child_windows_should_be_closed_before_parent(bool programaticClose) { using (UnitTestApplication.Start(TestServices.StyledWindow)) { @@ -178,112 +180,16 @@ namespace Avalonia.Controls.UnitTests window.Show(); child.Show(window); - var cancel = window.PlatformImpl.Closing(); - - Assert.Equal(false, cancel); - - Assert.Equal(2, windowClosing); - Assert.Equal(1, childClosing); - Assert.Equal(4, windowClosed); - Assert.Equal(3, childClosed); - } - } - - [Fact] - public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel_OSCloseButton() - { - using (UnitTestApplication.Start(TestServices.StyledWindow)) - { - var window = new Window(); - var child = new Window(); - - int count = 0; - int windowClosing = 0; - int childClosing = 0; - int windowClosed = 0; - int childClosed = 0; - - window.Closing += (sender, e) => - { - count++; - windowClosing = count; - e.Cancel = true; - }; - - child.Closing += (sender, e) => - { - count++; - childClosing = count; - }; - - window.Closed += (sender, e) => + if (programaticClose) { - count++; - windowClosed = count; - }; - - child.Closed += (sender, e) => - { - count++; - childClosed = count; - }; - - window.Show(); - child.Show(window); - - var cancel = window.PlatformImpl.Closing(); - - Assert.Equal(true, cancel); - - Assert.Equal(2, windowClosing); - Assert.Equal(1, childClosing); - Assert.Equal(0, windowClosed); - Assert.Equal(0, childClosed); - } - } - - [Fact] - public void Child_windows_should_be_closed_before_parent_Programatically() - { - using (UnitTestApplication.Start(TestServices.StyledWindow)) - { - var window = new Window(); - var child = new Window(); - - int count = 0; - int windowClosing = 0; - int childClosing = 0; - int windowClosed = 0; - int childClosed = 0; - - window.Closing += (sender, e) => - { - count++; - windowClosing = count; - }; - - child.Closing += (sender, e) => - { - count++; - childClosing = count; - }; - - window.Closed += (sender, e) => - { - count++; - windowClosed = count; - }; - - child.Closed += (sender, e) => + window.Close(); + } + else { - count++; - childClosed = count; - }; + var cancel = window.PlatformImpl.Closing(); - window.Show(); - child.Show(window); - - window.Close(); + Assert.Equal(false, cancel); + } Assert.Equal(2, windowClosing); Assert.Equal(1, childClosing); @@ -292,8 +198,10 @@ namespace Avalonia.Controls.UnitTests } } - [Fact] - public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel_Programatically() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Child_windows_must_not_close_before_parent_has_chance_to_Cancel_OSCloseButton(bool programaticClose) { using (UnitTestApplication.Start(TestServices.StyledWindow)) { @@ -334,7 +242,16 @@ namespace Avalonia.Controls.UnitTests window.Show(); child.Show(window); - window.Close(); + if (programaticClose) + { + window.Close(); + } + else + { + var cancel = window.PlatformImpl.Closing(); + + Assert.Equal(true, cancel); + } Assert.Equal(2, windowClosing); Assert.Equal(1, childClosing);