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

[android]: ModalContainer Destroy commit allowing state loss #23400

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,17 @@ public void Destroy()
_rootViewLayoutListener?.Invalidate();
_rootViewLayoutListener = null;

_fragmentManager
.BeginTransaction()
.Remove(_modalFragment)
.Commit();
if (_windowMauiContext.Context is not null)
{
_fragmentManager.RunOrWaitForResume(_windowMauiContext.Context, fm =>
{
fm
.BeginTransaction()
.Remove(_modalFragment)
.SetReorderingAllowed(true)
.Commit();
});
}

Modal = null;
_windowMauiContext = null;
Expand Down
50 changes: 50 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue23399.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
namespace Maui.Controls.Sample.Issues
{
[Issue(IssueTracker.Github, 23399, "Closing Modal While App is Backgrounded Fails", PlatformAffected.Android)]
public class Issue23399: NavigationPage
{

public Issue23399() : base(new TestPage())
{
}

public class TestPage : TestContentPage
{
protected override void Init()
{
Content = new VerticalStackLayout()
{
new Button()
{
Text = "Open Modal",
AutomationId = "OpenModal",
Command = new Command(async () =>
{
await Navigation.PushModalAsync(CreateDestinationPage());
})
}
};
}

ContentPage CreateDestinationPage()
{
return new ContentPage()
{
Title = "Test",
Content = new VerticalStackLayout()
{
new Button()
{
AutomationId = "StartCloseModal",
Text = "Close Modal",
Command = new Command(()=>
{
Navigation.PopModalAsync();
})
}
}
};
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#if ANDROID
using NUnit.Framework;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.TestCases.Tests.Issues
{
public class Issue23399(TestDevice device) : _IssuesUITest(device)
{
public override string Issue => "Closing Modal While App is Backgrounded Fails";

[Test]
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
[Test]
[Test]
[Category(UITestCategories.Navigation)]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the category in a964d40

Copy link
Member

Choose a reason for hiding this comment

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

We'll need to add a Category to this before merging. We recently merged some changes that requires every test to have a Category

I would add it myself, but, it looks like I don't have permission to make changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PureWeen it's probably because I forked the repository into our organization's workspace.

Added the category.

public void MakingFragmentRelatedChangesWhileAppIsBackgroundedFails()
{
try
{
App.WaitForElement("OpenModal");
App.Tap("OpenModal");
App.WaitForElement("StartCloseModal");
App.Tap("StartCloseModal");
App.BackgroundApp();
App.WaitForNoElement("StartCloseModal");
App.ForegroundApp();
App.WaitForElement("OpenModal");
}
catch
Copy link
Member

Choose a reason for hiding this comment

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

We don't need these bits anymore

We had to do this before we were just restarting the app between every test

So, you're fine to just remove this try/catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PureWeen removed the try catch block in.

a964d40

{
SaveUIDiagnosticInfo();

// Just in case these tests leave the app in an unreliable state
App.ResetApp();
FixtureSetup();
throw;
}
}
}
}
#endif
Loading