Feature Matrix Test Sample updates to verify the iOS Label performance improvement fix#31159
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Label Feature Matrix test sample and reference images following iOS performance improvements from PR #30864. The changes modify the test framework to support more granular testing of formatted text scenarios and update the UI test automation accordingly.
- Updated Label Feature Matrix sample to separate simple formatted text from complex scenarios
- Modified test automation to include new SimpleFormattedText interaction steps
- Updated reference images across platforms (Windows, Android, iOS) to reflect current rendering
Reviewed Changes
Copilot reviewed 4 out of 31 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
LabelFeatureTests.cs |
Added SimpleFormattedText constant and updated test methods to interact with new checkbox option |
LabelViewModel.cs |
Removed default FormattedText initialization and set LineHeight default to -1 |
LabelOptionsPage.xaml.cs |
Added event handler for SimpleFormattedText checkbox to create basic formatted text |
LabelOptionsPage.xaml |
Added UI checkbox for "Single Line Formatted Text" option with proper AutomationId |
Comments suppressed due to low confidence (1)
src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/LabelFeatureTests.cs:66
- Removing the WaitForElement check for "This is a Basic Label" may cause test instability. Since this text is now created via the SimpleFormattedText checkbox, the test should either wait for this element after tapping the checkbox or verify the UI state is ready before proceeding.
{
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
albyrock87
left a comment
There was a problem hiding this comment.
I see there are still a lot of failures in the CI regarding Entry, which is what I was experiencing locally with my PR.
In all those use cases I was wondering about the expectations, like sometimes I really can't tell which is the expected behavior.
....WinUI.Tests/snapshots/windows/VerifyHorizontalTextAlignmentWhenVerticalTextAlignmentSet.png
Show resolved
Hide resolved
...s/tests/TestCases.iOS.Tests/snapshots/ios/VerifyClearVisiblityButtonWhenTextColorChanged.png
Show resolved
Hide resolved
ca1d952 to
109383b
Compare
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
@jsuarezruiz, it looks like the drop folder is still unavailable because the build hasn’t completed yet. Could you please check this and do the needful? |
|
/rebase |
c8d5b8c to
7c6f58a
Compare
|
/azp run MAUI-UITests-public |
1 similar comment
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3e9f116 to
7d5c171
Compare
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — Reverted unwanted code changes ·
|
| Thread | Topic | Status |
|---|---|---|
| WinUI clear button position | Whether clear button shows on left or right | |
| iOS clear button color | Red-ish color after TextColor changed | |
| Label.Mapper.cs compound condition | Combine conditions with && |
✅ Resolved (author fixed) |
| Entry.Mapper.cs other platforms | Should Editor/SearchBar get same fix? | |
| Entry.Mapper.cs parameter rename | label → entry |
✅ Resolved (author fixed) |
| LabelControlPage.xaml FormattedText tests | Are FormattedText binding tests present? |
Review Status
- jsuarezruiz: CHANGES_REQUESTED (×3) — latest (Oct 10, 2025) noted failing Android tests
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31159 | Skip MapFormatting/UpdateValue during handler connection via IsConnectingHandler() guards | ⏳ PENDING (Gate) | 4 source files | Original PR |
🚦 Gate — Test Verification
📝 Review Session — Reverted unwanted code changes · 8dc29e7
Result: ❌ FAILED
Platform: iOS
Mode: Full Verification
Test Filter: VerifyLabelWithFormattedText
Verification Results
| Check | Expected | Actual | Result |
|---|---|---|---|
| Tests WITHOUT fix | FAIL | PASS | ❌ |
| Tests WITH fix | PASS | FAIL | ❌ |
Root Cause Analysis
The Gate failed for the following reasons:
Why tests PASS without fix:
- The code fix (
IsConnectingHandler()guards inLabel.iOS.cs,Entry.iOS.cs) is a performance/correctness fix that prevents redundantMapFormattingcalls during handler connection. For simple test scenarios on iOS, the visual output is the same with or without these guards, so the test screenshot comparison passes either way.
Why tests FAIL with fix:
- The PR adds a second
VerifyScreenshot()call afterApp.Tap(MainLabel)in every LabelFeatureTest, EntryFeatureTest, and ButtonFeatureTest. This verifies the rendering is correct after page re-initialization. - iOS snapshot images for the second (re-initialization) screenshot are MISSING from the PR. The PR only updated Android, Windows, and Mac snapshot images, but not iOS.
- When
VerifyScreenshot()is called the second time, it cannot find the expected reference image for iOS, causing all affected tests to fail.
Missing iOS Snapshots
The following test files are missing their post-reinitialization iOS snapshots:
VerifyLabelWithFormattedText(and ~20+ other Label tests)- ~10+ Entry tests
- ~6+ Button tests
These would need iOS snapshots added to src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/ (or ios/).
🔧 Fix — Analysis & Comparison
📝 Review Session — Reverted unwanted code changes · 8dc29e7
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31159 | Skip MapFormatting/UpdateValue during handler connection via IsConnectingHandler() guards | ❌ FAIL (Gate) | 4 source files | Gate failed - missing iOS snapshots |
Exhausted: N/A (skipped — Gate failed)
Selected Fix: N/A — Gate must pass before Fix phase
Why skipped: Gate phase failed because iOS snapshot reference images are missing for the new test states introduced by this PR. Per the PR agent workflow, Phase 3 (Fix) is only run after Gate passes. The recommended action is for the PR author to add the missing iOS snapshot images and resolve failing Android tests.
📋 Report — Final Recommendation
📝 Review Session — Reverted unwanted code changes · 8dc29e7
⚠️ Final Recommendation: REQUEST CHANGES
Summary
PR #31159 pairs a valid iOS performance improvement (skipping redundant MapFormatting during handler connection) with test infrastructure updates that verify the fix through page re-initialization. However, the iOS snapshot reference images for the new re-initialization test states are missing, causing all affected LabelFeatureTests, EntryFeatureTests, and ButtonFeatureTests to fail on iOS.
Gate Result: ❌ FAILED
- Tests PASS without fix (the
IsConnectingHandler()guard doesn't produce visible rendering differences in the specific test scenarios used) - Tests FAIL with fix (missing iOS reference snapshots for the second
VerifyScreenshot()call in each test)
Root Cause
The PR adds a second VerifyScreenshot() call after App.Tap(MainLabel) (page re-initialization) to every affected feature matrix test. The PR updated Android, Windows, and Mac snapshot images for these new states, but did not add the corresponding iOS snapshot images. When tests run on iOS, VerifyScreenshot() cannot find the expected reference image for the re-initialized page state and fails.
Code Review
Positive aspects:
- The
IsConnectingHandler()guards inLabel.iOS.cs,Label.Mapper.cs,Entry.iOS.cs, andEntry.Mapper.csare sound performance improvements that prevent redundantMapFormattingcalls during the handler connection phase - Moving the
MapTextTransforminto its own method (Entry.Mapper.cs) is cleaner separation of concerns - The
LabelViewModelchange (removing defaultFormattedText, changinglineHeightto-1) correctly resets the initial state for more accurate tests - The page re-initialization approach via
MainLabel_Tappedis a good pattern for testing handler connection behavior
Issues found:
-
Missing iOS snapshots (blocker) — The PR updated snapshots for Android, Windows, and Mac but NOT iOS. All LabelFeatureTests (~20+), EntryFeatureTests, and ButtonFeatureTests that now call
VerifyScreenshot()twice will fail on iOS without reference images for the post-reinitialization state. These should be added tosrc/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/(orios/). -
Failing Android tests (blocker) — Reviewer jsuarezruiz reported failing Android tests in their October 2025 review. Similar snapshot update issues may also affect Android.
-
Outstanding reviewer feedback — jsuarezruiz has requested changes 3 times (most recently Oct 10, 2025) and the PR is still marked
CHANGES_REQUESTED. The unresolved discussion about extending the fix toEditor,SearchBar, and other platforms is marked as future work in a separate PR — acceptable if documented. -
Minor comment quality — In
Entry.Mapper.cs,MapTextTransform's comment says "we don't want to map the text multiple times" but is more precisely "we skip TextTransform application during initial handler connection since MapText already handles text during connection." This is minor but could mislead future readers. -
Missing newline at end of file — Several modified
.csfiles are missing a newline at end of file, which is a minor code style issue but not blocking.
Requested Changes
-
Add iOS snapshot images for all tests with the new
App.Tap(MainLabel)+VerifyScreenshot()calls. Run the tests on iOS to generate the images and commit them. -
Address failing Android tests noted by jsuarezruiz. Verify Android snapshot images are complete and up-to-date for all modified tests.
-
Resolve outstanding reviewer discussions with jsuarezruiz who has pending
CHANGES_REQUESTEDreviews.
📋 Expand PR Finalization Review
Title: ✅ Good
Current: Feature Matrix Test Sample updates to verify the iOS Label performance improvement fix
Description: ❌ Needs Rewrite
Description needs updates. See details below.
✨ Suggested PR Description
[!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 from this PR and let us know in a comment if this change resolves your issue. Thank you!
Root Cause
On iOS, MapText in the handler always called MapFormatting immediately after updating the native text. When a handler connects, all properties are mapped in sequence — so MapText was triggering MapFormatting (which re-applies CharacterSpacing, MaxLength/CharacterSpacing, and HorizontalTextAlignment), only to have those properties re-applied again moments later as the mapper continued iterating. This caused redundant attributed string work and degraded performance.
Additionally, the PropertyMapper for Label, Entry, and Button did not guarantee that Text was mapped before formatting properties. If formatting (e.g., Font, TextDecorations) ran before Text, it operated on a stale or empty attributed string, forcing MapText to re-apply everything.
Description of Change
Core performance changes (iOS):
-
LabelHandler.cs— Introduced a newTextMapperthat explicitly mapsTextfirst, followed byFont,LineHeight,TextDecorations,CharacterSpacing,HorizontalTextAlignment,VerticalTextAlignment,TextColor. The mainMapperinheritsTextMapperas its first base, guaranteeing initialization order. -
EntryHandler.cs— SameTextMapperextraction:Text,MaxLength,Font,CharacterSpacing,TextColorare mapped in order before other properties. -
ButtonHandler.cs—Textmoved beforeCharacterSpacing,Font,TextColorinTextButtonMapper. -
LabelHandler.iOS.cs,EntryHandler.iOS.cs,ButtonHandler.iOS.cs—MapTextnow skipsMapFormattingwhenhandler.IsConnectingHandler()is true. During connection, formatting properties will be applied in proper order by the mapper, so callingMapFormattinginsideMapTextwould be redundant double-work. -
Label.iOS.cs,Entry.iOS.cs(Controls layer) — SameIsConnectingHandler()guard applied in the controls-layerMapTextoverride. -
Label.Mapper.cs—MapFormattedTextand theHasFormattedTextSpanspath inMapFontnow checkIsConnectingHandler()to avoid double mapping during handler connection. -
Entry.Mapper.cs— NewMapTextTransformmapper method introduced; skipsMapTextduring handler connection to avoid redundant mapping. TheTextTransformproperty now usesMapTextTransforminstead of pointing directly toMapText. -
EntryHandler.iOS.csMapFormatting— Refactored from direct platform calls (handler.PlatformView?.Update*) tohandler.UpdateValue(nameof(...))so formatting properties go through the mapper dispatch and benefit from the new ordering.
Test sample changes:
-
Label/Entry/Button
ControlPage— Added aTapGestureRecognizeron theMainLabelelement that callsInitializeComponent()again to recreate the page, enabling tests to verify handler reconnect/initial mapping behavior. -
LabelViewModel.cs— Removed defaultFormattedTextinitialization (label now starts with plain text by default).lineHeightdefault changed from0to-1to match native default. -
LabelOptionsPage.xaml/cs— New "Single Line Formatted Text" checkbox added to set a simpleFormattedStringwith a single Span for testing. -
EntryOptionsPage.xaml— AddedFontSize="10"to theHEndbutton to prevent layout overflow.
Snapshot updates:
- 40+ snapshot images updated across Android, iOS, Mac, and Windows reflecting the new default state (no default FormattedText, corrected lineHeight default).
Issues Fixed
Contributing to #30864
Platforms Tested
- iOS
- Android
- Mac Catalyst
- Windows
Code Review: ✅ Passed
Code Review — PR #31159
Code Review Findings
🟡 Suggestions
1. Missing newline at end of file in several files
The diff shows \ No newline at end of file for:
src/Core/src/Handlers/Entry/EntryHandler.cssrc/Core/src/Handlers/Label/LabelHandler.iOS.cs(indirectly via the Label.iOS.cs diff)src/Controls/src/Core/Entry/Entry.Mapper.cssrc/Controls/src/Core/Entry/Entry.iOS.cssrc/Controls/src/Core/Label/Label.Mapper.cssrc/Controls/src/Core/Label/Label.iOS.cssrc/Controls/tests/TestCases.HostApp/FeatureMatrix/Button/ButtonControlPage.xaml.cssrc/Controls/tests/TestCases.HostApp/FeatureMatrix/EntryControl/EntryControlPage.xaml.cssrc/Controls/tests/TestCases.HostApp/FeatureMatrix/Label/LabelControlPage.xaml.cssrc/Controls/tests/TestCases.HostApp/FeatureMatrix/Label/LabelOptionsPage.xaml.cs
These should each end with a newline. Run dotnet format to fix.
2. Inconsistent comment verbosity in iOS handler files
EntryHandler.iOS.cs MapText comment is verbose:
// If we're not connecting the handler, we need to update the text formatting
// This is because the text may have changed, and we need to ensure that
// any attributed string formatting is applied correctly.LabelHandler.iOS.cs MapText uses the original shorter comment:
// Any text update requires that we update any attributed string formattingFor consistency, both should use the same comment style. The original shorter comment is cleaner.
3. ButtonFeatureTests.cs uses wrong AutomationId constant
In ButtonFeatureTests.cs:
const string MainLabel = "ClickedEventLabel";But then it's used as:
App.Tap(MainLabel);This taps the ClickedEventLabel, not a MainLabel that triggers InitializeComponent() re-execution. Looking at ButtonControlPage.xaml.cs, the label that triggers reinit has AutomationId="MainLabel" and the handler is MainLabel_Tapped. The constant should be:
const string MainLabel = "MainLabel";This appears to be a bug in the test — it would be tapping the wrong element. Verify that ClickedEventLabel is actually the one with the TapGestureRecognizer, or update the constant to "MainLabel".
4. LabelFeatureTests.cs has inconsistent indentation in two blocks
In some VerifyScreenshot additions, 4-space indentation is used instead of tabs:
App.Tap(MainLabel);
VerifyScreenshot(tolerance: 0.5, retryTimeout: TimeSpan.FromSeconds(2));while the rest of the file uses tabs. Run dotnet format to normalize.
5. MapTextTransform in Entry.Mapper.cs guard is confusing
The new method:
static void MapTextTransform(IEntryHandler handler, Entry entry)
{
if (entry.IsConnectingHandler())
{
// If we're connecting the handler, we don't want to map the text multiple times.
return;
}
MapText(handler, entry);
}The comment says "we don't want to map the text multiple times" but it reads as if MapTextTransform always maps text even when there's no transform. The intent is correct (avoid double text mapping during connect), but the naming/comment could be clearer:
static void MapTextTransform(IEntryHandler handler, Entry entry)
{
if (entry.IsConnectingHandler())
{
// Text will be mapped in proper order during handler connection; skip here.
return;
}
// TextTransform affects the displayed text, so re-apply it when the transform changes.
MapText(handler, entry);
}✅ Looks Good
- Mapper ordering approach — Using a
TextMapperas a base for the mainMapperto guaranteeTextis mapped before formatting properties is clean and architecturally sound. EntryHandler.iOS.csMapFormattingrefactor — Switching fromhandler.PlatformView?.Update*()tohandler.UpdateValue(nameof(...))is correct; it lets the mapper dispatch handle the property update, which is consistent with how the rest of the framework works.IsConnectingHandler()guard — SkippingMapFormattingduring handler connection is the right fix; during connection all properties are mapped in order anyway.LabelViewModel.cs— Removing the defaultFormattedTextin the constructor simplifies the default state and makes the Label Feature Matrix tests start from a clean, predictable state. ChanginglineHeightdefault to-1correctly matches the platform default (no line height set).LabelOptionsPageSimpleFormattedTextcheckbox — Good addition; allows tests to opt-in to a minimalFormattedStringstate without requiring the full formatted text test flow.LabelControlPage.xamlreformatting — Indentation cleanup is welcome.
🚦 Gate - Test Before and After Fix📊 Expand Full Gate —
|
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31159 | Introduce TextMapper with ordered mappings; skip MapFormatting in MapText during handler connection; update test samples/snapshots |
❌ FAILED (Gate) | 10 impl + 13 test + 64 snapshots | Gate failed on Android |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | iOS-only IsConnectingHandler() guards in platform files, no mapper reorder |
❌ FAIL | 7 files | 12/13 pass; TailTruncation reinit fails 3.86% visual diff; Material3 baselines missing |
| 2 | try-fix | Revert MapFont IsConnectingHandler guard + fix Material3 test + update ALL baselines |
✅ PASS | 7+ files | 111/111 tests pass; root cause: MapFont guard redundant/buggy — skips UpdateValue on Android |
| 3 | try-fix | Keep PR TextMapper + remove MapFont guard only, no baseline update |
❌ FAIL | 1 file | 108/111; Head/MiddleTruncation baselines mismatch PR's updated images |
| 4 | try-fix | Platform-conditional TextMapper (iOS/Mac/Tizen only) + remove MapFont guard |
❌ FAIL | 3 files | Same 3 failures as Attempt 3; Android baselines mismatch PR images regardless |
| PR | PR #31159 | TextMapper + iOS IsConnectingHandler() guards + snapshot/test updates |
❌ FAILED (Gate) | 87 files | Bug in MapFont guard causes Android rendering inconsistency |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | Attempt 2 is correct and complete |
| claude-sonnet-4.6 | 2 | No | MapFont guard redundant — MapFormattedText already has its own guard |
| gpt-5.3-codex | 2 | Yes | Deferred post-connect UpdateValue on Android — overly complex vs. removing redundant guard |
| gpt-5.4 | 2 | No | Baselines must be updated; no approach avoids this |
Exhausted: Yes
Selected Fix: Attempt 2 — Remove MapFont !handler.IsConnectingHandler() guard + update Android baselines + fix Material3 test ordering. The guard is redundant (MapFormattedText already has its own guard) and silently skips UpdateValue(FormattedText) during connect on Android causing visual inconsistency.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | PR contributes to iOS Label perf improvement (#30864); 87 files (10 impl, 13 test, 64 snapshots) |
| Gate | ❌ FAILED | Android — tests did NOT behave as expected |
| Try-Fix | ✅ COMPLETE | 4 attempts; 1 passing (Attempt 2) |
| Report | ✅ COMPLETE |
Summary
PR #31159 extends the iOS Label performance improvement from PR #30864 by introducing a TextMapper that ensures Text is mapped before formatting attributes (Font, CharacterSpacing, etc.), adds IsConnectingHandler() guards in iOS platform files to skip redundant MapFormatting during initial handler connection, and updates Feature Matrix test samples and snapshots.
The Gate failed on Android because the PR contains a bug: Label.Mapper.cs's MapFont method incorrectly adds !handler.IsConnectingHandler() to a guard that is already made redundant by MapFormattedText's own IsConnectingHandler guard. On iOS this was a silent double-guard with no effect, but on Android it caused UpdateValue(FormattedText) to be silently skipped during initial handler connection, resulting in visual rendering inconsistency for FormattedText labels with certain LineBreakModes (Head/Middle Truncation).
Root Cause
In src/Controls/src/Core/Label/Label.Mapper.cs, MapFont contains:
// PR's buggy code:
if (label.HasFormattedTextSpans && !handler.IsConnectingHandler())
{
handler.UpdateValue(nameof(FormattedText));
}The !handler.IsConnectingHandler() guard is redundant and harmful. When HasFormattedTextSpans is true and the handler IS connecting, UpdateValue(FormattedText) is silently skipped. But MapFormattedText already has its own IsConnectingHandler guard:
static void MapFormattedText(ILabelHandler handler, Label label)
{
if (label.IsConnectingHandler()) return; // already handled
MapText(handler, label);
}So the correct behavior is for MapFont to call UpdateValue(FormattedText) unconditionally when HasFormattedTextSpans is true — the MapFormattedText guard ensures no double work on iOS. Removing the !handler.IsConnectingHandler() from MapFont is the correct fix.
Additionally, the Material3 Label test (Material3LabelFeatureTests) has a test ordering dependency on LabelFeatureTests for initial page state.
Fix Quality
PR's fix is mostly correct but has one bug. The required changes to the PR are:
-
Remove
!handler.IsConnectingHandler()fromMapFontinLabel.Mapper.cs— this guard is redundant and causes Android rendering inconsistency. Keep onlyif (label.HasFormattedTextSpans). -
Update Android snapshot baselines for
VerifyLabelWithFormattedTextWhenLineBreakModeHeadTruncationandVerifyLabelWithFormattedTextWhenLineBreakModeMiddleTruncation— theTextMapperreordering (Text first) legitimately changes how these render on Android, and the PR already provides updated images for these; they just need to match the correct rendering after fixing theMapFontbug. -
Fix Material3 test ordering —
Material3LabelFeatureTestsshould not depend onLabelFeatureTestsfor page state initialization.
The core architecture of the PR (TextMapper, IsConnectingHandler in iOS handlers, test reinit cycles) is sound. The fix is surgical: one redundant guard removal in Label.Mapper.cs.
Suggested Code Fix
// src/Controls/src/Core/Label/Label.Mapper.cs
static void MapFont(ILabelHandler handler, Label label, Action<IElementHandler, IElement> baseMethod)
{
- // if there is formatted text,
- // then we re-apply the whole formatted text
- if (label.HasFormattedTextSpans && !handler.IsConnectingHandler())
+ if (label.HasFormattedTextSpans)
{
handler.UpdateValue(nameof(FormattedText));
}
kubaflo
left a comment
There was a problem hiding this comment.
Could you please resolve conflicts?


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 from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
The iOS Label performance was improved in PR Improve iOS Label performance #30864. In that PR, the Label and Entry Feature Matrix test sample and script were modified, which caused discrepancies in the expected images due to changes which is due to test sample's default property values. In this PR, I updated the test sample and re-saved the images accordingly.
Windows - The Entry is now unfocused, so I re-saved the latest image.
Android - I modified the test sample by altering the default values and re-saved two images.
Issues Fixed
Contributing to #30864