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

Problem with selection in minidrawer #2688

Closed
5 tasks done
simoneraffaelli opened this issue Dec 7, 2020 · 9 comments · Fixed by #2690 or #2695
Closed
5 tasks done

Problem with selection in minidrawer #2688

simoneraffaelli opened this issue Dec 7, 2020 · 9 comments · Fixed by #2690 or #2695
Assignees
Labels

Comments

@simoneraffaelli
Copy link

simoneraffaelli commented Dec 7, 2020

About this issue

  • Hi, I made a custom implementation of MiniDrawerItem integrating also NavigationItem. The problem is that the selection isn't working properly. In particular the selection logic seems to work fine but is the view itself that isn't updating.
    The scenario is that when I select an Item i want the drawable icon to change color, in the expanded view all seems to work properly, in the collapsed one the view do not change color.
  • https://github.com/Raffaa/MaterialDrawerTestRepo

Details

  • [8.2.0] Used library version

Checklist

@mikepenz
Copy link
Owner

mikepenz commented Dec 7, 2020

@raffaa you mean you implemented a custom MiniDrawerItem? How does it react to selection changes? Do you have a StateListDrawable?

If not you may have to enable selectWithItemUpdate in the SelectExtension https://github.com/mikepenz/FastAdapter/blob/develop/library-core/src/main/java/com/mikepenz/fastadapter/select/SelectExtension.kt#L52

@mikepenz mikepenz self-assigned this Dec 7, 2020
@simoneraffaelli
Copy link
Author

@raffaa you mean you implemented a custom MiniDrawerItem? How does it react to selection changes? Do you have a StateListDrawable?

If not you may have to enable selectWithItemUpdate in the SelectExtension https://github.com/mikepenz/FastAdapter/blob/develop/library-core/src/main/java/com/mikepenz/fastadapter/select/SelectExtension.kt#L52

Hi mike, yeah I mean MiniDrawerItem, I provided a test project made up really really quickly so there might be some stupid errors (😄 ) but the problem is reproduced. I will dig into this "problem" in the next days

@simoneraffaelli
Copy link
Author

I made some tests, form my trials it came up that bindView is called only on the miniItem selected (the "PrimaryItem"'s bindView is called on all elements correctly).
I also tried to set selectWithItemUpdate to true but nothing changed.

@mikepenz
Copy link
Owner

@raffaa so I looked further into this and it looks like the problem is related to the NavController setup. The performNavigation will consume the event and no longer forward it to the mini drawer.

I'll see to provide an updated release with a fix for this

mikepenz added a commit that referenced this issue Dec 18, 2020
…ents before the `onDrawerItemClickListener`

  - FIX #2688
  - Note: This is a potential breaking change as it's no longer possible to consume the event from the `onDrawerItemClickListener` before informing the `miniDrawer` (which in general should be seen as an exception regardless, as it will prevent both to stay in sync)
@simoneraffaelli
Copy link
Author

simoneraffaelli commented Dec 18, 2020

Hi @mikepenz now it works, thanks!!
I noticed two little bugs:

  • With version 8.3.0 the closeOnClick property of the drawer layout won't work with miniDrawer. This because now to keep the drawers synced you call setSelection also on minidrawer, which calls the cossfader.crossfade. I think this is easily fixable by adding a simple check in the onItemClick method of the minidrawer: (don't know if this can produce some side effects)
fun onItemClick(selectedDrawerItem: IDrawerItem<*>): Boolean {
        //We only need to clear if the new item is selectable
        return if (selectedDrawerItem.isSelectable) {
            //crossfade if we are cross faded
            crossFader?.let {
                if (drawer?.closeOnClick != false && it.isCrossfaded) {
                    it.crossfade()
                }
            }
            //update everything
            if (selectedDrawerItem.hiddenInMiniDrawer) {
                selectExtension.deselect()
            } else {
                setSelection(selectedDrawerItem.identifier)
            }

            false
        } else {
            true
        }
    }
  • The mini drawer selection now work as expected (the icons change color), now there's a problem when I close the expanded drawer. In fact only when changing from "exapnded drawer" to minidrawer, the minidrawer icon of the selected item will be colored as non selected. Of course if I select other icons afterwards, they will color correctly. I didn't have much time to look at this problem in detail.

@mikepenz
Copy link
Owner

I will follow up on the first little bug you mentioned.

The second thing seems to may result from the tinting. be careful when using the same drawable for multiple views, with tinting. usually you should mutate all of them and try to have unique states for all of them, otherwise they may affect each other

@simoneraffaelli
Copy link
Author

I will follow up on the first little bug you mentioned.

The second thing seems to may result from the tinting. be careful when using the same drawable for multiple views, with tinting. usually you should mutate all of them and try to have unique states for all of them, otherwise they may affect each other

I didn't quite catch what you mean by "The second thing seems to may result from the tinting.". I treid to set isIconTinted to false and with this disabled when corssfading the color is mantained correctly but then it wont work the color change on the selection. Here's a video of the "problem": GDrive Link

@mikepenz
Copy link
Owner

I'd suggest you to cleanup your items as much as possible, most of the flexible tinting, ... stuff is not really relevant for your usecase as those things are only in the items for the super flexible API the library has to offer.

in your project I think you can eliminate most of these things.

The video looks to me like a shared drawable state is causing this, or some wrong state itself of the view.


the provided default items update the colors based on the view state using the android default state mechanisms and it fully depends on those to do their job :/

@simoneraffaelli
Copy link
Author

simoneraffaelli commented Dec 21, 2020

I'd suggest you to cleanup your items as much as possible, most of the flexible tinting, ... stuff is not really relevant for your usecase as those things are only in the items for the super flexible API the library has to offer.

in your project I think you can eliminate most of these things.

The video looks to me like a shared drawable state is causing this, or some wrong state itself of the view.

the provided default items update the colors based on the view state using the android default state mechanisms and it fully depends on those to do their job :/

Also the default items in my project have the same behavior, I tried to clean all my modifications and also then I was having the same issue. Then I changed the valorization of the icon, in particular I changed
icon = ImageHolder(AppCompatResources.getDrawable(requireContext(), R.drawable.whatever))
to
iconRes = R.drawable.whatever

and all seems working as expected

Thank you for your precious support 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants