Skip to content

[Android] Fix app crash caused by dynamic template switching in ListView #24808

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

Merged
PureWeen merged 8 commits into from
Feb 18, 2025
Merged

[Android] Fix app crash caused by dynamic template switching in ListView #24808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f3a35c7
fix-24701--Removed-Parent-from-PlatformView-Before-Adding-to-View.
BagavathiPerumal Sep 17, 2024
fad7b59
Fix-24701-Re-enabled the device test for android.
BagavathiPerumal Sep 23, 2024
ccbd4a8
fix-24701-Default-color-set-when-current-device-count-is-not-availabl…
BagavathiPerumal Oct 11, 2024
6b5e7f0
fix-24701-Removed a redundant if condition that was already checked i…
BagavathiPerumal Oct 16, 2024
368e00a
commit to handle null value effectively
SuthiYuvaraj Oct 23, 2024
cc79372
Commit for review concerns.
SuthiYuvaraj Oct 28, 2024
e5dc8c4
Update Application.cs
SuthiYuvaraj Oct 29, 2024
b6abc86
Update Application.cs
SuthiYuvaraj Oct 29, 2024
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
5 changes: 3 additions & 2 deletions src/Controls/src/Core/Application/Application.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,9 @@ public static Color? AccentColor

return null;
#elif ANDROID
if (Current?.Windows?.Count > 0)
return Current.Windows[0].MauiContext.Context?.GetAccentColor();
var context = (Current?.Windows?.Count > 0 && Current.Windows[0].MauiContext is not null) ? Current.Windows[0].MauiContext.Context : Current?.FindMauiContext()?.Context;
Copy link
Copy Markdown
Member

@PureWeen PureWeen Jan 6, 2025

Choose a reason for hiding this comment

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

Did you validate if Current?.FindMauiContext()?.Context will return an accent color? That's the application context on the Activity so I feel like it won't have the theme resources loaded into it.

Copy link
Copy Markdown
Contributor

@praveenkumarkarunanithi praveenkumarkarunanithi Jan 7, 2025

Choose a reason for hiding this comment

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

@PureWeen

Thank you for the feedback. Here are the findings:

Windows.Count Unavailability: The condition where Current?.Windows?.Count is not available and the fallback to FindMauiContext()?.Context is triggered occurs specifically during Android device tests. Notably, this behavior is not observed when running individual device tests locally, suggesting it is tied to the test execution environment rather than regular application behavior.

Accent Color Validation: When falling back to FindMauiContext()?.Context, we are still able to retrieve an accent color, which is: [Color: Red=0.101960786, Green=0.4509804, Blue=0.9098039, Alpha=1]

In contrast, when Windows.Count is available and the primary path is used (Current.Windows[0].MauiContext.Context), the accent color retrieved is: [Color: Red=0.20392157, Green=0.59607846, Blue=0.85882354, Alpha=1]

While there is a slight variation between these colors, both provide valid accent values for their respective scenarios.
Please let us know if there’s anything further you would like us to address.

if (context is not null)
return context?.GetAccentColor();
Copy link
Copy Markdown
Contributor

@jsuarezruiz jsuarezruiz Oct 29, 2024

Choose a reason for hiding this comment

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

If the current Window MauiContext is null, we keep having the same null Color exception, right?
We could fallback to Application.Current.FindMauiContext() in that case.

Copy link
Copy Markdown
Contributor

@SuthiYuvaraj SuthiYuvaraj Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@jsuarezruiz , Thank you for the feedback. I’ve added a fallback to ensure context is retrieved effectively in all cases, even when the primary window context might not be accessible.

Please review and let us know if any further changes are required.


return null;
#elif IOS
Expand Down
1 change: 1 addition & 0 deletions src/Controls/src/Core/Compatibility/Handlers/ListView/Android/ViewCellRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ public void Update(ViewCell cell)

var platformView = _viewCell.View.ToPlatform(Element.FindMauiContext());
_viewHandler = (IPlatformViewHandler)_viewCell.View.Handler;
platformView.RemoveFromParent();
AddView(platformView);

UpdateIsEnabled();
Expand Down
6 changes: 1 addition & 5 deletions src/Controls/tests/DeviceTests/Elements/ListView/ListViewTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,7 @@ void SetupBuilder()
}


[Fact
#if ANDROID
(Skip = "https://github.com/dotnet/maui/issues/24701")
#endif
]
[Fact]
public async Task ChangingTemplateTypeDoesNotCrash()
{
SetupBuilder();
Expand Down