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

Use DialogFragment for Android modals #22869

Merged
merged 3 commits into from
Jul 25, 2024
Merged

Conversation

pictos
Copy link
Contributor

@pictos pictos commented Jun 5, 2024

Description of Change

This should be merged on net9 branch, but since it's hard to make the .net9-preview don't mess-up with my env. I'm target .net8, when it gets approved this should rebased and re-target. I hope that @PureWeen can do this hard task for me xP.

Issues Fixed

Fixes #22587

@pictos pictos requested a review from a team as a code owner June 5, 2024 20:21
@pictos pictos requested review from mattleibow and rmarinho June 5, 2024 20:21
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jun 5, 2024
@PureWeen
Copy link
Member

PureWeen commented Jun 5, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

Don't worry @pictos, we can retarget to net9.0.

@PureWeen
Copy link
Member

PureWeen commented Jun 8, 2024

Android UITests failing
image

The controls device tests are crashing.
I'm guessing the modal tests inside device tests will need to be adjusted somewhat to handle DialogFragments

@pictos
Copy link
Contributor Author

pictos commented Jun 10, 2024

will take a look on those this week

@pictos
Copy link
Contributor Author

pictos commented Jun 11, 2024

@PureWeen, not sure if all tests will run again, but I think that I fixed the issue. I was using the DialogFragment.ShowNow method and it was causing issues with the FragmentManager, changing it to DialogFragment.Show seems to fix the issue.

image

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@pictos
Copy link
Contributor Author

pictos commented Jun 11, 2024

from the logs looks like the Appium couldn't find the element, but it navigates to the correct screen (at least locally), is that some Appium issue?

Failed AppDoesntCrashWhenReusingSameTitleView [16 s]
Error Message:
System.TimeoutException : Timed out waiting for element...
Stack Trace:
at UITest.Appium.HelperExtensions.Wait(Func1 query, Func2 satisfactory, String timeoutMessage, Nullable1 timeout, Nullable1 retryFrequency) in //src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 804
at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func1 query, String timeoutMessage, Nullable1 timeout, Nullable1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 821 at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable1 timeout, Nullable1 retryFrequency, Nullable1 postTimeout) in /
/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 316
at Microsoft.Maui.TestCases.Tests.Issues.Issue16499.AppDoesntCrashWhenReusingSameTitleView() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue16499.cs:line 19
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the pj/dialog-fragment branch from 78b2542 to cdd005f Compare June 26, 2024 00:45
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Jul 4, 2024

/rebase

@github-actions github-actions bot force-pushed the pj/dialog-fragment branch from cdd005f to 15c91f9 Compare July 4, 2024 20:13
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

These two tests fail for me locally

Maybe resolving them will fix the other exceptions?

image

@mattleibow
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Jul 6, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jonathanpeppers
Copy link
Member

/azp run

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@pictos pictos force-pushed the pj/dialog-fragment branch 2 times, most recently from 10d9bdb to 2e24b4e Compare July 22, 2024 17:03
add new shiny DialogFragment

refactoring code to find and dismiss DialogFragment

code cleanup

delete ModalContainer to use only ModalFragment

handle animation and add a map between page and dialogFragment

We've back button enabled!

After dismissing several demons summoned using obscure Android APIs, I was able to deal with the BackButtonPressed event

add modal animations as anim.xml files

using cleanup

remowork PopModalPlatformAsync to work with dialogFragment

remove tag

final adjustments on DialogFragment

change the ShowNow for Show to fix the issue

Wait for animation to complete

change local functions order

fix build

create window hooks for android (like iOS)

clean up ModalFragment fields

change Dictionary to ConditionalWeakTable

clean up event animation

refactor on Null notation

remove comments

- adjust back button

- different back button

code style

remove unused prop.

fix DontPushModalPagesWhenWindowIsDeactivated DeviceTest

completes the task

return back the way how modalManager handles android modals

normilize animation duration

Co-authored-by: Shane Neuville <shane94@hotmail.com>

remove focusability code

change how fragments are looked-up

code style
@PureWeen PureWeen force-pushed the pj/dialog-fragment branch from 2e24b4e to 31d5149 Compare July 22, 2024 18:44
@PureWeen PureWeen requested a review from a team as a code owner July 22, 2024 18:44
@PureWeen PureWeen changed the base branch from main to net9.0 July 22, 2024 18:44
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@pictos pictos force-pushed the pj/dialog-fragment branch from b0e0640 to 31d5149 Compare July 25, 2024 01:18
@pictos pictos force-pushed the pj/dialog-fragment branch from 3facb22 to 5b70755 Compare July 25, 2024 03:19
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

Test failures are on iOS. This PR only has changes on Android

@PureWeen PureWeen merged commit 63c6576 into dotnet:net9.0 Jul 25, 2024
108 of 111 checks passed
@samhouts samhouts added the fixed-in-net9.0-nightly This may be available in a nightly release! label Aug 2, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 13, 2024
@pictos pictos deleted the pj/dialog-fragment branch December 19, 2024 03:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-dialogalert DisplayAlert, dialog community ✨ Community Contribution fixed-in-net9.0-nightly This may be available in a nightly release! platform/android 🤖
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android] Convert Modal Implementation over to using Dialog Fragment
6 participants