-
Notifications
You must be signed in to change notification settings - Fork 473
Refactor system bar visibility handling on Android #2905
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
base: main
Are you sure you want to change the base?
Changes from 12 commits
c8bb250
d232160
74bc57f
bebea47
23fad18
2fa5a76
1d0eeef
3c36391
057968c
42c6154
26de3a4
07a243c
cdb62a9
a298d83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,8 @@ | |||||||||||||
| using AndroidX.Core.View; | ||||||||||||||
| using AndroidX.Media3.UI; | ||||||||||||||
| using CommunityToolkit.Maui.Views; | ||||||||||||||
| using View = Android.Views.View; | ||||||||||||||
| using Window = Android.Views.Window; | ||||||||||||||
|
|
||||||||||||||
| [assembly: UsesPermission(Android.Manifest.Permission.ForegroundServiceMediaPlayback)] | ||||||||||||||
| [assembly: UsesPermission(Android.Manifest.Permission.ForegroundService)] | ||||||||||||||
|
|
@@ -23,8 +25,6 @@ public class MauiMediaElement : CoordinatorLayout | |||||||||||||
| readonly RelativeLayout relativeLayout; | ||||||||||||||
| readonly PlayerView playerView; | ||||||||||||||
|
|
||||||||||||||
| int defaultSystemUiVisibility; | ||||||||||||||
| bool isSystemBarVisible; | ||||||||||||||
| bool isFullScreen; | ||||||||||||||
|
|
||||||||||||||
| #pragma warning disable CS8618 // Non-nullable field is uninitialized. Consider declaring as nullable. | ||||||||||||||
|
|
@@ -78,7 +78,7 @@ protected override void OnVisibilityChanged(Android.Views.View changedView, [Gen | |||||||||||||
| base.OnVisibilityChanged(changedView, visibility); | ||||||||||||||
| if (isFullScreen && visibility is ViewStates.Visible) | ||||||||||||||
| { | ||||||||||||||
| SetSystemBarsVisibility(); | ||||||||||||||
| SetStatusBarsHidden(isFullScreen); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -131,77 +131,33 @@ void OnFullscreenButtonClick(object? sender, PlayerView.FullscreenButtonClickEve | |||||||||||||
| isFullScreen = false; | ||||||||||||||
| layout?.RemoveView(relativeLayout); | ||||||||||||||
| AddView(relativeLayout); | ||||||||||||||
| relativeLayout.Invalidate(); | ||||||||||||||
| } | ||||||||||||||
| // Hide/Show the SystemBars and Status bar | ||||||||||||||
| SetSystemBarsVisibility(); | ||||||||||||||
| SetStatusBarsHidden(isFullScreen); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| void SetSystemBarsVisibility() | ||||||||||||||
| public static void SetStatusBarsHidden(bool hidden) | ||||||||||||||
|
||||||||||||||
| public static void SetStatusBarsHidden(bool hidden) | |
| static void SetStatusBarsHidden(bool hidden) |
Copilot
AI
Nov 20, 2025
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.
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"); |
Copilot
AI
Nov 20, 2025
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.
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.
Copilot
AI
Nov 20, 2025
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.
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);
}
Copilot
AI
Nov 20, 2025
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.
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.
Copilot
AI
Nov 24, 2025
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.
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.
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.
The new
SetStatusBarsHiddenmethod 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:hiddenparameterExample: