mikepenz / MaterialDrawer

The flexible, easy to use, all in one drawer library for your Android project. Now brand new with material 2 design.
https://mikepenz.dev
Apache License 2.0
11.67k stars 2.05k forks source link

[Feature request] Allow custom RecyclerView layout? #2611

Closed cketti closed 4 years ago

cketti commented 4 years ago

In K-9 Mail we're overriding the material_drawer_recycler_view layout to wrap the RecyclerView in a SwipeRefreshLayout. This is causing a problem when used in combination with a sticky footer because the library is changing the layout parameters of the RecyclerView directly to make it end above the sticky footer. For now we're working around this using this ugly hack: https://github.com/k9mail/k-9/pull/4805

@mikepenz: Both overriding the layout and the layout parameter hack are things we'd like to avoid. Is there a way you could change the library to support our use case?

mikepenz commented 4 years ago

I'll have a look and see what we can add to allow for this usecase

mikepenz commented 4 years ago

Hey @cketti I just had a look and it looks like you are still using v7 of the drawer library which tries to do all sorts of magic on its own.

With v8 I stopped doing this and give full control back to the hands of the integrating engineer.

E.g. you define the DrawerLayout the way you need it to in the app and the drawer library itself is only the slider.

E.g the RelativeLayout with the RecyclerView and the sticky footer on the bottom.

I had a look and started doing the migration of the code, but then I figured that the app still depends on appcompat which prevents the use as the MaterialDrawer v8 uses a material component theme as base.

do you have plans to migrate to material components instead of appcompat?

cketti commented 4 years ago

do you have plans to migrate to material components instead of appcompat?

Yes, though probably only after releasing a new stable version. We also have an open issue for migrating to MaterialDrawer v8 :smile:

I had a brief look at the develop branch and was still seeing this:

https://github.com/mikepenz/MaterialDrawer/blob/a69dbc24353ff6f7f3e0e8ce17bab19d612493d3/library/src/main/java/com/mikepenz/materialdrawer/util/DrawerUtils.kt#L189-L192

That lead me to believe it might still be an issue in MaterialDrawer v8. If you're saying that what we want to do is already possible in v8, that's great. In that case please close the issue. We'll keep the hack for now and fix it once we upgrade to MaterialDrawer v8.

Thank you very much for looking into this. Much appreciated :heart:

mikepenz commented 4 years ago

@cketti I started preparing v8 migration. but yeah due to material theme I didn't finish it :/

I can open a branch though to continue / have a base to migrate to v8 faster then.

Let me look some more next weekend.

if you wrap it like:

Drawer

Pull to Refresh

MaterialDrawerSliderView (is a RelativeLayout which then has the RV)

then it shouldn't cause problems. but I am not 100% sure right now if that may have nested scrolling issues. regardless. I'll have a look and add it somewhere in the sample app :)

mikepenz commented 4 years ago

As mentioned opened a PR with the adjustments needed to upgrade to v8 :) https://github.com/k9mail/k-9/pull/4829

mikepenz commented 4 years ago

Closing this as a PR to update to v8 was submitted and v8 supports the requested feature :)