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.

Sign up for GitHub

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

Jump to bottom

Fix syncing modal stack when the window is created/recreated #13025

Merged

PureWeen merged 42 commits into main from fix_modal_syncing

Mar 16, 2023

Member

PureWeen commented Jan 31, 2023 •

edited

Loading

Description of Change

Fix ModalNavigationManager so it will reprocess the current Modal stack when the root window handler changes. Currently we aren't processing any modal pages pushed before the app has loaded and we don't reprocess the modal stack if the window is destroyed and recreated (Android).

This PR adds code to detect when we need to reprocess the modal stack based on the root view changing or being destroyed

Breaking Changes

On Window if a lifecycle event is called at a place in time that doesn't make sense, we will now throw an exception. https://github.com/dotnet/maui/pull/13025/files#diff-07d5a2c0b5069d448623e88d890e41bc2c21e12f99bd01f1349903a332d4130cR475-R519

Issues Fixed

Fixes #8157

PureWeen changed the title ~~Fix modal syncing~~ Fix syncing modal stack when the window is created/recreated

Eilon added the area-controls-modal label

PureWeen force-pushed the fix_modal_syncing branch 3 times, most recently from d8e5af8 to 7548317 Compare

February 6, 2023 23:09

PureWeen commented

View reviewed changes

src/Controls/src/Core/HandlerImpl/Window/Window.Impl.cs

-              				nextPage?.SendNavigatedTo(new NavigatedToEventArgs(modal));
-              				return result;
+              				return _owner.ModalNavigationManager.PopModalAsync(animated);

Member Author

PureWeen Feb 6, 2023

I moved all of this code into ModalNavigationManager

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Windows.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Show resolved Hide resolved

mandel-macaque reviewed

View reviewed changes

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

PureWeen mentioned this pull request

Fix a couple life cycle scenarios on shell modals #13177

Merged

PureWeen requested a review from mandel-macaque

February 7, 2023 20:17

PureWeen marked this pull request as ready for review

February 7, 2023 20:43

mandel-macaque requested changes

View reviewed changes

Member

mandel-macaque left a comment

Some minor comments.

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Windows.cs Outdated Show resolved Hide resolved

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.iOS.cs Outdated Show resolved Hide resolved

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.iOS.cs Outdated Show resolved Hide resolved

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.iOS.cs Outdated Show resolved Hide resolved

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.iOS.cs Outdated Show resolved Hide resolved

src/Controls/tests/DeviceTests/ControlsHandlerTestBase.iOS.cs Outdated Show resolved Hide resolved

PureWeen requested a review from mandel-macaque

February 8, 2023 21:59

mandel-macaque approved these changes

View reviewed changes

Member

mandel-macaque left a comment

Just need a small comment about the try {} trick.

src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.cs Outdated Show resolved Hide resolved

PureWeen added 8 commits

February 10, 2023 10:27


          Better sync modal stack on startup

5091d36


          - fix up windows modal paths

d4233a5


          - simplify window and add shell tests

1ec4e36


          - add more tests and fix appearing on window

eaf5575


          - cleanup from comments

abd613f


          - fix tizen

afc1289


          Update ModalNavigationManager.Tizen.cs

f487f60


          Update ModalNavigationManager.Tizen.cs

e42c881

PureWeen requested a review from rmarinho

March 7, 2023 00:30

PureWeen added 4 commits

March 7, 2023 11:49


          Merge branch 'main' into fix_modal_syncing

334b129


          - Add better exception if user calls pop modal with no modal page pre…

2a9a11a

…sent


          - fix test code for dismissing modals

30ffd8d


          Merge branch 'main' into fix_modal_syncing

c74b697

Member Author

PureWeen commented Mar 14, 2023

/azp run

azure-pipelines bot commented Mar 14, 2023

Azure Pipelines successfully started running 2 pipeline(s).

PureWeen added 3 commits

March 14, 2023 07:26


          Merge branch 'main' into fix_modal_syncing

bf2e4b5


          - fix define on MaybeNull

43c148b


          Merge branch 'main' into fix_modal_syncing

ee78b12

mattleibow reviewed

View reviewed changes

src/Core/src/Platform/iOS/ModalWrapper.cs

+              namespace Microsoft.Maui.Platform
+              {
+              	internal class ModalWrapper : UIViewController

Member

mattleibow Mar 14, 2023

Should this rather be an interface if it does not do anything specific?

Member Author

PureWeen Mar 15, 2023

Perhaps eventually?

At some point we'll move modals into core and those APIs will all get designed. There's not really a larger use case here yet for an interface so I don't think we need to make that leap yet.

PureWeen and others added 5 commits

March 15, 2023 10:57


          - fix up tests related to keyboard

ac1e149


          - fix compile error on compatibility

9a39a53


          - fix timing of setting parent on modal page

a956239


          - fix shell and add tests

2c45ed8


          Auto-format source code

2a49b9a

Contributor

github-actions bot commented Mar 15, 2023

Thank you for your pull request. We are auto-formatting your source code to follow our code guidelines.


          Merge branch 'main' into fix_modal_syncing

f05b2ba

rmarinho reviewed

View reviewed changes

src/Compatibility/Core/src/Android/AppCompat/FormsAppCompatActivity.cs Show resolved Hide resolved


          - fix retrieving of CurrentPage

d39d677

PureWeen requested review from rmarinho and mattleibow

March 16, 2023 11:59

rmarinho approved these changes

View reviewed changes

PureWeen merged commit c9d697c into main

PureWeen deleted the fix_modal_syncing branch

March 16, 2023 14:24

PureWeen mentioned this pull request

Fix issues with shell and pushing modals early #13990

Closed

samhouts mentioned this pull request

Blazor Hybrid Windows: Window was already activated when minimize and restore from taskbar #14142

Closed

samhouts mentioned this pull request

[regression/8.0.0-preview.3.8149] - modal to modal doesnt call OnNavigatedTo #17978

Closed

github-actions bot locked and limited conversation to collaborators

samhouts added the fixed-in-8.0.0-preview.3.8149 label

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-modal fixed-in-8.0.0-preview.3.8149 needs-breaking-change-doc-created t/breaking 💥