[Testing] Refactoring Feature Matrix UITest Cases for Shapes Control - #34637
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34637Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34637" |
dotnet-policy-service
Bot
added
community ✨
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Refactors the Shapes Feature Matrix host UI + UITests to support additional shape properties and a new RoundRectangle shape, updating screenshot-based baselines accordingly.
Changes:
- Extended the Shapes host page/options UI and
ShapesViewModelto support new properties (e.g.,StrokeLineCap,StrokeLineJoin,FillRule,Aspect,CornerRadius) and introducedRoundRectangle. - Refactored options UI to reset state via
ResetToDefaults()for more deterministic screenshot tests. - Expanded/renamed screenshot UITest cases and updated snapshot baselines across platforms.
Reviewed changes
Copilot reviewed 7 out of 191 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/ShapesFeatureTests.cs | Refactors and expands screenshot-based tests; renames tests and adds many new cases including RoundRectangle and new property scenarios. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/Shapes/ShapesViewModel.cs | Adds new bindable properties and a ResetToDefaults() method; refactors point parsing and improves PathGeometry parse failure logging. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/Shapes/ShapesControlPage.xaml | Updates shape rendering bindings and adds the RoundRectangle shape with new property bindings. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/Shapes/ShapesControlPage.xaml.cs | Changes Options navigation to reset the existing view model instead of recreating it. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/Shapes/ShapesOptionsPage.xaml | Refactors the options UI layout, adds new property controls (Aspect/LineCap/LineJoin/FillRule), and introduces RoundRectangle options. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/Shapes/ShapesOptionsPage.xaml.cs | Adds handlers for new property radio groups and refactors property-section visibility logic. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/Shapes/ShapeConverters.cs | Removed unused converters (logic moved into the view model/bindings). |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/Shapes/EnumToBoolConverter.cs | Minor formatting-only change. |
| src/Controls/tests/TestCases.*.Tests/snapshots/** | Updates/adds screenshot baselines for the refactored shapes tests. |
MauiBot
added
s/agent-review-incomplete
s/agent-reviewed
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 5 findings
See inline comments for details.
| [Category(UITestCategories.Shape)] | ||
| public void PolyLine_DashArray_DashOffset_Thickness() | ||
| [Test, Order(9)] | ||
| public void Polyline_DashArray_DashOffset_Thickness() |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Regression Prevention — VerifyShapeScreenshot() derives the snapshot name from the NUnit method name (UITest.cs:401), so this renamed test now looks for Polyline_DashArray_DashOffset_Thickness.png. The PR still leaves the Android, iOS, and iOS-26 baselines under the old PolyLine_DashArray_DashOffset_Thickness.png casing, while only Mac/Windows have the new lowercase-l filename. On case-sensitive checkouts those platforms will report a missing baseline instead of using the updated image. Rename/add the remaining baselines to the Polyline_* filename (or pass an explicit legacy snapshot name).
| [Category(UITestCategories.Shape)] | ||
| public void PolyLine_Points_Thickness() | ||
| [Test, Order(14)] | ||
| public void Polyline_Points_Thickness() |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Regression Prevention — Same baseline-name mismatch for Polyline_Points_Thickness: the method now looks for Polyline_Points_Thickness.png, but Android, iOS, and iOS-26 still only have PolyLine_Points_Thickness.png. This is especially risky because the test also changed the entered point data, so the modified baseline must be reachable under the new method-derived name. Rename/add the missing Polyline_Points_Thickness.png baselines for those platforms.
| public void PolyLine_StrokeColor_Thickness() | ||
|
|
||
| [Test, Order(22)] | ||
| public void Polyline_StrokeColor_Thickness() |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Regression Prevention — The iOS-26 baselines were not renamed for the Polyline_StrokeColor_Thickness and Polyline_StrokeColor_DashArray_Thickness methods. The ios-26/ folder still contains PolyLine_StrokeColor_Thickness.png and PolyLine_StrokeColor_DashArray_Thickness.png, while the tests derive the lowercase-l Polyline_* names. iOS 26 snapshot runs will miss these baselines unless the files are renamed or explicit snapshot names are used.
| StrokeDashOffset="{Binding StrokeDashOffset}" | ||
| StrokeLineCap="{Binding StrokeLineCap}" | ||
| StrokeLineJoin="{Binding StrokeLineJoin}" | ||
| FillRule="{Binding FillRule}" |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Logic and Correctness — PolylineShape binds FillRule but does not bind Fill to FillColor, unlike PolygonShape above it. The new Polyline_FillRule_Nonzero test taps FillColorBlueRadioButton, but that value never reaches the Polyline, and FillRule has no visible effect without a fill. Add Fill="{Binding FillColor}" to the Polyline (and update baselines) so the test verifies the property it configures.
| Y1 = 0; | ||
| X2 = 280; | ||
| Y2 = 0; | ||
| Points = null; |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Logic and Correctness — ResetToDefaults() clears Points, PolylinePoints, and PathData to null even though the view model initializes them with non-empty default geometry. Because the Options button now calls this reset before every visit, selecting Polygon/Polyline/Path and applying without re-entering geometry renders an empty point collection or the fallback path instead of the default sample shape. Reset these properties to the same default strings (or remove/document the field defaults) so reset is actually deterministic.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
removed
the
s/agent-review-in-progress
When the HostApp installs but its first page never renders, every test in a fixture fails at OneTimeSetup with 'Timed out waiting for Go To Test button to appear (the app did not recover after crash-recovery attempts)' (from UtilExtensions.NavigateToGallery -> WaitForGoToTestButtonWithRecovery). The per-category deep loop only retries when the captured output matches an environment-error pattern; this app-render signature was NOT in the list, so the category was treated as a 'real test failure' and returned all-setup-failed after a SINGLE attempt -> the user sees 'N marked failed (setup failed)' (e.g. #35640 Material3 0/338, #34637 Shape 0/61, #30875). This is an intermittent infra flake, not a code failure: the SAME HostApp head passes on a different agent (#36575 IndicatorView 41/41). The test's own crash-recovery only force-stops+relaunches the app; a pipeline retry additionally 'adb reboot's and rebuilds/reinstalls the app fresh, clearing the stuck emulator state. Add the two app-render signatures to the shared env-error pattern list so these categories retry (up to MaxAttempts) instead of reporting fake failures. Verified the signature is now retryable while a normal assertion failure ('Expected: True But was: False') is not. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 3 findings
See inline comments for details.
| Y1 = 0; | ||
| X2 = 280; | ||
| Y2 = 0; | ||
| Points = null; |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Logic and Correctness Verification — ResetToDefaults clears the constructor's non-empty polygon, polyline, and path geometries. Unlike the previous fresh ShapesViewModel, opening Options and selecting Polygon, Polyline, or Path now renders nothing unless the user re-enters geometry. Restore the actual field defaults here instead of assigning null.
| [Category(UITestCategories.Shape)] | ||
| public void PolyLine_DashArray_DashOffset_Thickness() | ||
| [Test, Order(9)] | ||
| public void Polyline_DashArray_DashOffset_Thickness() |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Regression Prevention and Test Coverage — Renaming this method changes the implicit baseline name used by VerifyScreenshot() (TestContext.CurrentContext.Test.MethodName), but Android and both iOS baseline directories still contain only PolyLine_DashArray_DashOffset_Thickness.png. The same mismatch affects Polyline_Points_Thickness at line 460. Those platform runs will fail baseline lookup; the PR renames the corresponding files only for Mac and WinUI.
| App.WaitForElement("PathDataEntry"); | ||
| App.Tap("PathDataEntry"); | ||
| App.ClearText("PathDataEntry"); | ||
| App.EnterText("PathDataEntry", "M 10,84 C 10,84 40,15 100,55 C 160,15 190,84 190,84 C 190,84 100,135 100,135 C 100,135 10,84 10,84 Z"); |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Regression Prevention and Test Coverage — This cap test supplies a path ending in Z, so the figure is closed and has no endpoints on which StrokeLineCap can render. Consequently the added Round and Square baselines are pixel-identical on every platform, so an ignored or broken Path.StrokeLineCap still passes. Use an open path here and in Path_StrokeLineCap_Square.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@HarishKumarSF4517 — new AI review results are available based on this last commit:
36ee5eb. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate: Test Verification (Failure-Only Mode)
Result:
This is a test-only change (no fix files detected in the diff), so the gate only verifies that the new/changed tests fail against the merge base — proving they reproduce the bug they target.
Platform: ANDROID
Merge base: 0395a53b
| Test | Type | Outcome |
|---|---|---|
ShapesFeatureTests |
UITest |
Diagnostics
- ShapesFeatureTests:
New snapshot test(s) — baseline image not yet created for 2 test(s); the gate cannot validate brand-new VerifyScreenshot tests (baseline PNGs are added separately by a maintainer)
📱 UI Tests — Shape
Detected UI test categories: Shape
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Shape |
0/61 (setup failed; 61 marked failed) | — |
⚠️ Shape — fixture setup failed for 61 tests
NUnit reported a OneTimeSetUp/fixture setup failure before test bodies ran; the TRX marked each affected test failed.
Multiple setup failure signatures were present; showing the first one. See the TRX artifact for all details.
OneTimeSetUp: System.TimeoutException : Timed out waiting for Go To Test button to appear (the app did not recover after crash-recovery attempts)
at Microsoft.Maui.TestCases.Tests.UtilExtensions.WaitForGoToTestButtonWithRecovery(IApp app, String timeoutMessage) in /_/src/Controls/tests/TestCases.Shared.Tests/UtilExtensions.cs:line 91
at Microsoft.Maui.TestCases.Tests.UtilExtensions.NavigateToGallery(IApp app, String page) in /_/src/Controls/tests/TestCases.Shared.Tests/UtilExtensions.cs:line 37
at Microsoft.Maui.TestCases.Tests._GalleryUITest.FixtureSetup() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/_GalleryUITest.cs:line 57
at UITest.Appium.NUnit.UITestBase.OneTimeSetup() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 221
at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)
📋 Pre-Flight — Context & Validation
Issue: #34637 - [Testing] Refactoring Feature Matrix UITest Cases for Shapes Control
PR: #34637 - [Testing] Refactoring Feature Matrix UITest Cases for Shapes Control
Platforms Affected: Android, iOS, MacCatalyst, Windows UI tests; requested candidate testing platform: Android
Files Changed: 7 implementation/host-app files, 261 test/snapshot files
Key Findings
- The PR is a Shapes feature-matrix UI-test refactor: it adds RoundRectangle coverage, expands shape option controls, updates screenshot tests, and refreshes many platform baselines.
- The PR has no linked product bug found from the PR body; it is test coverage/refactoring work.
- Prior inline review comments identify unresolved screenshot baseline filename mismatches where
Polyline_*test method names still map to legacyPolyLine_*snapshot files on some platforms. ShapesViewModel.ResetToDefaults()currently clearsPoints,PolylinePoints, andPathDatatonull, diverging from the non-empty default geometry used by the initial page state.- Android is impacted because
Polyline_DashArray_DashOffset_ThicknessandPolyline_Points_Thicknessdo not have exact Android baseline filenames in the current tree, while legacyPolyLine_*files exist.
Code Review Summary
Verdict: NEEDS_CHANGES
Confidence: low
Errors: 1 | Warnings: 1 | Suggestions: 1
Key code review findings:
- ✗
ShapesFeatureTests.cs:280,:460,:675,:865— renamedPolyline_*screenshot tests can miss existing legacyPolyLine_*baselines on case-sensitive lookup. - ⚠
ShapesViewModel.cs:54-56— reset clears geometry strings instead of restoring the backing-field defaults. - ℹ
ShapesFeatureTests.cs:1447,:1478— Path line-cap tests use a closed path, which weakly exercises endpoint cap behavior.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #34637 | Large Shapes feature-matrix refactor with many snapshot additions/renames and expanded host-page options | 268 files | Original PR; local gh auth unavailable and gate already reported environment/build blocker |
🔬 Code Review — Deep Analysis
Code Review — PR #34637
Independent Assessment
What this changes: Refactors the Shapes feature-matrix host page/tests: adds RoundRectangle, new shape properties, many screenshot tests, and refreshed baselines.
Inferred motivation: Broaden UI coverage for Shapes and make each scenario start from a deterministic state.
Reconciliation with PR Narrative
Author claims: Adds shape-property coverage, RoundRectangle support, reset defaults, and screenshot baseline updates.
Agreement/disagreement: The code matches the stated direction, but some renamed tests still do not have matching platform baselines, and reset defaults do not restore the declared geometry defaults.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
[major] renamed Polyline_* tests missing matching Android/iOS/iOS-26 baselines |
MauiBot inline comments | ❌ Unresolved | Current tree still has PolyLine_DashArray_DashOffset_Thickness.png / PolyLine_Points_Thickness.png for Android, iOS, iOS-26, while tests at ShapesFeatureTests.cs:280 and :460 call VerifyScreenshot() under Polyline_* method names. |
[major] iOS-26 Polyline_StrokeColor_* baseline filename mismatch |
MauiBot inline comments | ❌ Unresolved | ios-26 still contains PolyLine_StrokeColor_Thickness.png and PolyLine_StrokeColor_DashArray_Thickness.png, but tests are Polyline_StrokeColor_Thickness (:675) and Polyline_StrokeColor_DashArray_Thickness (:865). |
[moderate] ResetToDefaults() clears default Polygon/Polyline/Path geometry |
MauiBot inline comments | ❌ Unresolved | ShapesViewModel.cs:54-56 still assigns Points, PolylinePoints, and PathData to null. |
Blast Radius Assessment
- Runs for all instances: No; limited to Shapes feature-matrix test host page.
- Startup impact: No app-wide startup impact.
- Static/shared state: No new static/shared state.
CI Status
- Required-check result:
gh pr checks --requiredunavailable becauseghis unauthenticated. Gate result supplied to this run is inconclusive due build/environment. - Classification: Undetermined by local environment; prior review evidence indicates PR-caused UI screenshot lookup failures are likely.
- Action taken: Confidence capped low because required-check status and Android device execution were unavailable.
Findings
❌ Error — Renamed Polyline screenshot tests do not have matching baselines
ShapesFeatureTests.cs:280, :460, :675, and :865 rename tests to Polyline_*, and VerifyShapeScreenshot() uses the test method name as the baseline name. However the current snapshot tree still has legacy PolyLine_* files for:
- Android/iOS/iOS-26:
DashArray_DashOffset_Thickness,Points_Thickness - iOS-26:
StrokeColor_Thickness,StrokeColor_DashArray_Thickness
These platform runs will look for missing Polyline_*.png baselines and fail instead of validating screenshots.
⚠️ Warning — ResetToDefaults does not restore geometry defaults
ShapesViewModel.cs:54-56 sets Points, PolylinePoints, and PathData to null, despite non-empty backing-field defaults. After opening Options, selecting Polygon/Polyline/Path without re-entering geometry renders empty point collections or the fallback path, not the default sample shape. Restore the default strings instead.
💡 Suggestion — Path line-cap tests use a closed path
ShapesFeatureTests.cs:1447 and :1478 use a path ending in Z. Closed figures do not exercise open endpoints, so these tests are weak coverage for StrokeLineCap. Consider using an open path for Round/Square cap scenarios.
Failure-Mode Probing
- Selecting Polygon/Polyline/Path without editing geometry after reset: renders empty/fallback geometry because reset clears strings.
- Running renamed Polyline screenshot tests on Android/iOS/iOS-26: baseline lookup uses
Polyline_*, but files remainPolyLine_*, so visual tests fail before meaningful comparison. - Handler/page lifecycle: no subscriptions/static state added; no accumulation issue found.
Verdict: NEEDS_CHANGES
Confidence: low
Summary: The PR still has concrete test-breaking baseline filename mismatches on multiple platforms, corroborated by prior unresolved [major] findings and red/inconclusive UI checks. Fix the baseline names, pass stable explicit screenshot names, or otherwise make baseline lookup deterministic before merge.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Add scoped explicit screenshot-name support for legacy PolyLine_* baselines, restore geometry defaults, and use open Path cap geometry |
2 code files + 2 Path cap baselines if recaptured | Best scoped alternative; avoids binary churn but preserves naming inconsistency | |
| 2 | try-fix | Canonicalize all remaining PolyLine_*.png snapshots to Polyline_*.png, restore geometry defaults, and use open Path cap geometry |
PolyLine_* stragglers and 2 Android exact-baseline gaps |
Multiple snapshot files + 2 code files | Clean naming but high binary churn and case-only rename risk |
| 3 | try-fix | Restore legacy PolyLine_* test method names and convert Path cap screenshots to behavior-only checks |
❌ FAIL (static expert review) | 1 test file | Reduces visual coverage and would introduce non-canonical category usage |
| 4 | try-fix | Add global case-insensitive screenshot baseline fallback using search pattern | ❌ FAIL (static expert review) | 1 infrastructure file | Search-pattern lookup can remain case-sensitive on Linux, so it may not find case-different files |
| 5 | try-fix | Corrected global case-insensitive screenshot baseline fallback by enumerating all files and comparing names ordinal-ignore-case | 1 infrastructure file | Viable fallback but changes global visual-regression behavior | |
| PR | PR #34637 | Large Shapes feature-matrix refactor with many snapshot additions/renames and expanded host-page options | 268 files | Original PR; prior review indicates unresolved baseline-name and reset-default issues |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| gpt-5.5 / maui-expert-reviewer | 1 | Yes | Scoped screenshot-name aliases plus reset-default constants and open Path cap geometry |
| claude-opus-4.7 / maui-expert-reviewer | 1 | Yes | Complete canonical snapshot rename approach with single-source defaults |
| gpt-5.3-codex / maui-expert-reviewer | 1 | Yes | Restore legacy method names and split unstable Path cap visuals into behavior-only checks |
| gpt-5.5 / maui-expert-reviewer | 2 | Yes | Global visual-regression case-insensitive baseline fallback |
| gpt-5.5 / maui-expert-reviewer | 2 | Yes | Corrected fallback that avoids Linux search-pattern case-sensitivity |
Lessons from Failed / Blocked Attempts
- The unresolved Android issue is mechanically reproducible by filename inspection:
Polyline_DashArray_DashOffset_Thickness.pngandPolyline_Points_Thickness.pngare requested by method names, but only legacyPolyLine_*Android files exist. - Device/gate validation could not be rerun because the supplied gate was already inconclusive and the instructions explicitly prohibited rerunning gate verification.
- Candidate 3 was rejected because it weakens visual coverage and conflicts with UI-test category conventions.
- Candidate 4 was rejected because its file-enumeration pattern could remain case-sensitive on Linux and miss the intended fallback.
- Candidate 5 fixes candidate 4 but broadens behavior globally in
VisualRegressionTester, which is riskier than a local Shapes test fix.
Exhausted: Yes. The remaining alternatives are variations of three meaningful strategies: local screenshot-name mapping, canonical snapshot renaming, or infrastructure-level case-insensitive lookup. Further attempts would be trivial variations of those.
Selected Fix: Candidate #1 — It is the best alternative candidate for Android because it is the narrowest code-only change that addresses the current Android baseline lookup gap while preserving screenshot coverage. It is not a full verified pass because Android UI execution was blocked, but it is demonstrably safer than the global infrastructure fallback and lower churn than the canonical snapshot rename approach.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current description is detailed, but the winning fix adds reviewer-driven corrections for legacy PolyLine baselines, reset geometry defaults, and Path line-cap test validity that the metadata does not capture.
Recommended title
[Testing] Shapes Feature Matrix: Refactor Shapes UI test coverage
Recommended description
### Description of Change
Refactors and expands the Shapes feature-matrix UI tests and host app coverage.
Key changes:
- Adds `RoundRectangle` support to the Shapes feature matrix, including host-app selection, bindings, property visibility, and screenshot coverage.
- Adds bindings and option controls for additional shape properties: `StrokeLineCap`, `StrokeLineJoin`, `FillRule`, `Aspect`, and `CornerRadius`.
- Updates `ShapesViewModel` defaults and reset behavior so each test starts from a known state before navigating to the options page.
- Preserves default Polygon, Polyline, and Path geometry when resetting, instead of clearing geometry values.
- Adds scoped legacy `PolyLine_*` screenshot names for the renamed Polyline tests on Android/iOS where the existing baselines still use legacy casing.
- Uses open Path geometry for `Path_StrokeLineCap_Round` and `Path_StrokeLineCap_Square` so the screenshots actually validate endpoint cap rendering.
- Refreshes Shapes screenshot baselines across the affected test platforms.
- Removes the unused `ShapeConverters.cs` file after the bindings/conversions moved into the view model and XAML.
### Testing Notes
- Target platform for this review: Android.
- Gate result was inconclusive because the tests could not be built/run in the review environment.
### Issues Fixed
None linked; this is Shapes feature-matrix UI-test refactoring and coverage work.
🏁 Report — Final Recommendation
Comparative Report — PR #34637
Candidates compared
| Rank | Candidate | Regression status | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Winner. Keeps the PR's Shapes feature-matrix refactor and applies the expert-review fixes: scoped Android/iOS legacy PolyLine_* screenshot aliases for the two renamed tests, restored geometry defaults in ResetToDefaults(), and open Path geometry for line-cap screenshots. |
|
| 2 | try-fix-1 |
Very close to the winner and effectively the best independent alternative. It has the same scoped strategy, but it is an alternative patch rather than the PR fix plus reviewer feedback. | |
| 3 | try-fix-2 |
Correct and clean from a naming perspective, but it requires broader binary snapshot churn and case-only renames across platforms. This is riskier than the scoped PR-plus fix for the same known Android/iOS gaps. | |
| 4 | try-fix-5 |
The corrected global case-insensitive baseline fallback is viable, but it changes visual-regression infrastructure globally and can mask future filename casing mistakes. It is broader than needed for this PR. | |
| 5 | pr |
The raw PR contains valuable coverage work but leaves baseline-name, reset-default, and Path cap test-validity issues unresolved. | |
| 6 | try-fix-3 |
❌ Failed static expert review | Ranked below all non-failing candidates. It weakens visual coverage by converting Path cap screenshots to behavior-only checks and introduced non-canonical category risk. |
| 7 | try-fix-4 |
❌ Failed static expert review | Ranked below all non-failing candidates. Its Directory.EnumerateFiles(..., imageFileName, ...) fallback can remain case-sensitive on Linux and miss the case-different baseline. |
Why pr-plus-reviewer wins
pr-plus-reviewer is the single best candidate because it preserves the PR's intended feature-matrix expansion while correcting all actionable expert-review findings with localized changes. It avoids the failed approaches (try-fix-3, try-fix-4), avoids the global infrastructure blast radius of try-fix-5, and avoids the binary snapshot churn/case-only rename risk of try-fix-2.
The only stronger alternative is try-fix-1, but pr-plus-reviewer incorporates the same scoped fixes as reviewer feedback on top of the submitted PR, making it the best merge-path candidate. The gate remains inconclusive due to environment/build issues, so the choice is based on static correctness and candidate risk rather than a verified Android run.
Winning candidate
Winner: pr-plus-reviewer
Rationale: It fixes the raw PR's static review issues without broadening the change beyond the Shapes tests and host page. No candidate has a verified passing Android regression run, but the candidates that failed static/regression review are ranked lower as required.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
|
Some shape tests failed on android |
This comment has been minimized.
This comment has been minimized.
Tests Failure Analysis
Test Failure Review: Not ready - click to expandOverall verdict: Not ready. 77 distinct UI-test failures are red on this PR but green across all 5 recent
Coverage: 162 checks · 152 passing · 10 failing · 0 pending · 0 inaccessible · 1 unmapped · 16 unexplained build legs · 0 unaccounted failing checks · 2 aborted failing checks · 0 canceled-build checks · 7 device-test unverified · 16 unattributed · 77 regressed-vs-base. Deterministic ceiling: Not ready — 77 legs regressed vs base plus unexplained/aborted/unverified legs. Builds (this PR): maui-pr-uitests 1506616, maui-pr-devicetests 1506617. Base sampling (main, 5 recent builds per definition): uitests 1503618, 1503332, 1503036; devicetests 1503431. Recommended actionInvestigate the 77 regressed-vs-base visual/UITest failures — they are new on this PR and consistent with the Shapes feature-matrix refactoring; regenerate/verify the affected snapshot baselines and confirm the refactored test cases before merge. The 2 |
Tests Failure Analysis
Test Failure Review: Not ready - click to expandOverall verdict: Not ready CI is not ready to merge: the deterministic gate found 78 failures/legs red on this PR while green on the sampled base builds, including a MacCatalyst device-test build error and many WinUI visual/UI failures. Only 2 of 173 distinct failures also appear on the base branch, and 95 failures remain unattributed or otherwise require human review. Coverage: 162 checks · 152 passing · 10 failing · 0 pending · 0 inaccessible · 1 unmapped · 17 unexplained build legs · 0 unaccounted failing checks · 2 aborted failing checks · 0 canceled-build checks · 7 device-test unverified · 95 unattributed · 78 regressed-vs-base · 1 demoted by ci-scan. Deterministic ceiling: Not ready — reasons: Build Analysis has no inspectable AzDO build evidence; 17 failed build legs produced no extractable failure; 95 failures are unattributed; 2 UI-test checks were cancelled; 7 green device-test checks could not be confirmed
Recommended actionDo not merge this CI run. Start with the 78 Evidence detailsPR scope: 268 changed test files, centered on the Shapes feature matrix page/tests and platform snapshot baselines under Failing checks: Build evidence: device-test build 1506617 failed with 4 failed timeline records and one failed MacCatalyst Helix work item; UI-test build 1506616 failed with 30 failed timeline records and 191 extracted log/test failures. Baseline evidence: device-test base builds sampled were 1518287, 1503431, 1503150, 1503037, and 1499502; UI-test base builds sampled were 1503618, 1503332, 1503036, 1499834, and 1499259. The UI-test baseline row is explicitly incomplete because only the first 8 of 11 failed build logs were inspected. Distinct PR failures also on base: 2 of 173. Known issue evidence: Known Build Error matching was queried successfully with 1 matcher loaded, but no distinct failure reached Visual evidence limitation: 92 visual comparisons were detected, but visual result discovery for build 1506616 inspected only the first 100 of 156 failed test results. Individual visual panels and image links are intentionally omitted here; the trusted local merger owns those assets. Visual failure comparisonsFull-resolution CI baseline, actual, and diff images are embedded below. They supplement the failure classification and do not change the deterministic verdict ceiling.
|
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
ShellPages_PresentationModeAnimated - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
ChangeShellContentTitle - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
SubPageNavigationShouldWorkAfterFirstTabBecomesInvisible - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
TabBarDisabledColorAppliedToDisabledTab - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
TabBarVisibilityHidesOnPage2UsingBinding - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
NavigationBarShouldRemainHiddenAfterNavigatingBack - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
ShellRTLFlowDirectionShouldNotCauseOverlap - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x592 pixels, actual is 1920x1080 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
ShouldHideHeaderWhenTitleEmpty - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1506616.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
 |
 |
 |
Snapshots: - Delete wrong-named Line_FillColorWithStrokeColor_Shadow.png and PolyLine_FillColorWithStrokeColor_Shadow.png (tests are named Line_StrokeColor_Shadow / Polyline_StrokeColor_Shadow) - Rename PolyLine_* → Polyline_* on all platforms to match test method names (Android, iOS, ios-26, Mac, Windows) ShapesViewModel: - Extract duplicate point-parsing into ParsePointCollection() helper - Use CultureInfo.InvariantCulture in double.TryParse to avoid locale-dependent parsing of coordinate strings - Replace bare catch with catch (Exception ex) and log via Debug.WriteLine - Add System.Globalization using directive ShapesOptionsPage: - Remove redundant FillColor = null in OnFillColorChanged; HasFillColor setter already clears FillColor when set to false - Make GetColorByName static; use fully-qualified Microsoft.Maui.Graphics.Colors consistently for all color cases (was using bare Colors.Red for 'red') ShapesControlPage: - Add comment explaining why ResetToDefaults() is called before navigation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
36ee5eb to
feabe58
Compare
<!-- 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 Automates visual snapshot evidence in `/review tests`. When public AzDO results contain failed screenshot comparisons, the command now emits exactly one test-failure analysis comment containing bounded, expandable baseline/actual/diff panels. Visual evidence remains supplementary: it does not change `gate.verdictCeiling`, deterministic attribution, or the merge-readiness verdict. #### One-comment flow 1. Trusted pre-activation code discovers failed visual results through the public AzDO `resultsbybuild` API, including retry-suffixed attachments such as `Snapshot[1].png` and `Snapshot-diff[1].png`. 2. It resolves baselines from the exact source version tested by AzDO and maps runtime evidence to the correct snapshot directory (`ios-26`, `android-notch-36`, `mac`, or `windows`). 3. It streams and validates bounded PNG assets, then stores them on `review-tests-assets` using immutable commit-pinned `raw.githubusercontent.com` URLs. 4. The Copilot agent emits the normal single `add_comment` analysis payload with a trusted insertion marker. 5. A sealed post-step validates the published asset manifest and injects as many expandable comparison panels as fit into that same comment. Each collapsed panel shows a conservative relationship label: - `Likely PR-caused` for an exact test/platform base regression or directly changed snapshot/test; - `Likely unrelated` for an exact base/known-issue match without direct visual scope; - `Needs human investigation` for unmatched or mixed evidence. 6. Excess comparisons are summarized as omitted instead of creating a companion comment. The local `.github/scripts/Review-Tests.ps1 -PostComment` path uses the same merger. It also recognizes complete reports returned in Copilot's final response, preserving nested evidence code fences without wrapping a second title or badge section. ### Security and Failure Safety - PR text, logs, test names, attachment metadata, changed files, and visual labels remain untrusted input. - The merger script and visual context are copied to a root-owned location before the workflow checks out the untrusted PR branch. - The post-step runs without `COPILOT_GITHUB_TOKEN`, `GH_TOKEN`, or `GITHUB_TOKEN`. - AzDO attachment URLs must match the expected public project and attachment route. - Published assets are size-bounded, signature-checked PNGs with validated dimensions and repository paths. - Raw image URLs must match the exact repository, asset commit, PR directory, and safe filename. - Labels are HTML-escaped and `@` is neutralized before insertion. - Relationship labels use fixed trusted text. Untrusted attribution values are never rendered, and same-named snapshots changed on another platform do not count as PR scope. - The final body is checked panel-by-panel against conservative limits of 45 URLs, 10 mentions, and 60,000 UTF-16 characters, below gh-aw's throwing limits. - The analysis JSON update is atomic (written to a temp file, then renamed over the original). Invalid context, malformed output, missing analysis payloads, limit failures, and dry-run/noop output leave the original analysis unchanged. - The publisher never creates or patches PR comments; only the existing gh-aw `add_comment` payload is mutated. ### What NOT to Do - Do not use the ordinary anonymous AzDO test-runs listing for discovery; it redirects to sign-in. Use the public failed-results endpoint. - Do not resolve baselines from the current PR head; use the source version actually tested by the selected AzDO build. - Do not let the agent construct or trust visual asset URLs. - Do not publish a second companion comment; merge bounded panels into the single analysis payload. ### Validation - 98 focused Pester tests pass. - Changed PowerShell scripts parse successfully. - `gh aw compile copilot-review-tests --approve` completes without errors or warnings. - A real `agent_output.json` from gh-aw run [29674953402](https://github.com/dotnet/maui/actions/runs/29674953402) was replayed through the post-step: - one `add_comment` item remained one item; - five visual panels were inserted; - the final body contained 26 URLs, one mention, and 9,585 characters. ### Live Single-Comment Examples The exact local `/review tests` path from this branch posted or repaired these merged comments after the PRs' `/azp run` pipelines completed: | PR | Single merged result | Included evidence | Relationship labels | Final limits | | --- | --- | --- | --- | --- | | #36413 | [Test-failure analysis with visual panels](#36413 (comment)) | 5 panels / 15 images | 1 PR-caused, 4 investigate | 23 URLs, 13,156 chars | | #36631 | [Test-failure analysis with visual panels](#36631 (comment)) | 6 panels / 18 images | 2 PR-caused, 4 investigate | 31 URLs, 21,367 chars | | #36395 | [Test-failure analysis with visual panels](#36395 (comment)) | 11 panels / 33 images; 19 omitted | 11 investigate | 43 URLs, 19,858 chars | | #36404 | [Test-failure analysis with visual panels](#36404 (comment)) | 14 panels / 40 images; 81 omitted | 14 investigate | 45 URLs, 23,779 chars | | #35846 | [Test-failure analysis with visual panels](#35846 (comment)) | 10 panels / 30 images; 9 omitted | 10 investigate | 43 URLs, 22,496 chars | | #36277 | [Test-failure analysis with visual panels](#36277 (comment)) | 7 panels / 19 images | 3 PR-caused, 4 investigate | 31 URLs, 18,355 chars | | #36170 | [Test-failure analysis with visual panels](#36170 (comment)) | 11 panels / 33 images; 8 omitted | 11 investigate | 44 URLs, 23,180 chars | | #35578 | [Test-failure analysis with visual panels](#35578 (comment)) | 12 panels / 36 images; 50 omitted | 12 investigate | 44 URLs, 25,443 chars | | #36672 | [Test-failure analysis with visual panels](#36672 (comment)) | 14 panels / 40 images; 9 omitted | 14 investigate | 45 URLs, 25,915 chars | | #31755 | [Test-failure analysis with visual panels](#31755 (comment)) | 12 panels / 36 images; 3 omitted | 12 investigate | 44 URLs, 22,728 chars | | #34637 | [Test-failure analysis with visual panels](#34637 (comment)) | 9 panels / 27 images; 81 omitted | 9 investigate | 43 URLs, 22,325 chars | | #35156 | [Test-failure analysis with visual panels](#35156 (comment)) | 2 panels / 6 images | 1 PR-caused, 1 investigate | 19 URLs, 11,911 chars | | #35885 | [Test-failure analysis with no visual failures](#35885 (comment)) | 0 panels / 0 images | No visual failures detected | 8 URLs, 4,239 chars | | #36577 | [Test-failure analysis with visual panels](#36577 (comment)) | 1 panel / 3 images | 1 investigate | 44 URLs, 21,768 chars | | #36212 | [Test-failure analysis with no visual failures](#36212 (comment)) | 0 panels / 0 images | No visual failures detected | 5 URLs, 5,982 chars | Each result contains one `Tests Failure Analysis` title and one merged review marker. Across 114 rendered panels, all 336 embedded image URLs returned HTTP 200. Seven panels were safely classified as likely PR-caused; no panel in this sample had enough exact evidence to be safely classified as likely unrelated, so the remaining 107 stayed at `Needs human investigation`. Another 260 comparisons were omitted safely by the comment limits. The latest eight-example batch was regenerated concurrently, and #36672, #31755, #34637, #35156, #35885, #36577, and #36212 were added afterward. The current [`review-tests-assets` head](f937993) retains the full asset history. The protected `copilot-pat-pool` environment rejects feature-branch `workflow_dispatch` runs before job execution. The live local-runner examples validate comment generation and asset publication, while the real gh-aw output replay validates the workflow post-step mutation without weakening that branch protection. ### Issues Fixed N/A - reviewer workflow enhancement. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: kubaflo <kubaflo@users.noreply.github.com> Copilot-Session: a280b482-e102-4ca0-9ff9-1cfe1946e21f Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d Copilot-Session: 478d195b-20f3-4bc6-aeed-f6aa88b55fda
…34637) > [!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! This pull request significantly enhances the shapes feature matrix in the test host app by adding support for new shape properties, introducing the `RoundRectangle` shape, and improving the testability and flexibility of shape options. Key changes include new bindings for advanced shape properties, an updated view model with a reset-to-defaults method, and UI/UX improvements for shape selection and property editing. **Shape property and binding enhancements:** * Added support for `StrokeLineCap`, `StrokeLineJoin`, `FillRule`, `Aspect`, and `CornerRadius` properties in the `ShapesViewModel` and bound these to the corresponding shape controls in `ShapesControlPage.xaml`, allowing for more comprehensive shape customization and testing. [[1]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eR163-R197) [[2]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R33-R56) [[3]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R68-R69) [[4]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R86-R87) [[5]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R102-R107) [[6]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R120-R125) [[7]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R139-L127) * Updated the view model to provide sensible default values for all shape properties and added a `ResetToDefaults` method, ensuring each test starts from a known state. The navigation logic was updated to call this reset before opening the options page. [[1]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL25-R57) [[2]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL176-R226) [[3]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL191-R241) [[4]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL219-R269) [[5]](diffhunk://#diff-f8f1fb85f05f57f9920790d96c964ace1fd2f2d9e6383fa02b7045b75baed27bL26-R27) **New shape and UI improvements:** * Introduced support for the `RoundRectangle` shape, including view model support, XAML bindings, and property section visibility logic in the options page. [[1]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eR12) [[2]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R33-R56) [[3]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL28-R36) * Refactored the shape options page to simplify and generalize property section visibility, and added event handlers for new property radio button groups (aspect, line cap, line join, fill rule). [[1]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL28-R36) [[2]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL87-R105) **Code and test cleanup:** * Removed the unused `ShapeConverters.cs` file, as the shape property bindings and conversions are now handled directly in the view model and XAML. * Minor improvements and bug fixes, such as using the correct color namespace and ensuring fill color is properly unset. [[1]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dR1-R2) [[2]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL67) [[3]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL87-R105) These changes make the shapes feature matrix more robust, extensible, and easier to test for advanced shape scenarios. Existing Issue Identified: 1. #29812 --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…34637) > [!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! This pull request significantly enhances the shapes feature matrix in the test host app by adding support for new shape properties, introducing the `RoundRectangle` shape, and improving the testability and flexibility of shape options. Key changes include new bindings for advanced shape properties, an updated view model with a reset-to-defaults method, and UI/UX improvements for shape selection and property editing. **Shape property and binding enhancements:** * Added support for `StrokeLineCap`, `StrokeLineJoin`, `FillRule`, `Aspect`, and `CornerRadius` properties in the `ShapesViewModel` and bound these to the corresponding shape controls in `ShapesControlPage.xaml`, allowing for more comprehensive shape customization and testing. [[1]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eR163-R197) [[2]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R33-R56) [[3]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R68-R69) [[4]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R86-R87) [[5]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R102-R107) [[6]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R120-R125) [[7]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R139-L127) * Updated the view model to provide sensible default values for all shape properties and added a `ResetToDefaults` method, ensuring each test starts from a known state. The navigation logic was updated to call this reset before opening the options page. [[1]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL25-R57) [[2]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL176-R226) [[3]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL191-R241) [[4]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eL219-R269) [[5]](diffhunk://#diff-f8f1fb85f05f57f9920790d96c964ace1fd2f2d9e6383fa02b7045b75baed27bL26-R27) **New shape and UI improvements:** * Introduced support for the `RoundRectangle` shape, including view model support, XAML bindings, and property section visibility logic in the options page. [[1]](diffhunk://#diff-5106b6740b7b8d60b67ceab6b6e3405ae3bdcbba781b867eeaa77564ee1fb74eR12) [[2]](diffhunk://#diff-8349c983a5d8ff6576ae8eb2b60cb8e7bac2edaa643956f28699f834074293b1R33-R56) [[3]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL28-R36) * Refactored the shape options page to simplify and generalize property section visibility, and added event handlers for new property radio button groups (aspect, line cap, line join, fill rule). [[1]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL28-R36) [[2]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL87-R105) **Code and test cleanup:** * Removed the unused `ShapeConverters.cs` file, as the shape property bindings and conversions are now handled directly in the view model and XAML. * Minor improvements and bug fixes, such as using the correct color namespace and ensuring fill color is properly unset. [[1]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dR1-R2) [[2]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL67) [[3]](diffhunk://#diff-badc0c21e36f892d45eb33dd7a3a8b75a5323cb5e028014ab925864c96cb816dL87-R105) These changes make the shapes feature matrix more robust, extensible, and easier to test for advanced shape scenarios. Existing Issue Identified: 1. #29812 --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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!
This pull request significantly enhances the shapes feature matrix in the test host app by adding support for new shape properties, introducing the
RoundRectangleshape, and improving the testability and flexibility of shape options. Key changes include new bindings for advanced shape properties, an updated view model with a reset-to-defaults method, and UI/UX improvements for shape selection and property editing.Shape property and binding enhancements:
Added support for
StrokeLineCap,StrokeLineJoin,FillRule,Aspect, andCornerRadiusproperties in theShapesViewModeland bound these to the corresponding shape controls inShapesControlPage.xaml, allowing for more comprehensive shape customization and testing. [1] [2] [3] [4] [5] [6] [7]Updated the view model to provide sensible default values for all shape properties and added a
ResetToDefaultsmethod, ensuring each test starts from a known state. The navigation logic was updated to call this reset before opening the options page. [1] [2] [3] [4] [5]New shape and UI improvements:
Introduced support for the
RoundRectangleshape, including view model support, XAML bindings, and property section visibility logic in the options page. [1] [2] [3]Refactored the shape options page to simplify and generalize property section visibility, and added event handlers for new property radio button groups (aspect, line cap, line join, fill rule). [1] [2]
Code and test cleanup:
Removed the unused
ShapeConverters.csfile, as the shape property bindings and conversions are now handled directly in the view model and XAML.Minor improvements and bug fixes, such as using the correct color namespace and ensuring fill color is properly unset. [1] [2] [3]
These changes make the shapes feature matrix more robust, extensible, and easier to test for advanced shape scenarios.
Existing Issue Identified: