Skip to content

Feature Matrix Test Sample updates to verify the iOS Label performance improvement fix#31159

Open
Tamilarasan-Paranthaman wants to merge 21 commits intodotnet:inflight/currentfrom
Tamilarasan-Paranthaman:label-perf-impr
Open

Feature Matrix Test Sample updates to verify the iOS Label performance improvement fix#31159
Tamilarasan-Paranthaman wants to merge 21 commits intodotnet:inflight/currentfrom
Tamilarasan-Paranthaman:label-perf-impr

Conversation

@Tamilarasan-Paranthaman
Copy link
Copy Markdown
Member

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman commented Aug 13, 2025

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

@dotnet-policy-service dotnet-policy-service bot added community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration labels Aug 13, 2025
@Tamilarasan-Paranthaman Tamilarasan-Paranthaman marked this pull request as ready for review August 13, 2025 18:56
Copilot AI review requested due to automatic review settings August 13, 2025 18:56
@Tamilarasan-Paranthaman Tamilarasan-Paranthaman requested a review from a team as a code owner August 13, 2025 18:56
Copy link
Copy Markdown
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 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.
	{

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman changed the title [Testing] Update Label Feature Matrix sample and images Feature Matrix Test Sample updates to verify the iOS Label performance improvement fix Aug 14, 2025
@PureWeen
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Copy Markdown
Contributor

@albyrock87 albyrock87 left a comment

Choose a reason for hiding this comment

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

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.

@jsuarezruiz
Copy link
Copy Markdown
Contributor

/azp run MAUI-UITests-public

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
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.

image Pending snapshots already available in the latest build. Could you commit the images?

@github-project-automation github-project-automation bot moved this from Todo to Changes Requested in MAUI SDK Ongoing Sep 4, 2025
@Tamilarasan-Paranthaman
Copy link
Copy Markdown
Member Author

image Pending snapshots already available in the latest build. Could you commit the images?

@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?

@PureWeen PureWeen modified the milestones: .NET 9 SR11, .NET 9 SR12 Sep 10, 2025
@PureWeen PureWeen added the p/0 Current heighest priority issues that we are targeting for a release. label Sep 10, 2025
@jsuarezruiz
Copy link
Copy Markdown
Contributor

/rebase

@jsuarezruiz
Copy link
Copy Markdown
Contributor

/azp run MAUI-UITests-public

1 similar comment
@jsuarezruiz
Copy link
Copy Markdown
Contributor

/azp run MAUI-UITests-public

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rmarinho
Copy link
Copy Markdown
Member

rmarinho commented Feb 18, 2026

🤖 AI Summary

📊 Expand Full Review
🔍 Pre-Flight — Context & Validation
📝 Review SessionReverted unwanted code changes · 8dc29e7

PR: #31159 - Feature Matrix Test Sample updates to verify the iOS Label performance improvement fix
Author: Tamilarasan-Paranthaman (community/Syncfusion partner)
Milestone: .NET 10 SR6
Platforms Affected: iOS, macOS, Android, Windows (snapshot/test sample fixes; core code fix is iOS/Entry)
Files Changed: 4 source code files, 12+ snapshot images, 9 test infrastructure files

PR Summary

This PR contains two distinct categories of changes:

  1. iOS/Entry Performance Fix (Source Code):

    • Label.iOS.cs: Wraps MapFormatting call in if (!handler.IsConnectingHandler()) to avoid redundant formatting during handler connection
    • Label.Mapper.cs: Fixes MapFont to skip re-applying FormattedText during handler connection, cleans up comment placement
    • Entry.iOS.cs: Wraps EntryHandler.MapFormatting in if (!handler.IsConnectingHandler())
    • Entry.Mapper.cs: Separates MapTextTransform from MapText, adds IsConnectingHandler guard
  2. Test Sample & Snapshot Updates:

    • LabelViewModel.cs: Removed default FormattedText initialization, changed lineHeight default from 0 to -1
    • LabelOptionsPage.xaml.cs: Added SimpleFormattedTextCheckBox_CheckedChanged handler for new checkbox
    • LabelControlPage.xaml.cs, EntryControlPage.xaml.cs, ButtonControlPage.xaml.cs: Added MainLabel_Tapped handlers that re-initialize pages (for testing initial mapper behavior)
    • Android snapshot images: 4 updated (Label tests)
    • Windows snapshot images: 11 updated (Label and Entry tests)
    • Mac snapshot images: 9 updated (Entry tests)

Related PR

Contributing to #30864 (iOS Label performance improvement)

Reviewer Discussion

Thread Topic Status
WinUI clear button position Whether clear button shows on left or right ⚠️ Unresolved (explained behavior: shows on right)
iOS clear button color Red-ish color after TextColor changed ⚠️ Unresolved (new issue #31326 filed)
Label.Mapper.cs compound condition Combine conditions with && ✅ Resolved (author fixed)
Entry.Mapper.cs other platforms Should Editor/SearchBar get same fix? ⚠️ Unresolved (author wants separate PR)
Entry.Mapper.cs parameter rename labelentry ✅ Resolved (author fixed)
LabelControlPage.xaml FormattedText tests Are FormattedText binding tests present? ⚠️ Unresolved (author responded: yes, they exist)

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 SessionReverted 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 in Label.iOS.cs, Entry.iOS.cs) is a performance/correctness fix that prevents redundant MapFormatting calls 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 after App.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 SessionReverted 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 SessionReverted 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 in Label.iOS.cs, Label.Mapper.cs, Entry.iOS.cs, and Entry.Mapper.cs are sound performance improvements that prevent redundant MapFormatting calls during the handler connection phase
  • Moving the MapTextTransform into its own method (Entry.Mapper.cs) is cleaner separation of concerns
  • The LabelViewModel change (removing default FormattedText, changing lineHeight to -1) correctly resets the initial state for more accurate tests
  • The page re-initialization approach via MainLabel_Tapped is a good pattern for testing handler connection behavior

Issues found:

  1. 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 to src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/ (or ios/).

  2. Failing Android tests (blocker) — Reviewer jsuarezruiz reported failing Android tests in their October 2025 review. Similar snapshot update issues may also affect Android.

  3. 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 to Editor, SearchBar, and other platforms is marked as future work in a separate PR — acceptable if documented.

  4. 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.

  5. Missing newline at end of file — Several modified .cs files are missing a newline at end of file, which is a minor code style issue but not blocking.

Requested Changes

  1. 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.

  2. Address failing Android tests noted by jsuarezruiz. Verify Android snapshot images are complete and up-to-date for all modified tests.

  3. Resolve outstanding reviewer discussions with jsuarezruiz who has pending CHANGES_REQUESTED reviews.


📋 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 new TextMapper that explicitly maps Text first, followed by Font, LineHeight, TextDecorations, CharacterSpacing, HorizontalTextAlignment, VerticalTextAlignment, TextColor. The main Mapper inherits TextMapper as its first base, guaranteeing initialization order.

  • EntryHandler.cs — Same TextMapper extraction: Text, MaxLength, Font, CharacterSpacing, TextColor are mapped in order before other properties.

  • ButtonHandler.csText moved before CharacterSpacing, Font, TextColor in TextButtonMapper.

  • LabelHandler.iOS.cs, EntryHandler.iOS.cs, ButtonHandler.iOS.csMapText now skips MapFormatting when handler.IsConnectingHandler() is true. During connection, formatting properties will be applied in proper order by the mapper, so calling MapFormatting inside MapText would be redundant double-work.

  • Label.iOS.cs, Entry.iOS.cs (Controls layer) — Same IsConnectingHandler() guard applied in the controls-layer MapText override.

  • Label.Mapper.csMapFormattedText and the HasFormattedTextSpans path in MapFont now check IsConnectingHandler() to avoid double mapping during handler connection.

  • Entry.Mapper.cs — New MapTextTransform mapper method introduced; skips MapText during handler connection to avoid redundant mapping. The TextTransform property now uses MapTextTransform instead of pointing directly to MapText.

  • EntryHandler.iOS.cs MapFormatting — Refactored from direct platform calls (handler.PlatformView?.Update*) to handler.UpdateValue(nameof(...)) so formatting properties go through the mapper dispatch and benefit from the new ordering.

Test sample changes:

  • Label/Entry/Button ControlPage — Added a TapGestureRecognizer on the MainLabel element that calls InitializeComponent() again to recreate the page, enabling tests to verify handler reconnect/initial mapping behavior.

  • LabelViewModel.cs — Removed default FormattedText initialization (label now starts with plain text by default). lineHeight default changed from 0 to -1 to match native default.

  • LabelOptionsPage.xaml/cs — New "Single Line Formatted Text" checkbox added to set a simple FormattedString with a single Span for testing.

  • EntryOptionsPage.xaml — Added FontSize="10" to the HEnd button 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.cs
  • src/Core/src/Handlers/Label/LabelHandler.iOS.cs (indirectly via the Label.iOS.cs diff)
  • src/Controls/src/Core/Entry/Entry.Mapper.cs
  • src/Controls/src/Core/Entry/Entry.iOS.cs
  • src/Controls/src/Core/Label/Label.Mapper.cs
  • src/Controls/src/Core/Label/Label.iOS.cs
  • src/Controls/tests/TestCases.HostApp/FeatureMatrix/Button/ButtonControlPage.xaml.cs
  • src/Controls/tests/TestCases.HostApp/FeatureMatrix/EntryControl/EntryControlPage.xaml.cs
  • src/Controls/tests/TestCases.HostApp/FeatureMatrix/Label/LabelControlPage.xaml.cs
  • src/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 formatting

For 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 TextMapper as a base for the main Mapper to guarantee Text is mapped before formatting properties is clean and architecturally sound.
  • EntryHandler.iOS.cs MapFormatting refactor — Switching from handler.PlatformView?.Update*() to handler.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 — Skipping MapFormatting during handler connection is the right fix; during connection all properties are mapped in order anyway.
  • LabelViewModel.cs — Removing the default FormattedText in the constructor simplifies the default state and makes the Label Feature Matrix tests start from a clean, predictable state. Changing lineHeight default to -1 correctly matches the platform default (no line height set).
  • LabelOptionsPage SimpleFormattedText checkbox — Good addition; allows tests to opt-in to a minimal FormattedString state without requiring the full formatted text test flow.
  • LabelControlPage.xaml reformatting — Indentation cleanup is welcome.

@rmarinho rmarinho added s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-gate-failed AI could not verify tests catch the bug s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Feb 18, 2026
@PureWeen PureWeen modified the milestones: .NET 10 SR6, .NET 10 SR7 Mar 3, 2026
@MauiBot
Copy link
Copy Markdown
Collaborator

MauiBot commented Mar 29, 2026

🚦 Gate - Test Before and After Fix

📊 Expand Full Gate8dc29e7 · Reverted unwanted code changes

Gate Result: ❌ FAILED

Platform: android


@MauiBot
Copy link
Copy Markdown
Collaborator

MauiBot commented Mar 29, 2026

🤖 AI Summary

📊 Expand Full Review8dc29e7 · Reverted unwanted code changes
🔍 Pre-Flight — Context & Validation

Issue: Contributing to #30864 - Improve iOS Label performance
PR: #31159 - Feature Matrix Test Sample updates to verify the iOS Label performance improvement fix
Platforms Affected: iOS, Android, Windows, MacCatalyst
Files Changed: 10 implementation, 13 test/FeatureMatrix, 64 snapshot images

Key Findings

  • PR Improve iOS Label performance #30864 improved iOS Label performance by ensuring Text is mapped before formatting attributes. This PR extends/validates those changes and updates test samples/snapshots affected by the behavioral change.
  • Core implementation change: TextMapper introduced in LabelHandler.cs and EntryHandler.cs to guarantee Text is mapped before Font, LineHeight, CharacterSpacing, HorizontalTextAlignment, TextColor, etc. This ensures AttributedText is constructed correctly on iOS.
  • LabelHandler.iOS.cs / EntryHandler.iOS.cs / ButtonHandler.iOS.cs: MapText now skips calling MapFormatting when IsConnectingHandler() is true, since formatting attributes will be applied in order via TextMapper.
  • Label.Mapper.cs: MapFont now checks !handler.IsConnectingHandler() before triggering a full FormattedText refresh.
  • Entry.Mapper.cs: TextTransform mapping now uses MapTextTransform (skips during connect) instead of MapText.
  • Tests add a "tap MainLabel → verify screenshot" cycle to verify that page re-initialization (which reconnects handlers) renders identically to the first render.
  • Android snapshot updates: 4 android images updated due to test sample default property changes.
  • Gate ❌ FAILED on Android — tests did NOT behave as expected.
  • jsuarezruiz review noted Android may also benefit from similar optimizations (deferred to separate PR).
  • Prior agent review exists from rmarinho (2026-02-18) but details unavailable.
  • Open (unresolved) review thread: extending the optimization to Editor and SearchBar controls (deferred by PR author).
  • ButtonControlPage.xaml.cs page reinit calls scope.UnregisterName() — unusual pattern that could cause issues if names change.

Fix Candidates

# 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:

  1. Remove !handler.IsConnectingHandler() from MapFont in Label.Mapper.cs — this guard is redundant and causes Android rendering inconsistency. Keep only if (label.HasFormattedTextSpans).

  2. Update Android snapshot baselines for VerifyLabelWithFormattedTextWhenLineBreakModeHeadTruncation and VerifyLabelWithFormattedTextWhenLineBreakModeMiddleTruncation — the TextMapper reordering (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 the MapFont bug.

  3. Fix Material3 test orderingMaterial3LabelFeatureTests should not depend on LabelFeatureTests for 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));
     }

@MauiBot MauiBot added s/agent-review-incomplete AI agent could not complete all phases (blocker, timeout, error) s/agent-changes-requested AI agent recommends changes - found a better alternative or issues and removed s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-review-incomplete AI agent could not complete all phases (blocker, timeout, error) labels Mar 29, 2026
@kubaflo kubaflo changed the base branch from main to inflight/current April 5, 2026 11:10
Copy link
Copy Markdown
Contributor

@kubaflo kubaflo left a comment

Choose a reason for hiding this comment

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

Could you please resolve conflicts?

@github-project-automation github-project-automation bot moved this from Ready To Review to Changes Requested in MAUI SDK Ongoing Apr 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community ✨ Community Contribution p/0 Current heighest priority issues that we are targeting for a release. partner/syncfusion Issues / PR's with Syncfusion collaboration s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-gate-failed AI could not verify tests catch the bug s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review)

Projects

Status: Changes Requested

Development

Successfully merging this pull request may close these issues.

10 participants