unoplatform / uno

Open-source platform for building cross-platform native Mobile, Web, Desktop and Embedded apps quickly. Create rich, C#/XAML, single-codebase apps from any IDE. Hot Reload included! 90m+ NuGet Downloads!!
https://platform.uno
Apache License 2.0
9.06k stars 736 forks source link

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

Open takla21 opened 3 years ago

takla21 commented 3 years ago

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:

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

Affected platform(s):

IDE:

Relevant plugins:

Anything else we need to know?

jeromelaban commented 3 years ago

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

https://github.com/unoplatform/uno/blob/d59985ffb51895568c5aa7fbd02c9499071894fe/src/Uno.UI/UI/Xaml/Window.Android.cs#L156-L159

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

Youssef1313 commented 3 years ago

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

https://github.com/unoplatform/uno/blob/0c075f5f053b4ced918da339e74ad94bf5147f88/src/Uno.UI/UI/Xaml/Window.Android.cs#L91-L92

Nothing gets subtracted when android:windowTranslucentStatus is false.

@davidjohnoliver Since you worked on a similar issue in https://github.com/unoplatform/uno/pull/4352, would you be able to help where/how to investigate?

davidjohnoliver commented 3 years ago

@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 commented 2 years ago

Start work on that issue, error still repro.

rafaelrmoukc commented 2 years ago

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?

Just a moment...
jeromelaban commented 2 years ago

@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:

https://github.com/unoplatform/uno/blob/d59985ffb51895568c5aa7fbd02c9499071894fe/src/Uno.UI/UI/Xaml/Window.Android.cs#L105-L110

When android:windowDrawsSystemBarBackgrounds is set to true.

Can you validate the value of that property?

rafaelrmoukc commented 2 years ago

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

rafaelrmoukc commented 2 years ago

@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 commented 2 years ago

I change the trueVisibleBounds too, and got same result.

jeromelaban commented 2 years ago

interesting. So the CalculateVisibleBounds method is not doing the right computation, maybe because of this line? https://github.com/unoplatform/uno/blob/73247e5b154985abe84472157ea96c80762cb6e7/src/Uno.UI/UI/Xaml/Window.Android.cs#L162

rafaelrmoukc commented 2 years ago

I Will check that nos, thanks.

Em terça-feira, 7 de junho de 2022, Jérôme Laban @.***> escreveu:

interesting. So the CalculateVisibleBounds method is not doing the right computation, maybe because of this line? https://github.com/unoplatform/uno/blob/73247e5b154985abe84472157ea96c 80762cb6e7/src/Uno.UI/UI/Xaml/Window.Android.cs#L162

— Reply to this email directly, view it on GitHub https://github.com/unoplatform/uno/issues/6218#issuecomment-1148600006, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZNT4LEDYW775RF57SHJFC3VN46CVANCNFSM46NCEQKQ . You are receiving this because you were mentioned.Message ID: @.***>

rafaelrmoukc commented 2 years ago

@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 commented 2 years ago

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: https://github.com/unoplatform/uno/blob/489be09cf3b5a227d72ce5efe5b1f79d98dd8f6e/src/Uno.UI/Behaviors/VisibleBoundsPadding.cs#L214

rafaelrmoukc commented 2 years ago

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

rafaelrmoukc commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

rafaelrmoukc commented 2 years ago

Of course, thanks!

ghost commented 2 years ago

Nice! There are many issues related to this behavior, specially with Android. For example: https://github.com/unoplatform/uno/issues/6216

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

rafaelrmoukc commented 2 years ago

@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 commented 2 years ago

It's very curious, I do not see how https://github.com/unoplatform/uno/pull/8960 could have had an impact on this behavior... it seems completely unrelated to layout management.

rafaelrmoukc commented 2 years ago

@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