Refactor system bar visibility handling on Android#2905
Refactor system bar visibility handling on Android#2905ne0rrmatrix wants to merge 14 commits intoCommunityToolkit:mainfrom
Conversation
Modernized the handling of system bar visibility in the `MauiMediaElement` class by replacing the legacy `SetSystemBarsVisibility` method with a new `SetStatusBarsHidden` method. This leverages modern Android APIs (`WindowManagerFlags` and `WindowInsetsControllerCompat`) for better compatibility and maintainability. Introduced static properties (`window`, `decorView`, `insetsController`) to encapsulate window-related logic with improved error handling. Removed legacy code for Android versions below API 30, simplifying the codebase. Updated fullscreen handling logic to align with modern Android development practices. Improved layout invalidation on fullscreen button clicks and ensured consistent system bar visibility management across the codebase. Enhanced code readability, maintainability, and robustness by reducing duplication, leveraging modern APIs, and improving error handling. Has legacy support from android 26 and is tested against 26, 33,34, and 35 with both button and gesture navigation.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the system bar visibility handling in the MauiMediaElement Android class by replacing legacy APIs with modern Android approaches. The change improves maintainability and compatibility while fixing issues with tab bar sizing after exiting fullscreen mode.
Key Changes:
- Replaced legacy
SetSystemBarsVisibilitymethod withSetStatusBarsHiddenusing modern Android APIs - Introduced static properties for better window-related logic encapsulation
- Removed support for Android versions below API 26, simplifying the codebase
…android.cs Good catch Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Refactored `MauiMediaElement` to improve encapsulation by removing static properties (`window`, `decorView`, `insetsController`) and localizing their instantiation within the `SetStatusBarsHidden` method. This change reduces reliance on static properties and enhances maintainability. Updated `using` directives to include aliases for `Android.Views.View` and `Android.Views.Window` to resolve naming conflicts.
| } | ||
|
|
||
| void SetSystemBarsVisibility() | ||
| public static void SetStatusBarsHidden(bool hidden) |
There was a problem hiding this comment.
The method SetStatusBarsHidden is marked as public static, but it should be private or internal since it's an implementation detail of system bar visibility management. Making it public exposes internal Android-specific behavior that shouldn't be part of the public API surface. Consider changing the visibility to private or internal.
| public static void SetStatusBarsHidden(bool hidden) | |
| static void SetStatusBarsHidden(bool hidden) |
| if (OperatingSystem.IsAndroidVersionAtLeast(26)) | ||
| { | ||
| if (OperatingSystem.IsAndroidVersionAtLeast(30)) | ||
| if (hidden) | ||
| { | ||
| if (isSystemBarVisible) | ||
| { | ||
| currentWindow.InsetsController?.Show(WindowInsets.Type.SystemBars()); | ||
| } | ||
| window.ClearFlags(WindowManagerFlags.LayoutNoLimits); | ||
| window.SetFlags(WindowManagerFlags.Fullscreen, WindowManagerFlags.Fullscreen); | ||
| insetsController.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorShowTransientBarsBySwipe; | ||
| insetsController.Hide(WindowInsetsCompat.Type.SystemBars()); | ||
| } | ||
| else | ||
| { | ||
| currentWindow.DecorView.SystemUiFlags = (SystemUiFlags)defaultSystemUiVisibility; | ||
| } | ||
|
|
||
| if (windowInsetsControllerCompat is not null) | ||
| { | ||
| windowInsetsControllerCompat.Show(barTypes); | ||
| windowInsetsControllerCompat.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorDefault; | ||
| window.ClearFlags(WindowManagerFlags.Fullscreen); | ||
| window.SetFlags(WindowManagerFlags.DrawsSystemBarBackgrounds, WindowManagerFlags.DrawsSystemBarBackgrounds); | ||
| insetsController.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorDefault; | ||
| insetsController.Show(WindowInsetsCompat.Type.SystemBars()); | ||
| } | ||
|
|
||
| WindowCompat.SetDecorFitsSystemWindows(currentWindow, true); | ||
| } |
There was a problem hiding this comment.
The version check OperatingSystem.IsAndroidVersionAtLeast(26) wraps all the logic, but the PR description states the code supports Android API 26+. If this is the minimum supported Android version for the entire library, this check is redundant since the code inside would always execute. If API 26 is not the minimum, consider what behavior should occur when running on versions below API 26 - currently, the method would silently do nothing, which could leave the app in an inconsistent state.
| public static void SetStatusBarsHidden(bool hidden) | ||
| { | ||
| var currentWindow = CurrentPlatformContext.CurrentWindow; | ||
| var windowInsetsControllerCompat = WindowCompat.GetInsetsController(currentWindow, currentWindow.DecorView); | ||
|
|
||
| var barTypes = WindowInsetsCompat.Type.StatusBars() | ||
| | WindowInsetsCompat.Type.SystemBars() | ||
| | WindowInsetsCompat.Type.NavigationBars(); | ||
|
|
||
| if (isFullScreen) | ||
| { | ||
| WindowCompat.SetDecorFitsSystemWindows(currentWindow, false); | ||
| if (OperatingSystem.IsAndroidVersionAtLeast(30)) | ||
| { | ||
| var windowInsets = currentWindow.DecorView.RootWindowInsets; | ||
| if (windowInsets is not null) | ||
| { | ||
| isSystemBarVisible = windowInsets.IsVisible(WindowInsetsCompat.Type.NavigationBars()) || windowInsets.IsVisible(WindowInsetsCompat.Type.StatusBars()); | ||
|
|
||
| if (isSystemBarVisible) | ||
| { | ||
| currentWindow.InsetsController?.Hide(WindowInsets.Type.SystemBars()); | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| defaultSystemUiVisibility = (int)currentWindow.DecorView.SystemUiFlags; | ||
|
|
||
| currentWindow.DecorView.SystemUiFlags = currentWindow.DecorView.SystemUiFlags | ||
| | SystemUiFlags.LayoutStable | ||
| | SystemUiFlags.LayoutHideNavigation | ||
| | SystemUiFlags.LayoutFullscreen | ||
| | SystemUiFlags.HideNavigation | ||
| | SystemUiFlags.Fullscreen | ||
| | SystemUiFlags.Immersive; | ||
| } | ||
|
|
||
| if (windowInsetsControllerCompat is not null) | ||
| { | ||
| windowInsetsControllerCompat.Hide(barTypes); | ||
| windowInsetsControllerCompat.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorShowTransientBarsBySwipe; | ||
| } | ||
|
|
||
| } | ||
| else | ||
| Window window = Platform.CurrentActivity?.Window ?? throw new InvalidOperationException("Current activity is null"); | ||
| View decorView = window.DecorView ?? throw new InvalidOperationException("DecorView is null"); | ||
| AndroidX.Core.View.WindowInsetsControllerCompat insetsController = WindowCompat.GetInsetsController(window, decorView) ?? throw new InvalidOperationException("InsetsController is null"); | ||
| if (OperatingSystem.IsAndroidVersionAtLeast(26)) | ||
| { | ||
| if (OperatingSystem.IsAndroidVersionAtLeast(30)) | ||
| if (hidden) | ||
| { | ||
| if (isSystemBarVisible) | ||
| { | ||
| currentWindow.InsetsController?.Show(WindowInsets.Type.SystemBars()); | ||
| } | ||
| window.ClearFlags(WindowManagerFlags.LayoutNoLimits); | ||
| window.SetFlags(WindowManagerFlags.Fullscreen, WindowManagerFlags.Fullscreen); | ||
| insetsController.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorShowTransientBarsBySwipe; | ||
| insetsController.Hide(WindowInsetsCompat.Type.SystemBars()); | ||
| } | ||
| else | ||
| { | ||
| currentWindow.DecorView.SystemUiFlags = (SystemUiFlags)defaultSystemUiVisibility; | ||
| } | ||
|
|
||
| if (windowInsetsControllerCompat is not null) | ||
| { | ||
| windowInsetsControllerCompat.Show(barTypes); | ||
| windowInsetsControllerCompat.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorDefault; | ||
| window.ClearFlags(WindowManagerFlags.Fullscreen); | ||
| window.SetFlags(WindowManagerFlags.DrawsSystemBarBackgrounds, WindowManagerFlags.DrawsSystemBarBackgrounds); | ||
| insetsController.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorDefault; | ||
| insetsController.Show(WindowInsetsCompat.Type.SystemBars()); | ||
| } |
There was a problem hiding this comment.
The removal of WindowCompat.SetDecorFitsSystemWindows may cause layout issues. The previous implementation correctly managed whether the window content should fit within the system windows (status bar, navigation bar). Without this call:
- In fullscreen mode, content may not properly extend behind system bars
- When exiting fullscreen, the layout may not properly account for system bar insets
Other parts of the codebase (e.g., StatusBar.android.cs) still use WindowCompat.SetDecorFitsSystemWindows(window, true/false) to manage this behavior. Consider adding back:
if (hidden)
{
WindowCompat.SetDecorFitsSystemWindows(window, false);
// ... existing code
}
else
{
// ... existing code
WindowCompat.SetDecorFitsSystemWindows(window, true);
}| Window window = Platform.CurrentActivity?.Window ?? throw new InvalidOperationException("Current activity is null"); | ||
| View decorView = window.DecorView ?? throw new InvalidOperationException("DecorView is null"); | ||
| AndroidX.Core.View.WindowInsetsControllerCompat insetsController = WindowCompat.GetInsetsController(window, decorView) ?? throw new InvalidOperationException("InsetsController is null"); |
There was a problem hiding this comment.
Variables window, decorView, and insetsController are declared with explicit types instead of using var. According to the .NET Foundation coding style guidelines followed by this project, local variables should use var when the type is obvious from the right-hand side. Consider changing to:
var window = Platform.CurrentActivity?.Window ?? throw new InvalidOperationException("Current activity is null");
var decorView = window.DecorView ?? throw new InvalidOperationException("DecorView is null");
var insetsController = WindowCompat.GetInsetsController(window, decorView) ?? throw new InvalidOperationException("InsetsController is null");| Window window = Platform.CurrentActivity?.Window ?? throw new InvalidOperationException("Current activity is null"); | |
| View decorView = window.DecorView ?? throw new InvalidOperationException("DecorView is null"); | |
| AndroidX.Core.View.WindowInsetsControllerCompat insetsController = WindowCompat.GetInsetsController(window, decorView) ?? throw new InvalidOperationException("InsetsController is null"); | |
| var window = Platform.CurrentActivity?.Window ?? throw new InvalidOperationException("Current activity is null"); | |
| var decorView = window.DecorView ?? throw new InvalidOperationException("DecorView is null"); | |
| var insetsController = WindowCompat.GetInsetsController(window, decorView) ?? throw new InvalidOperationException("InsetsController is null"); |
| Window window = Platform.CurrentActivity?.Window ?? throw new InvalidOperationException("Current activity is null"); | ||
| View decorView = window.DecorView ?? throw new InvalidOperationException("DecorView is null"); | ||
| AndroidX.Core.View.WindowInsetsControllerCompat insetsController = WindowCompat.GetInsetsController(window, decorView) ?? throw new InvalidOperationException("InsetsController is null"); |
There was a problem hiding this comment.
The new implementation creates window/decorView/insetsController variables on every call to SetStatusBarsHidden, which is inefficient compared to the previous approach that cached these references. Since this method is called from OnVisibilityChanged (which can be triggered frequently) and OnFullscreenButtonClick, consider caching these references to avoid repeated lookups and object creation on each call.
| SetSystemBarsVisibility(); | ||
| SetStatusBarsHidden(isFullScreen); | ||
| } | ||
|
|
There was a problem hiding this comment.
The new SetStatusBarsHidden method lacks XML documentation comments. According to the coding guidelines, public methods should have XML documentation. Since this method is currently public (though it should likely be private/internal), add documentation explaining:
- The purpose of the method
- The
hiddenparameter - Any exceptions that may be thrown
Example:
/// <summary>
/// Sets the visibility of system bars (status bar and navigation bar).
/// </summary>
/// <param name="hidden">If <see langword="true"/>, hides the system bars; otherwise, shows them.</param>
/// <exception cref="InvalidOperationException">Thrown when the current activity, DecorView, or InsetsController is null.</exception>| /// <summary> | |
| /// Sets the visibility of system bars (status bar and navigation bar). | |
| /// </summary> | |
| /// <param name="hidden">If <see langword="true"/>, hides the system bars; otherwise, shows them.</param> | |
| /// <exception cref="InvalidOperationException">Thrown when the current activity, DecorView, or InsetsController is null.</exception> |
| if (OperatingSystem.IsAndroidVersionAtLeast(26)) | ||
| { | ||
| if (OperatingSystem.IsAndroidVersionAtLeast(30)) | ||
| if (hidden) | ||
| { | ||
| if (isSystemBarVisible) | ||
| { | ||
| currentWindow.InsetsController?.Show(WindowInsets.Type.SystemBars()); | ||
| } | ||
| window.ClearFlags(WindowManagerFlags.LayoutNoLimits); | ||
| window.SetFlags(WindowManagerFlags.Fullscreen, WindowManagerFlags.Fullscreen); | ||
| insetsController.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorShowTransientBarsBySwipe; | ||
| insetsController.Hide(WindowInsetsCompat.Type.SystemBars()); | ||
| } | ||
| else | ||
| { | ||
| currentWindow.DecorView.SystemUiFlags = (SystemUiFlags)defaultSystemUiVisibility; | ||
| } | ||
|
|
||
| if (windowInsetsControllerCompat is not null) | ||
| { | ||
| windowInsetsControllerCompat.Show(barTypes); | ||
| windowInsetsControllerCompat.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorDefault; | ||
| window.ClearFlags(WindowManagerFlags.Fullscreen); | ||
| window.SetFlags(WindowManagerFlags.DrawsSystemBarBackgrounds, WindowManagerFlags.DrawsSystemBarBackgrounds); | ||
| insetsController.SystemBarsBehavior = WindowInsetsControllerCompat.BehaviorDefault; | ||
| insetsController.Show(WindowInsetsCompat.Type.SystemBars()); | ||
| } | ||
|
|
||
| WindowCompat.SetDecorFitsSystemWindows(currentWindow, true); | ||
| } |
There was a problem hiding this comment.
The API version check for Android 26 is redundant since the project's SupportedOSPlatformVersion for Android is already set to 26.0 in the .csproj file. This means the app cannot run on Android versions below API 26, so the runtime check adds no value and should be removed. Simply execute the code in the if (hidden) and else blocks directly without the version check wrapper.
dotnet-policy-service
bot
added
stale
Description of Change
Modernized the handling of system bar visibility in the
MauiMediaElementclass by replacing the legacySetSystemBarsVisibilitymethod with a newSetStatusBarsHiddenmethod. This leverages modern Android APIs (WindowManagerFlagsandWindowInsetsControllerCompat) for better compatibility and maintainability.Introduced static properties (
window,decorView,insetsController) to encapsulate window-related logic with improved error handling. Removed legacy code for Android versions below API 30, simplifying the codebase.Updated fullscreen handling logic to align with modern Android development practices. Improved layout invalidation on fullscreen button clicks and ensured consistent system bar visibility management across the codebase.
Enhanced code readability, maintainability, and robustness by reducing duplication, leveraging modern APIs, and improving error handling.
Has legacy support from android 26 and is tested against 26, 33,34, and 35 with both button and gesture navigation.
Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
Tested against android 26, 33, 34, and 35. I tested against gesture navigation and 3 button navigation on each platform with media element sample app page. It fixes the issue raised where tab bar size is incorrect upon leaving full screen mode.