material-components / material-components-android

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

DrawerLayout not retaining selected state during configuration change with Jetpack Navigation #4187

Closed ArcherEmiya05 closed 4 months ago

ArcherEmiya05 commented 5 months ago

Description:

DrawerLayout selected state not surviving configuration change for menu that has nested group menu items.

Expected behavior:

Should be able to retain selected state.

Source code:

https://github.com/material-components/material-components-android/assets/38008900/cd6b7d86-2922-4e21-8e3a-96434e8a9880

Minimal sample app repro:

<?xml version="1.0" encoding="utf-8"?>
<menu xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    tools:showIn="navigation_view">

    <group android:checkableBehavior="single">

        <item
            android:id="@+id/menu_asset"
            android:icon="@drawable/ic_crypto"
            android:title="@string/assets" />

        <item
            android:id="@+id/menu_marketcap"
            android:icon="@drawable/ic_marketcap"
            android:title="@string/marketCap" />

        <item
            android:id="@+id/menu_news"
            android:icon="@drawable/ic_news"
            android:title="@string/news" />

        <item
            android:id="@+id/menu_videos"
            android:icon="@drawable/ic_video"
            android:title="@string/videos" />

    </group>

    <item android:title="@string/legal">
        <menu>
            <group android:checkableBehavior="single">
                <item
                    android:id="@+id/menu_about"
                    android:icon="@drawable/ic_about"
                    android:title="@string/about" />
            </group>
        </menu>
    </item>

</menu>
                // Passing each menu ID as a set of Ids because each menu should be considered as top level destinations.
                appBarConfiguration = AppBarConfiguration(
                    setOf(R.id.menu_asset, R.id.menu_marketcap, R.id.menu_news, R.id.menu_videos, R.id.menu_about),
                    drawerLayoutView
                )

                // Attach NavController to this NavigationView
                it.setupWithNavController(navController)

            }

Android API version: 21

Material Library version: 1.12.0

Device: Pixel (AVD)

dsn5ft commented 5 months ago

I just tried changing the selected item and rotating in our Catalog app demo for Nav Drawer:

https://github.com/material-components/material-components-android/blob/master/catalog/java/io/material/catalog/navigationdrawer/NavigationDrawerDemoActivity.java

And it retained the selected state as expected. So from the perspective of the NavigationView component I believe this is working correctly.

@ArcherEmiya05 can you check out our Catalog app and try comparing your setup to see what the difference might be?

dsn5ft commented 5 months ago

Alternatively if you could provide a separate minimal sample app that reproduces your setup and the issue, that would be helpful for debugging.

ArcherEmiya05 commented 5 months ago

I don't think the sample you link is having the same setup with the problem, however you may check the sample project below which demonstrate the bug. Note that this is the original sample from Android Studio project wizard for responsive UI, I just remove the image and change the drawer menu items. Again the issue here is the items in second group does not retain selected state. ResponsiveExample.zip

dsn5ft commented 5 months ago

Do you have any idea what the difference is between our sample and what is required to reproduce the bug?

ArcherEmiya05 commented 5 months ago

Do you have any idea what the difference is between our sample and what is required to reproduce the bug?

Hi, I already included the sample project which should run on latest Android Studio Jellyfish.

dsn5ft commented 5 months ago

Ah we actually have a pull request that looks like it would fix this bug:

https://github.com/material-components/material-components-android/pull/4155

Will see if I can reproduce the issue and import the PR.