[NavigationRailView] Start insets only use the system bars instead of the system bar and display cutout insets

[t895]

Description: The navigation rail applies start insets only based on the system bars. This results in the rail appearing behind display cutouts. This component used to change its start insets based on the system bars and the display cutout.

Expected behavior: Apply start insets based on the system bars and display cutout

Source code:

material-components-android/lib/java/com/google/android/material/navigationrail/NavigationRailView.java

Line 216 in a2309ce

ViewUtils.isLayoutRtl(view) ? systemBarInsets.right : systemBarInsets.left;

Minimal sample app repro: https://github.com/t895/yuzu/tree/material-update

Android API version: API 34

Material Library version: 1.10.0+ The bug was introduced with this commit

Device: Samsung Galaxy S21 and Google Pixel 7 both running their latest stable software updates as of 1/11/24

[michael-winkler]

Still wrong on 1.13.0-alpha09

[michael-winkler]

@drchen @manabu-nakamura Please fix this. Thank you very much :)

[manabu-nakamura]

Sorry, I can't understand this issue.

[tfonteyn]

@manabu-nakamura https://developer.android.com/develop/ui/views/layout/edge-to-edge

[michael-winkler]

See here:
#3991

[manabu-nakamura]

I don't know whether NavigationRailView itself have to dodge the display cutout or not. Tell me some documentation.

Test10.zip
(before #3991)


(after #3991) The NavigationRailView is under the system navigation bar.

[manabu-nakamura]

#4006 ([Catalog] window overlaps with cutout area in landscape mode) was committed 1 year ago...

[michael-winkler]

From Google News. There is padding where the display cutout area is.

[manabu-nakamura]

Thank you. But it's not clear from just looking at this screenshot whether the NavigationRailView itself automatically have to dodge the display cutout or not.

https://developer.android.com/develop/ui/views/layout/display-cutout#best-practices:

Be mindful of the placement of critical elements of the UI. Don't let the cutout area obscure any important text, controls, or other information.

We (app developers) may need to manually dodge the display cutout if necessary...

[michael-winkler]

Thats' right but if we want to handle it by ourself we need to do it like this:

android:fitsSystemWindows="false"

and then:

ViewCompat.setOnApplyWindowInsetsListener(navigationRailView) { view, windowInsets ->
    val insets = windowInsets.getInsets(WindowInsetsCompat.Type.systemBars() + WindowInsetsCompat.Type.displayCutout())

    navigationRailView.updatePadding(
        left = insets.left,
        bottom = insets.bottom
    )

    ViewCompat.onApplyWindowInsets(view, windowInsets)
}

But why should we do it like this if a fix of the wrong values see here:
https://github.com/material-components/material-components-android/pull/3991/files

is enough?

[manabu-nakamura]

How about this?

ViewCompat.setOnApplyWindowInsetsListener(getWindow().getDecorView(), (v, insets) -> {
    Insets cutout = insets.getInsets(WindowInsetsCompat.Type.displayCutout());
    v.setPadding(cutout.left, 0, cutout.right, 0);
    return insets;
});

https://github.com/material-components/material-components-android/commits/master/lib/java/com/google/android/material/navigationrail/NavigationRailView.java:
@imhappi, could you tell us whether NavigationRailView itself automatically have to dodge the display cutout or not?

[tfonteyn]

if I may add my very humble opinion... (and please correct me if you think I'm wrong!)

The main issue is that the component handles the system-bars, but does not handle the cutouts (or gesture) insets.

Fundamentally the component should handle all if fitsSystemWindows==true, and none if fitsSystemWindows=false.

As you already handle systembars, then in-the-name-of backwards compatibility, you should also handle cutouts/gestures (fitsSystemWindows==true)... or give users the option to handle them themselves ( fitsSystemWindows=false)
... but avoid/fix the halfway house it is currently.

Personally I gave up, and either use fitsSystemWindows=false + handle insets myself; and don't use partially broken components at all.

[michael-winkler]

if I may add my very humble opinion... (and please correct me if you think I'm wrong!)

The main issue is that the component handles the system-bars, but does not handle the cutouts (or gesture) insets.

Fundamentally the component should handle all if fitsSystemWindows==true, and none if fitsSystemWindows=false.

As you already handle systembars, then in-the-name-of backwards compatibility, you should also handle cutouts/gestures (fitsSystemWindows==true)... or give users the option to handle them themselves ( fitsSystemWindows=false)
... but avoid/fix the halfway house it is currently.

Personally I gave up, and either use fitsSystemWindows=false + handle insets myself; and don't use partially broken components at all.

Thats correct. Not only cutout area but also systembars.

Then it is working fine.

[t895]

Just got caught up on the discussion here. I see that I failed to properly communicate the original issue and I've updated titles/descriptions to make things more clear. My main issue was that you used to be able to use fitsSystemWindows to avoid display cutouts and now that behavior changed. Even when using the new paddingStartSystemWindowInsets flag, it didn't avoid the display cutout when using the shortEdges flag*.

Also, sorry for the error on my PR that allowed the rail to go "under" the navigation bar. I've updated it to align with the previous behavior while avoiding those deprecated methods (Ex: insets.getSystemWindowInsetLeft())

[imhappi]

Hi all, sorry catching up on the discussion here as well.

Is the issue then that the cutout display doesn't add extra padding to the nav rail, so that the cutout is overlaid on top of the content? If so, then this PR #3991 looks good to me. I'll push it through this week

[michael-winkler]

Correct. The missing padding is the bug.

[manabu-nakamura]

Thank you for your explanation, @imhappi. I understand the spec.