Skip to content
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

[Android] VisibleBoundsPadding.PaddingMask.Top does not always work as intended #6218

Closed
3 of 24 tasks
takla21 opened this issue Jun 10, 2021 · 28 comments · Fixed by #19474
Closed
3 of 24 tasks

[Android] VisibleBoundsPadding.PaddingMask.Top does not always work as intended #6218

takla21 opened this issue Jun 10, 2021 · 28 comments · Fixed by #19474
Assignees
Labels
difficulty/starter 🚀 Categorizes an issue for which the difficulty level is reachable by newcomers kind/bug Something isn't working platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform project/layout 🧱 Categorizes an issue or PR as relevant to layouting and containers (Measure/Arrange, Collections,..)

Comments

@takla21
Copy link
Contributor

takla21 commented Jun 10, 2021

Current behavior

When I removed the status bar by adding those 4 lines in the style.xml

<item name="android:windowTranslucentStatus">false</item>
<item name="android:windowTranslucentNavigation">true</item>
<item name="android:windowDrawsSystemBarBackgrounds">true</item>
<item name="android:statusBarColor">@android:color/transparent</item>

Applying VisibleBoundsPadding.PaddingMask.Top on top of my xaml, it won't give enough space for the status bar and notch (if available)

Android
image

*See workaround for expected result

Expected behavior

Expected result and Actual result should be mostly aligned vertically.

iOS
image

How to reproduce it (as minimally and precisely as possible)

Download StatusBarPaddingMask.zip
Deploy on Android

Workaround

To achieve similar result, I am able to retrieve the actual status bar height with StatusBar.GetForCurrentView().OccludedRect.Height. I am wondering if this should actually be used to calculate VisibleBoundsPadding.PaddingMask.Top

Environment

Nuget Package:

  • Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia
  • Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
  • Uno.SourceGenerationTasks
  • Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
  • Other:

Nuget Package Version(s): 3.9.0-dev.6

Affected platform(s):

  • iOS
  • Android
  • WebAssembly
  • WebAssembly renderers for Xamarin.Forms
  • macOS
  • Skia
    • WPF
    • GTK (Linux)
    • Tizen
  • Windows
  • Build tasks
  • Solution Templates

IDE:

  • Visual Studio 2017 (version: )
  • Visual Studio 2019 (version: 16.8.4)
  • Visual Studio for Mac (version: )
  • Rider Windows (version: )
  • Rider macOS (version: )
  • Visual Studio Code (version: )

Relevant plugins:

  • Resharper (version: )

Anything else we need to know?

@takla21 takla21 added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform labels Jun 10, 2021
@MartinZikmund MartinZikmund added difficulty/starter 🚀 Categorizes an issue for which the difficulty level is reachable by newcomers project/layout 🧱 Categorizes an issue or PR as relevant to layouting and containers (Measure/Arrange, Collections,..) good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. and removed difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. triage/untriaged Indicates an issue requires triaging or verification labels Jun 10, 2021
@jeromelaban
Copy link
Member

The computation is supposed to use the status bar size to set the ApplicationView.VisibleBounds property:

var visibleBounds = CalculateVisibleBounds(statusBarSizeExcluded);
var trueVisibleBounds = CalculateVisibleBounds(statusBarSize);
ApplicationView.GetForCurrentView()?.SetVisibleBounds(visibleBounds);
ApplicationView.GetForCurrentView()?.SetTrueVisibleBounds(trueVisibleBounds);

Could be the computation is done too early or not using the same content from occluded area.

@Youssef1313
Copy link
Member

My best guess is that what stated in this comment isn't actually working:

// The real metrics excluded the StatusBar only if it is plain.
// We want to subtract it if it is translucent. Otherwise, it will be like we subtract it twice.

Nothing gets subtracted when android:windowTranslucentStatus is false.

@davidjohnoliver Since you worked on a similar issue in #4352, would you be able to help where/how to investigate?

@davidjohnoliver
Copy link
Contributor

@Youssef1313 This is the 'where', indeed. :)

The point of the comment, IIRC, is that the native metrics returned by Android are different depending if the status bar is opaque or translucent. Accordingly we don't to 'double dip' in certain parts of the math by subtracting it twice. But clearly if the VisibleBounds is not taking the translucent status bar into account then the logic is faulty somewhere.

@rafaelrmoukc
Copy link
Contributor

Start work on that issue, error still repro.

@jeromelaban jeromelaban changed the title [Android] VisibleBoundsPadding.PaddingMask.Top does not always work as attended [Android] VisibleBoundsPadding.PaddingMask.Top does not always work as intended Jun 2, 2022
@rafaelrmoukc
Copy link
Contributor

rafaelrmoukc commented Jun 7, 2022

After some study i got that:

Android has a "fitsSystemWindows" attr to sets the padding to your view (if needed) to prevent your content from being obscured by the system status bar or navigation bar.

What realy need to do isn't change the padding manually, but set fitsSystemWindows=true at the Layouts. Only problem is the fitsSystemWindows not work with some Layouts (eg.: FrameLayout) and need a override like that https://proandroiddev.com/draw-under-status-bar-like-a-pro-db38cfff2870

@jeromelaban what you think about?

@jeromelaban
Copy link
Member

@rafaelrmoukc the issue here is about the computations made by the Window.VisibleBounds property. I would not be surprised that we're not computing the size of the status bar here:

var statusBarSizeExcluded = IsStatusBarTranslucent() ?
// The real metrics excluded the StatusBar only if it is plain.
// We want to subtract it if it is translucent. Otherwise, it will be like we subtract it twice.
statusBarSize :
0;
var navigationBarSizeExcluded = GetLogicalNavigationBarSizeExcluded();

When android:windowDrawsSystemBarBackgrounds is set to true.

Can you validate the value of that property?

@rafaelrmoukc
Copy link
Contributor

@jeromelaban of course, I will do that now, thanks for the help.

@rafaelrmoukc
Copy link
Contributor

@jeromelaban only to make a test I set the statusBarSizeExcluded to 100, and receive the same result at GetVisualBoundsLegacy (no changes at app).

Without set to 100, the size is 24 (right size), and not change (same result).

See below.

Captura de Tela (70)

Captura de Tela (71)

I think the problem isn't on computing the size, but when apply that. Any idea?

@iury-kc see that.

@rafaelrmoukc
Copy link
Contributor

I change the trueVisibleBounds too, and got same result.

@jeromelaban
Copy link
Member

interesting. So the CalculateVisibleBounds method is not doing the right computation, maybe because of this line?

var topHeightExcluded = Math.Max(Insets.Top, excludedStatusBarHeight);

@rafaelrmoukc
Copy link
Contributor

rafaelrmoukc commented Jun 8, 2022 via email

@rafaelrmoukc
Copy link
Contributor

@jeromelaban I set 100 on both vars, topHeightExcluded and statusBarSizeExcluded, but nothing change on screen. Like i said before, the values of Bound are rigth, but the values isn't applied at the view, see the screenshot:

Captura de Tela (76)

Captura de Tela (72)

Captura de Tela (73)

If u check the workaround @takla21 do, he change the padding of Layout (grid), not of the Window (Activity?)

private static void OnIsEnabledChanged(DependencyObject sender, DependencyPropertyChangedEventArgs args) { #if !WINDOWS_UWP var baseElement = (Grid)sender; var statusBar = StatusBar.GetForCurrentView().OccludedRect; var thickness = new Thickness( baseElement.Padding.Left, baseElement.Padding.Top + (int)statusBar.Height, baseElement.Padding.Right, baseElement.Padding.Bottom); baseElement.Padding = thickness; #endif }

What u think about?

See @iury-kc

@jeromelaban
Copy link
Member

The window is not supposed to have a padding. The configuration of the app is done in such a way that the entirety of the screen is supposed to be used.

If the VisibleBounds is correct, then it may be an issue with the VisibleBoundsPadding itself:

private void UpdatePadding()

@rafaelrmoukc
Copy link
Contributor

I will check that, thank you for the help @jeromelaban !

@rafaelrmoukc
Copy link
Contributor

rafaelrmoukc commented Jun 9, 2022

@jeromelaban After update Uno Source to last version the issue stop to occur, screenshots bellow on API <= 29 (simulator) and API > 29 (device)

@iury-kc

This can be close.

@jeromelaban
Copy link
Member

Interesting! Would you be able to find which version of Uno fixed this? Which one were you using and which one are you using now? It'll be interesting to find which change fixed this, as there may be related issues that would benefit from knowing what happened. Thanks!

@rafaelrmoukc
Copy link
Contributor

@jeromelaban Before i have 4.4.0-dev.164 now 4.4.0-dev.205, I can check one by one if u need. Do you?

@jeromelaban
Copy link
Member

Yes please, there won't be that many packages to validate, thanks!

@rafaelrmoukc
Copy link
Contributor

Of course, thanks!

@ghost
Copy link

ghost commented Jun 9, 2022

Nice! There are many issues related to this behavior, specially with Android.
For example:
#6216

Probably, it will help/guide to close most of them.

@rafaelrmoukc
Copy link
Contributor

rafaelrmoukc commented Jun 9, 2022

@jeromelaban after the PR #8960 the problem disappear.

What happen before #8960: Window class compute padding, but the UpdatePadding never fire

After #8960: Window class compute padding, the UpdatePadding fire

I Will test #6216 to check if that is solved too.

@jeromelaban
Copy link
Member

It's very curious, I do not see how #8960 could have had an impact on this behavior... it seems completely unrelated to layout management.

@rafaelrmoukc
Copy link
Contributor

@jeromelaban @iury-kc sorry, after a double check i add activity.Window.Attributes.Flags.HasFlag(WindowManagerFlags.DrawsSystemBarBackgrounds) on the code before update and on update source code that stay. That check solve the problem

Test 6216 and if both are solve i will implement TESTS

@Lee31416
Copy link

Lee31416 commented Jan 29, 2025

I still get this issue on uno "5.6.20" which is the latest when I updated today. I have a sample but for some reason I can't paste it here, but Kevin's issue still holds true with the android translucent flag and his workaround also still works althought it's not ideal

@jeromelaban
Copy link
Member

@Lee31416 would you be able to try with SafeArea instead?

@Lee31416
Copy link

Lee31416 commented Feb 5, 2025

@jeromelaban I already tried that and most other workaround we had done before like VisibleBoundsPaddingMask, SafeArea.insets, the safearea control and nothing seems to work

@MartinZikmund MartinZikmund self-assigned this Feb 7, 2025
@MartinZikmund MartinZikmund removed the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Feb 10, 2025
@MartinZikmund MartinZikmund linked a pull request Feb 10, 2025 that will close this issue
6 tasks
@Lee31416
Copy link

@jeromelaban which version of uno will contain the fix ?

@jeromelaban
Copy link
Member

jeromelaban commented Feb 11, 2025

@Lee31416 it should be an upcoming 5.6 service release, but there's an additional update coming in #19501

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