-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix safe area defaults #30915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix safe area defaults #30915
Conversation
|
/azp run |
There was a problem hiding this 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 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). |
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.