Conversation
Member
Author
|
/azp run |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes safe area default behavior by implementing proper handling when SafeAreaEdges is set to Default. The changes ensure that ContentView and Border controls return SafeAreaRegions.None when their SafeAreaEdges property is Default, while Page controls use legacy safe area behavior for backward compatibility.
Key changes:
- Updated default safe area handling for ContentView and Border controls to return None instead of unhandled Default values
- Modified Page safe area logic to use legacy IgnoreSafeArea property for backward compatibility
- Added comprehensive UI tests to validate safe area behavior across different edge configurations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/src/Core/ContentView/ContentView.cs | Added Default handling to return SafeAreaRegions.None for ContentView |
| src/Controls/src/Core/Border/Border.cs | Added Default handling to return SafeAreaRegions.None for Border |
| src/Controls/src/Core/Page/Page.cs | Replaced SafeAreaElement logic with legacy IgnoreSafeArea behavior |
| src/Controls/tests/TestCases.HostApp/Issues/Issue28986_ContentView.xaml | Updated XAML structure and added AutomationIds for ContentView test page |
| src/Controls/tests/TestCases.HostApp/Issues/Issue28986_Border.xaml | Updated XAML structure and added AutomationIds for Border test page |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_ContentView.cs | Added UI tests validating ContentView safe area behavior |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_Border.cs | Added UI tests validating Border safe area behavior |
Comments suppressed due to low confidence (3)
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_ContentView.cs:27
- The test assumes ContentView will be positioned at Y=0 when SafeAreaEdges is Default, but this behavior may vary across different iOS devices with different safe area configurations (e.g., devices with notches vs without). Consider using a more flexible assertion or testing on multiple device configurations to ensure reliable test results.
Assert.That(contentViewWithDefaultSettings.Y, Is.EqualTo(0),
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_Border.cs:28
- The test assumes Border content will be positioned at Y=5 due to stroke thickness, but this may not hold true across different iOS devices with varying safe area implementations. The hardcoded Y position assertion could cause test failures on devices with different safe area behaviors.
Assert.That(borderWithDefaultSettings.Y, Is.EqualTo(5), "Border should start at Y=5 when SafeAreaEdges is set to Default");
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_Border.cs:44
- Similar to the previous assertion, this hardcoded Y=5 expectation may not be reliable across different iOS device configurations. Consider using relative positioning assertions or device-specific test data to improve test reliability.
Assert.That(borderSafeAreaEdgesNone.Y, Is.EqualTo(5),
|
Azure Pipelines successfully started running 3 pipeline(s). |
rmarinho
approved these changes
Jul 30, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
By default we want "Border" and "ContentView" to be edge to edge controls because that's how they've always worked. We don't want this behavior to change for users.
This PR restores those defaults and adds tests to make sure that doesn't change in the future and that DEFAULT will always mean edge to edge
Functional Improvements to Safe Area Handling:
Border.csandContentView.cs: Added logic to returnSafeAreaRegions.Nonewhen the safe area edges are set toDefault, ensuring proper fallback behavior when no specific edges are configured. [1] [2]Page.cs: Updated safe area handling to respect legacyIgnoreSafeAreasettings, returningSafeAreaRegions.Nonefor edge-to-edge behavior orSafeAreaRegions.Containerotherwise.UI Test Enhancements:
Issue28986_Border.cs: Added a new test case to validate the behavior ofSafeAreaEdgesinBordercomponents, ensuring correct functionality forDefault,All, andNoneconfigurations.Updates to XAML Test Cases:
Issue28986_Border.xamlandIssue28986_ContentView.xaml: Refactored XAML files to useSafeAreaEdges="Default"and added test-specific configurations forNoneand edge-specific settings. Improved formatting for better readability. [1] [2]These changes collectively enhance safe area management and improve test coverage for edge-specific configurations.