Skip to content

[Android] Fix: Modal Animation Repeats When Returning from Background#28538

Closed
bhavanesh2001 wants to merge 7 commits intodotnet:mainfrom
bhavanesh2001:fix_28492
Closed

[Android] Fix: Modal Animation Repeats When Returning from Background#28538
bhavanesh2001 wants to merge 7 commits intodotnet:mainfrom
bhavanesh2001:fix_28492

Conversation

@bhavanesh2001
Copy link
Contributor

@bhavanesh2001 bhavanesh2001 commented Mar 20, 2025

Description of Change

This PR provides a fix for #28492 and is an alternative to #28522. It ensures that modal animations only play once when the modal is first displayed and do not replay when the app returns from the background.

Rather than starting the animation in OnStart(),this fix attaches a GlobalLayoutListener to the view inside OnCreateView(). When the view's layout is complete for the first time, the listener invokes the animation and then removes itself to prevent any further invocations. Additionally, the OnAnimationEnded method has been moved out of the lambda for improved readability.

After Fix:

fix.mov

Issues Fixed

Fixes #28492

Related PR #28522

@bhavanesh2001 bhavanesh2001 requested a review from a team as a code owner March 20, 2025 13:45
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Mar 20, 2025

Hey there @bhavanesh2001! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Mar 20, 2025
@jsuarezruiz jsuarezruiz added area-core-lifecycle XPlat and Native UIApplicationDelegate/Activity/Window lifecycle events platform/android area-controls-modal labels Mar 20, 2025
jsuarezruiz
jsuarezruiz suggested changes Mar 20, 2025
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

Could include an UITest like
https://github.com/user-attachments/assets/bf28a45f-dcde-4b26-bdb3-1342dc9d4a3c
?

In the test, can use:

App.BackgroundApp();
App.ForegroundApp();

To move the App to background etc.

@bhavanesh2001
Copy link
Contributor Author

bhavanesh2001 commented Mar 20, 2025

@jsuarezruiz Would it be possible to include a UI test for this? Since the issue is that the modal animation replays when returning from the background, how can we verify in an automated test that the animation has not been triggered again?

@bhavanesh2001 bhavanesh2001 requested a review from pictos March 20, 2025 15:06
@kubaflo
Copy link
Contributor

kubaflo commented Mar 20, 2025

I think GenericGlobalLayoutListener will cause memory leaks

@kubaflo
Copy link
Contributor

kubaflo commented Mar 20, 2025

IMO the animation should start in the OnStart() method like @pictos initially implemented.

GlobalLayoutListener is typically used to detect changes in the view's layout. Using it solely to start an animation adds unnecessary complexity.

@bhavanesh2001
Copy link
Contributor Author

bhavanesh2001 commented Mar 20, 2025

@kubaflo I get that GlobalLayoutListener might not seem like the perfect fit, but using OnStart() for this isn’t a good alternative either. Relying on flags isn't a clean solution as it introduces unnecessary state tracking instead of addressing the root cause directly, which is why I explored a different approach in this PR.

@mattleibow mattleibow added this to the .NET 9 SR6 milestone Mar 21, 2025
@mattleibow mattleibow moved this from Todo to Ready To Review in MAUI SDK Ongoing Mar 21, 2025
@bhavanesh2001
Copy link
Contributor Author

bhavanesh2001 commented Mar 21, 2025

I think GenericGlobalLayoutListener will cause memory leaks

@kubaflo I’ve added a memory leak test for when a modal is opened with animation, and it passes with the GenericGlobalLayoutListener implementation.

device_test.mov

@jsuarezruiz could you run pipelines on this?

@bhavanesh2001 bhavanesh2001 requested a review from PureWeen March 21, 2025 16:16
@PureWeen PureWeen modified the milestones: .NET 9 SR6, .NET 9 SR7 Mar 24, 2025
@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Mar 24, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Mar 24, 2025

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz suggested changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

The test Issue31366PushingAndPoppingModallyCausesArgumentOutOfRangeException is failing with a crash:

image

The app was expected to be running still, investigate as possible crash

Could you review if is related with the changes?

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Mar 24, 2025
@bhavanesh2001
Copy link
Contributor Author

bhavanesh2001 commented Mar 24, 2025 •
edited
Loading

@jsuarezruiz fixed failing tests.
Screenshot 2025-03-24 at 7 58 25 PM

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Mar 28, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Mar 28, 2025

Azure Pipelines successfully started running 3 pipeline(s).

@bhavanesh2001
Copy link
Contributor Author

bhavanesh2001 commented Apr 18, 2025

/rebase

@PureWeen PureWeen modified the milestones: .NET 9 SR7, .NET 9 SR8 May 8, 2025
@PureWeen PureWeen moved this from Changes Requested to Ready To Review in MAUI SDK Ongoing May 8, 2025
@bhavanesh2001
Copy link
Contributor Author

bhavanesh2001 commented May 16, 2025

Current Implementation Setting animation on SetWindowAnimations()
current_implementaton.mov
setting.windowanimations.mov

PureWeen
PureWeen requested changes May 16, 2025
View reviewed changes
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.

I'd like to iterate on this path first
https://github.com/dotnet/maui/tree/modal_window_anim

See if this just naturally works

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing May 16, 2025
@bhavanesh2001 bhavanesh2001 mentioned this pull request May 16, 2025
Merged
@PureWeen
Copy link
Member

PureWeen commented May 21, 2025

closing in favor of #29557

@PureWeen PureWeen closed this May 21, 2025
@github-project-automation github-project-automation bot moved this from Changes Requested to Done in MAUI SDK Ongoing May 21, 2025
@bhavanesh2001 bhavanesh2001 mentioned this pull request May 28, 2025
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@PureWeen PureWeen PureWeen requested changes

@sheiksyedm sheiksyedm sheiksyedm approved these changes

@jfversluis jfversluis Awaiting requested review from jfversluis jfversluis was automatically assigned from dotnet/dotnet-maui-reviewers

@tj-devel709 tj-devel709 Awaiting requested review from tj-devel709 tj-devel709 was automatically assigned from dotnet/dotnet-maui-reviewers

@pictos pictos Awaiting requested review from pictos

@jsuarezruiz jsuarezruiz Awaiting requested review from jsuarezruiz

Assignees

No one assigned

Labels

area-controls-modal area-core-lifecycle XPlat and Native UIApplicationDelegate/Activity/Window lifecycle events community ✨ Community Contribution partner/syncfusion/review platform/android

Projects

MAUI SDK Ongoing
Status: Done

Milestone

.NET 9 SR8

Development

Successfully merging this pull request may close these issues.

[Android] Modal Animation Repeats When Returning from Background

7 participants

@bhavanesh2001 @kubaflo @jsuarezruiz @PureWeen @pictos @sheiksyedm @mattleibow