[release/11.0.1xx-preview7] Reset WinUI Label line height to default - #37062
[release/11.0.1xx-preview7] Reset WinUI Label line height to default#37062kubaflo wants to merge 12 commits into
Conversation
Refresh the 17 deterministic Label baselines produced by the current Preview 7 test host. The same failures reproduce with the blessed SDK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
/azp run |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37062Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37062" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
I don’t think updating these shared baselines fixes the underlying failure. Each affected Label test compares the same baseline both before and after Suggested change: keep the existing baselines and preserve the page, navigation stack, toolbar, and surrounding layout. Put This remains entirely test-host-only and preserves the intended invariant that rendering is identical before and after recreation. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
Implemented the review feedback at |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Controls/tests/TestCases.HostApp/FeatureMatrix/Label/LabelControlPage.xaml.cs:28
_mainLabel = (Label)MainLabelHost.Children[0];assumes the host has at least one child and that the first child is aLabel. If the XAML changes (or is reordered), this will throw (IndexOutOfRange/InvalidCast) and fail the feature matrix page initialization. Consider validating count/type and producing a clearer failure.
InitializeComponent();
_mainLabel = (Label)MainLabelHost.Children[0];
_viewModel = viewModel;
src/Controls/tests/TestCases.HostApp/FeatureMatrix/Label/LabelControlPage.xaml.cs:56
CreateMainLabelsetsBindingContext = _viewModelon the new Label. After the first tap, the Label has a local BindingContext, so laterBindingContext = _viewModel = new LabelViewModel();inNavigateToOptionsPage_Clickedwill no longer propagate to the recreated label (it will keep the old VM). Rely on inherited BindingContext instead, or explicitly update_mainLabel.BindingContextwhen the page BindingContext changes.
var label = new Label
{
Style = style,
BindingContext = _viewModel,
};
src/Controls/tests/TestCases.HostApp/FeatureMatrix/Label/LabelControlPage.xaml.cs:39
- PR title/description say this is a baseline-only update (“Update WinUI Label snapshots”), but this PR also changes the HostApp Label feature-matrix page behavior (recreating the main Label instance on tap and refactoring bindings into a Style). If the intent is baseline-only, these functional changes should be moved to a separate PR; otherwise, please update the description to mention the behavioral change so future archaeology doesn’t miss it.
void MainLabel_Tapped(object sender, TappedEventArgs e)
{
var oldLabel = _mainLabel;
var style = oldLabel.Style ?? throw new InvalidOperationException("MainLabel style is required.");
Let recreated labels inherit the page BindingContext so reopening options can replace the active view model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
Follow-up d6faea8 removes the recreated Label local BindingContext, so it continues inheriting the page context when the Options flow replaces the view model. The HostApp builds successfully. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
Head |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Core/src/Handlers/Label/LabelHandler.cs:22
- The comment says WinUI must apply “text formatting” before
Text, but in this mapperTextColorandTextDecorationsare intentionally applied afterText. That makes the comment misleading; suggest clarifying that only line-metric-affecting properties (font/alignment/character spacing/line height) must run beforeText.
// WinUI must apply text formatting before Text. Otherwise, a TextBlock
// connected without text retains different line metrics when text is added later.
Clear the TextBlock LineHeight dependency property when Label.LineHeight returns to its default value, and cover the transition with a Windows device test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
Head |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
Head |
|
@PureWeen current head Exact-head evidence:
The remaining red legs are platform-isolated from this Windows-only diff: the two known Android long-Entry failures fixed by #37052, an iOS |
<!-- 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! ### Description of Change Bundles independently valid Preview 7 test and test-infrastructure stabilizations into one merge path. Changes are cherry-picked or extracted from their source PRs without modifying MAUI runtime or product behavior. The aggregate currently covers: - exact DragEvents label polling and shared Appium text-wait behavior; - waiting for the iOS Entry keyboard before its snapshot; - adaptive ListView and CollectionView screenshot stabilization; - RefreshView gesture retry and SwipeView callback-result waiting; - stable initial SafeArea layout measurement; - deterministic iOS Picker-to-Entry keyboard transition handling; - a bounded Android screenshot tolerance for stable post-rotation antialiasing variance; - independent iOS and MacCatalyst Graphics image-scaling device coverage; - a bounded UIKit cleanup wait for the CarouselView leak assertion; - Android-only scoping for the Android modal-animation leak regression test; - an exact detached-page transition with a synchronous alert-request return signal; and - resilient Windows test-machine resolution setup, with focused Pester coverage for each fallback and failure path. Eligible UI tests, device tests, unit tests, test infrastructure, and screenshot baselines can all be included here when they are independently valid without a corresponding product-code change. ### Source PRs The standalone source PRs below are closed in favor of this aggregate. For #37057, only the independently valid test subset is included; its product-code rewrite remains excluded. | PR | Included test-side fix | |---|---| | #37044 | Windows screen-resolution test infrastructure and Pester coverage | | #37045 | DragEvents exact-text waits and Appium helper | | #37053 | iOS Entry keyboard readiness | | #37055 | ListView screenshot retry window | | #37057 | Graphics image-scaling device tests that pass without the runtime rewrite | | #37058 | RefreshView pull-to-refresh retry | | #37059 | SwipeView result-label wait | | #37066 | SafeArea initial-layout retry | | #37069 | Picker keyboard transition stabilization | | #37074 | Empty CollectionView footer screenshot retry/tolerance | | #37086 | iOS/MacCatalyst CarouselView leak-test cleanup wait | | #37088 | Detached-page alert request synchronization | | #37091 | Android modal-animation leak-test platform scope | | #37092 | Android Issue22306 post-rotation screenshot tolerance | ### Scope exclusions Mixed runtime/test fixes are included only when their test-side changes pass independently against the unmodified product code. The current device tests in #37052, the four nonpositive-size cases in #37057, and the device test plus screenshots in #37062 remain excluded because they expose or describe behavior that requires those PRs' functional fixes. #37070 has no test-side changes. Closed ineffective or unsafe fixes are also excluded. ### Validation - All 25 directly reusable source commits were cherry-picked in their original order. - The #37057 device-test subset was extracted into one additional test-only commit after proving it against the unmodified product implementation. - Every directly cherry-picked aggregate file matches the corresponding included source PR head. - The aggregate diff contains no MAUI runtime or product files. - Targeted Release builds pass for `UITest.Appium`, `Controls.TestCases.iOS.Tests`, `Controls.TestCases.Mac.Tests`, and `Controls.TestCases.Android.Tests`. - The focused screen-resolution Pester suite passes 14/14. - Exact local xUnit XML reports 46/46 Graphics device tests passing on both iOS 26 and MacCatalyst, including all 13 extracted cases. - The Graphics deadlock regression now asserts the bounded five-second completion result before awaiting the scaling task. - The CarouselView category passes 6 consecutive MacCatalyst runs at 4/4 each and passes 4/4 on iOS 26 with the final 100 ms cleanup wait. - The Android modal-animation leak test passes in both discovered Android variants, while the iOS device-test assembly excludes that Android-specific regression test. - The Issue22306 Android failure's three retries were byte-for-byte identical at a 1.85% visual difference, below the new Android-only 2% tolerance. - The unloaded-page alert probe now signals immediately after `DisplayAlertAsync` returns, avoiding a teardown-order dependency on the detached page task. - Rebased the aggregate onto release head `dee83edd121`; the resulting diff remains limited to tests and test infrastructure. - The Pester workflow now triggers when `eng/scripts/Set-ScreenResolution.ps1` changes. - A standalone `/azp run` was posted for aggregate head `83da465424b`; exact merge `bad9e4e9a57` builds `1539860`, `1539861`, and `1539862` are required before merge. ### Issues Fixed Contributes to stabilizing the .NET 11 Preview 7 test branch. --------- Co-authored-by: Vally Fixture <vally-fixture@example.invalid> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: kubaflo <34349119+kubaflo@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16 Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
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 ordered Label feature tests set
LineHeight=3, then later option navigations replace the view model with the defaultLineHeight=-1. WinUI'sUpdateLineHeightassigned a localTextBlock.LineHeightfor positive values but ignored the default negative value, so the reused nativeTextBlockretained the old three-line-height setting. Recreating the page produced a freshTextBlockwith the correct default line height, causing 17 deterministic before/after screenshot mismatches.This change:
TextBlock.LineHeightPropertywhenLabel.LineHeightreturns to its default negative value, restoring style/native-default precedence;3to-1transition;1538857.Validation
1538707disproved the mapper-order experiment: Android and iOS Label suites passed, while WinUI failed the same 17 post-LineHeight tests before and after retry.5a702384e1passed main build1538856; Android, iOS, and MacCatalyst Label runs had no failed individual results in UI build1538857; and Windows device run42375462passedLineHeight Resets To Defaulttwice. The device pipeline was red only for two unrelated Android Entry tests already covered by [release/11.0.1xx-preview7] Preserve Android Entry text during handler reuse #37052.42376752proved the original 17 failures fixed and left seven deterministic visual shifts. Every remaining actual capture was byte-identical across four retries, and each replacement now compares at zero RMS against that exact capture.cd9f74f764requires fresh exact-head/azp runproof for all WinUI Label tests and the Windows device regression test.Issues Fixed
Contributes to stabilizing the .NET 11 Preview 7 UI-test branch.