material-components / material-components-android

Modular and customizable Material Design UI components for Android
Apache License 2.0
16.3k stars 3.06k forks source link

Request: Update BottomSheet edge to edge style api names #2137

Closed ColtonIdle closed 2 years ago

ColtonIdle commented 3 years ago

Bottom sheets now apparently adhere to these theme styles:

On API 21 and above the BottomSheet will be rendered fullscreen (edge to edge) if the navigationBar is transparent, and enableEdgeToEdge is true. It can automatically add insets if any of paddingBottomSystemWindowInsets, paddingLeftSystemWindowInsets, paddingRightSystemWindowInsets, or paddingTopSystemWindowInsets are set to true in the style, either by updating the style passed to the constructor, or by updating the default style specified by the bottomSheetDialogTheme attribute in your theme.

Which is a little misleading in my opinion because it seems like setting paddingBottomSystemWindowInsets will enable padding throughout the whole app.

   <style name="RollerToasterTheme" parent="Theme.MaterialComponents.DayNight.NoActionBar">
        <!-- Window decor -->
        <item name="android:statusBarColor">@color/transparent</item>
        <item name="android:navigationBarColor">@color/navcolor</item>
        <item name="enableEdgeToEdge">true</item>
        <item name="paddingBottomSystemWindowInsets">true</item>
    </style>

Am I applying the attributes in the wrong place? Wouldn't a better name be bottomSheetEnableEdgeToEdge and bottomSheetPaddingBottomSystemWindowInsets?

AfzalivE commented 3 years ago

Is that style your bottomSheetDialogTheme? Or the theme for your whole app?

It should be set like this in your app's theme:

https://github.com/material-components/material-components-android/blob/master/catalog/java/io/material/catalog/application/theme/res/values/themes.xml#L23

<style name="Theme.Catalog" parent="Theme.MaterialComponents.DayNight.NoActionBar">
    <item name="catalogToolbarStyle">@style/Widget.Catalog.Toolbar</item>
    <item name="catalogToolbarWithCloseButtonStyle">@style/Widget.Catalog.Toolbar.WithCloseButton</item>
    <item name="textInputSignInStyle">@style/Widget.MaterialComponents.TextInputLayout.OutlinedBox</item>
    <item name="bottomSheetDialogTheme">@style/ThemeOverlay.Catalog.BottomSheetDialog</item>
    <item name="windowActionModeOverlay">true</item>
</style>

Where ThemeOverlay.Catalog.BottomSheetDialog is like this:

<style name="ThemeOverlay.Catalog.BottomSheetDialog" parent="ThemeOverlay.MaterialComponents.BottomSheetDialog">
        <item name="paddingBottomSystemWindowInsets">true</item>
</style>

At least that's my understanding.

drchen commented 2 years ago

Well it's meant to be set in the bottom sheet styles. (Setting it in themes of course also works though...)

So I think the name is fine. And since it's a public API we can't really to change it either...

ColtonIdle commented 2 years ago

No idea that it was supposed to be put there. I think when I originally read the docs/rel notes on it that didn't seem clear. But that's probably because themes/styles in android view land can be insanely hard to understand. I haven't touched views in like a year since I'm all on the compose train now, so feel free to close whatever bugs I opened here lol