Skip to content

Conversation

@jfversluis
Copy link
Member

@jfversluis jfversluis commented Aug 25, 2025

Copilot AI review requested due to automatic review settings August 25, 2025 12:37
@jfversluis jfversluis requested a review from a team as a code owner August 25, 2025 12:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces improvements for AOT/trimming compatibility, enhanced gesture handling, UI bug fixes, and expanded test coverage. The changes focus on Windows platform gesture management, CarouselView initialization, navigation stack management, and platform-specific color handling optimizations.

  • Adds AOT compatibility configuration to multiple project files and enables unsafe blocks where needed
  • Refactors Windows gesture event handling to use modern C# pattern matching and improves finger tracking
  • Fixes theme change handling for input controls and optimizes color management on iOS/Android platforms

Reviewed Changes

Copilot reviewed 58 out of 78 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Multiple .csproj files Add IsAotCompatible and AllowUnsafeBlocks properties for AOT/trimming support
Multiple Windows platform classes Convert to partial classes for source generator compatibility
GesturePlatformManager.Windows.cs Refactor gesture handling with pattern matching and improved finger tracking
Various iOS/Android platform files Optimize color handling and theme change responsiveness
Test case files Add new UI test cases and update existing CarouselView test
NavigationPage.Legacy.cs Fix navigation stack update timing for proper event firing

Comment on lines 50 to 67
var typedValue = new TypedValue();
if (OperatingSystem.IsAndroidVersionAtLeast(23) &&
editText.Context?.Theme is Resources.Theme theme &&
theme.ResolveAttribute(Android.Resource.Attribute.TextColorPrimary, typedValue, true) &&
editText.Resources?.GetColor(typedValue.ResourceId, theme) is Android.Graphics.Color color)
{
editText.SetTextColor(color);
}
}
}

Copy link

Copilot AI Aug 25, 2025

Choose a reason for hiding this comment

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

The fallback color logic is duplicated between UpdateTextColor and UpdatePlaceholderColor methods. Consider extracting this into a shared helper method to reduce code duplication.

Suggested change
var typedValue = new TypedValue();
if (OperatingSystem.IsAndroidVersionAtLeast(23) &&
editText.Context?.Theme is Resources.Theme theme &&
theme.ResolveAttribute(Android.Resource.Attribute.TextColorPrimary, typedValue, true) &&
editText.Resources?.GetColor(typedValue.ResourceId, theme) is Android.Graphics.Color color)
{
editText.SetTextColor(color);
}
}
}
var fallbackColor = GetFallbackColor(editText, Android.Resource.Attribute.TextColorPrimary);
if (fallbackColor is not null)
{
editText.SetTextColor(fallbackColor.Value);
}
}
}
// Shared helper for fallback color logic
private static Android.Graphics.Color? GetFallbackColor(EditText editText, int attribute)
{
var typedValue = new TypedValue();
if (OperatingSystem.IsAndroidVersionAtLeast(23) &&
editText.Context?.Theme is Resources.Theme theme &&
theme.ResolveAttribute(attribute, typedValue, true) &&
editText.Resources?.GetColor(typedValue.ResourceId, theme) is Android.Graphics.Color color)
{
return color;
}
return null;
}

Copilot uses AI. Check for mistakes.
Comment on lines +58 to +65








Copy link

Copilot AI Aug 25, 2025

Choose a reason for hiding this comment

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

The file contains excessive blank lines at the end (lines 60-65). These should be removed for cleaner code formatting.

Suggested change

Copilot uses AI. Check for mistakes.
@sheiksyedm
Copy link
Contributor

@kubaflo can you please look at these CI failures due to your PR changes?
image
image

@kubaflo
Copy link
Contributor

kubaflo commented Aug 27, 2025

@kubaflo can you please look at these CI failures due to your PR changes? image image

#31366
#31358

jfversluis added a commit that referenced this pull request Aug 27, 2025
#31358)

I have addressed the UI test image failure in this PR
(#31327) by updating and re-saving
the necessary UI test image.

I've verified the failure, and the snapshot had to be uploaded. It is
correct, as the default placeholder color should be white in the dark
theme
jfversluis added a commit that referenced this pull request Aug 27, 2025
#31346)

## Description
I have addressed the UI test image failures in this PR
(#31327) by updating and re-saving
the necessary UI test images.

#### TestCases:

- LinearGradientShouldHaveTheSameTopColorAsBackground
- BackgroundFrameResizesFastAndCorrectly
jfversluis added a commit that referenced this pull request Aug 28, 2025
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

### Issue Details
In the failed test
`AppDoesntCrashWhenSettingNewTitleViewOnExistingPage`, the TitleView was
updated in the OnAppearing method. When popping the page using PopAsync,
the OnAppearing event is triggered twice with my fix, whereas previously
it was triggered only once: once from
[FireAppearing](https://github.com/dotnet/maui/blob/b2c3452f68298f407ca18f8fe9e0f8841e7c9cad/src/Controls/src/Core/NavigationPage/NavigationPage.Legacy.cs#L67)
and once from
[CurrentPageChanged](https://github.com/dotnet/maui/blob/b2c3452f68298f407ca18f8fe9e0f8841e7c9cad/src/Controls/src/Core/NavigationPage/NavigationPage.Legacy.cs#L70).
This additional trigger causes the title to mismatch the expected value,
resulting in the test failure.

### Root Cause
The fix moves FireAppearing to after RemoveFromInnerChildren(page), but
at that point InternalChildren.Last() has already changed. Currently,
StackDepth - 2 is calculated based on the original stack depth. Once the
popped page is removed from stack, accessing InternalChildren[StackDepth
- 2] is not valid.

### Description of Change
Remove the page first, then use InternalChildren.Last(); directly to get
the correct page that should appear.

Regressed PR: #28666

Fixes AppDoesntCrashWhenSettingNewTitleViewOnExistingPage test failure
in PR: #31327

### Screenshots

<img width="540" height="300" alt="image"
src="https://github.com/user-attachments/assets/9dfaed05-0b66-489b-9b81-9a285088cc96"
/>
<img width="540" height="300" alt="image"
src="https://github.com/user-attachments/assets/81626605-6567-4a30-b183-d26513d5f60c"
/>
<img width="540" height="300" alt="image"
src="https://github.com/user-attachments/assets/260ca192-4baa-45e2-9991-8abcdadb2961"
/>
jfversluis added a commit that referenced this pull request Aug 29, 2025
… case failure in PR 31327 (#31366)

### Description of Change

Refactored fallback logic to use ObtainStyledAttributes and
ColorStateList for determining the system default text color. This
ensures correct color selection based on the enabled state of EditText,
improving consistency with system themes.

This also addresses the UI test image failure in this PR
(#31327)
@sheiksyedm
Copy link
Contributor

@kubaflo Still, this test failed. Can you look at this?
image

@kubaflo
Copy link
Contributor

kubaflo commented Sep 2, 2025

@kubaflo Still, this test failed. Can you look at this? image

Sure! Thanks for letting me know about this failure. Here's the pr fixing it: #31460

jfversluis added a commit that referenced this pull request Sep 4, 2025
… August 25th, 2025 Candidate) (#31460)

Introduces an internal UpdateTextColor method to TimePickerExtensions
for updating the text color of MauiTimePicker based on the ITimePicker's
TextColor property.

Fixes #31327 (comment)
@PureWeen
Copy link
Member

PureWeen commented Sep 4, 2025

/rebase

@PureWeen
Copy link
Member

PureWeen commented Sep 5, 2025

/rebase

kubaflo and others added 7 commits September 5, 2025 20:45
…28666)

* Fixed - 28414 : iOS] Popping a page includes in the NavigationStack when the OnAppearing method is called

* updated NavigationPage.Legacy.cs

* Updated NavigationPage.Legacy.cs
Corrects the color attribute assignment in AttributedStringExtensions for iOS to prevent crashes when setting CharacterSpacing on buttons. Adds new test cases and UI tests to verify the fix for issue #31238.
* Mark projects IsAotCompatible and AllowUnsafeBlocks

* Make Windows Platform classes partial

* Mark PlatformGraphicsView partial

* Mark SkiaGraphicsView partial
…ed returning false in a modally pushed page causes stack overflow - fix (#28812)

* Update ModalNavigationManager.Android.cs

[Android] ModalNavigationManager

* Update ModalNavigationManager.Android.cs

* Remove unused back propagation logic in Android modal manager

Eliminated the preventBackPropagation variable and related event handler code from ModalNavigationManager.Android.cs, simplifying the OnBackPressed lifecycle event handling.
kubaflo and others added 25 commits September 5, 2025 20:45
Refactored fallback logic to use ObtainStyledAttributes and ColorStateList for determining the system default text color. This ensures correct color selection based on the enabled state of EditText, improving consistency with system themes.
Introduces an internal UpdateTextColor method to TimePickerExtensions for updating the text color of MauiTimePicker based on the ITimePicker's TextColor property.
Updates the UpdateTextColor method to restore the default theme primary text color when no custom color is set, instead of passing null. This ensures consistent appearance on Android API 23+.
@PureWeen
Copy link
Member

PureWeen commented Sep 5, 2025

#31507

@PureWeen PureWeen closed this Sep 5, 2025
@PureWeen PureWeen deleted the inflight/candidate branch September 8, 2025 20:18
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants