Skip to content

Conversation

NirmalKumarYuvaraj
Copy link
Contributor

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

This pull request primarily removes Android-specific test exclusion directives (#if TEST_FAILS_ON_ANDROID and related #endif) from several test files, making previously excluded tests run on Android. Additionally, it refactors the safe area logic for handling keyboard insets on Android, simplifying and improving the calculation for the bottom edge when the keyboard is visible.

Test coverage improvements:

  • Removed #if TEST_FAILS_ON_ANDROID preprocessor directives from Issue28986.cs, Issue28986_ContentPage.cs, and Issue28986_SafeAreaBorderOrientation.cs, enabling tests that were previously skipped on Android to now run on that platform. [1] [2] [3] [4] [5] [6]

Safe area logic refactoring:

  • Simplified the logic in SafeAreaExtensions.cs by removing a special case for SoftInput.AdjustPan and refactoring how the bottom safe area is calculated when the keyboard is showing, delegating to a new helper method for clarity and correctness. [1] [2]
  • Introduced the HandleSoftInputRegion helper method to ensure the bottom safe area uses the larger of the keyboard inset or the original safe area when the keyboard is visible, and applies safe area only for "All" regions when the keyboard is hidden.

Issues Fixed

Fixes #

Output

Before After
Before.mov
After.mov

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Oct 22, 2025
@dotnet-policy-service
Copy link
Contributor

Hey there @@NirmalKumarYuvaraj! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Oct 22, 2025
@NirmalKumarYuvaraj NirmalKumarYuvaraj marked this pull request as ready for review October 22, 2025 11:07
@Copilot Copilot AI review requested due to automatic review settings October 22, 2025 11:07
Copy link
Contributor

@Copilot 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 fixes inconsistent soft input handling on Android by refactoring the safe area logic and re-enabling previously failing tests. The changes simplify keyboard inset calculations and remove Android-specific test exclusions that are now passing.

Key Changes

  • Refactored SafeAreaExtensions.cs to remove the SoftInput.AdjustPan special case and introduce a clearer HandleSoftInputRegion method for calculating bottom safe areas when the keyboard is visible
  • Removed #if TEST_FAILS_ON_ANDROID preprocessor directives from three test files (Issue28986.cs, Issue28986_ContentPage.cs, Issue28986_SafeAreaBorderOrientation.cs), enabling previously skipped tests to run on Android
  • Deleted one entire test method (SafeAreaBorderSoftInputWithOrientationChange) that was previously excluded on Android

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/Core/src/Platform/Android/SafeAreaExtensions.cs Refactored safe area calculation logic by removing AdjustPan special case and introducing HandleSoftInputRegion helper method
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_SafeAreaBorderOrientation.cs Removed Android test exclusion directives and deleted SafeAreaBorderSoftInputWithOrientationChange test
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_ContentPage.cs Removed Android test exclusion directives for SafeAreaPerEdgeValidation test
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986.cs Removed Android test exclusion directives for SafeAreaPerEdgeValidation test

Comment on lines +253 to +254
// When keyboard is hidden, only apply safe area for "All" regions
return safeAreaRegion == SafeAreaRegions.All ? originalSafeArea : 0;
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

The logic for when the keyboard is hidden appears incorrect. When safeAreaRegion is SafeAreaRegions.SoftInput and the keyboard is hidden, this returns 0. However, according to the previous logic (line 242 in the old code), when the keyboard is not showing for bottom edge SoftInput, it should return 0, which matches. But this new implementation doesn't account for other safe area region combinations that include SoftInput flag. The check safeAreaRegion == SafeAreaRegions.All is too restrictive - it won't apply safe area for combinations like SafeAreaRegions.All | SafeAreaRegions.SoftInput or other valid flag combinations. Consider using SafeAreaEdges.HasFlag pattern or checking if the region includes the All flag: return (safeAreaRegion & SafeAreaRegions.All) != 0 ? originalSafeArea : 0;

Suggested change
// When keyboard is hidden, only apply safe area for "All" regions
return safeAreaRegion == SafeAreaRegions.All ? originalSafeArea : 0;
// When keyboard is hidden, only apply safe area for regions that include "All"
return (safeAreaRegion & SafeAreaRegions.All) != 0 ? originalSafeArea : 0;

Copilot uses AI. Check for mistakes.

Comment on lines +257 to +259
// When keyboard is showing, use the larger of keyboard inset or original safe area
var keyboardInset = keyBoardInsets.Bottom;
return Math.Max(keyboardInset, originalSafeArea);
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

This logic unconditionally returns the maximum of keyboard inset and original safe area for all SoftInput regions when the keyboard is showing, but this doesn't align with the original behavior. The previous code (line 242) returned keyBoardInsets.Bottom directly when SafeAreaRegion was SoftInput and keyboard was showing. Using Math.Max will always apply at least the original safe area padding even when SoftInput is set alone (without All), which changes the behavior. For SafeAreaRegions.SoftInput only (without All), it should return just the keyboard inset, not the maximum. Consider checking if All flag is present: return (safeAreaRegion & SafeAreaRegions.All) != 0 ? Math.Max(keyboardInset, originalSafeArea) : keyboardInset;

Suggested change
// When keyboard is showing, use the larger of keyboard inset or original safe area
var keyboardInset = keyBoardInsets.Bottom;
return Math.Max(keyboardInset, originalSafeArea);
// When keyboard is showing, use the larger of keyboard inset or original safe area only if "All" is present
var keyboardInset = keyBoardInsets.Bottom;
return (safeAreaRegion & SafeAreaRegions.All) != 0 ? Math.Max(keyboardInset, originalSafeArea) : keyboardInset;

Copilot uses AI. Check for mistakes.

@jsuarezruiz
Copy link
Contributor

/azp run MAUI-UITests-public

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

var right = GetSafeAreaForEdge(GetSafeAreaRegionForEdge(2, layout), baseSafeArea.Right, 2, isKeyboardShowing, keyboardInsets);
var bottom = GetSafeAreaForEdge(GetSafeAreaRegionForEdge(3, layout), baseSafeArea.Bottom, 3, isKeyboardShowing, keyboardInsets);

if (isKeyboardShowing &&
Copy link
Contributor

Choose a reason for hiding this comment

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

How does the new logic handle AdjustPan mode correctly without this check?, no longer necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jsuarezruiz , The new approach uses SafeAreaRegions flags to determine if SoftInput should be handled, making it more flexible and declarative rather than checking the window's SoftInput mode directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When SafeAreaRegions.SoftInput is not set for the bottom edge, GetSafeAreaForEdge will return the original safe area or 0, effectively achieving the same result as the AdjustPan check but in a more systematic way.


// When keyboard is showing, use the larger of keyboard inset or original safe area
var keyboardInset = keyBoardInsets.Bottom;
return Math.Max(keyboardInset, originalSafeArea);
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously just returned keyboardInset. With floating/split keyboards, the keyboard inset might be smaller than the navigation bar safe area. Could verify?

#if TEST_FAILS_ON_ANDROID // Landscape orientation causes keyboard to occupy fullview
[Test]
[Category(UITestCategories.SafeAreaEdges)]
public void SafeAreaBorderSoftInputWithOrientationChange()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove it? Still have sense to validate orientation changes, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jsuarezruiz , In Android Once the orientation is changed to Landscape, the input control and the keyboard occupies the entire screen, this will cause failure in the Rect lookup. The portrait soft input scenarios are already handled in other test files (SafeAreaPerEdgeValidation).
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants