search

thefex / Xamarin.Bindings.AdvancedRecyclerView

Xamarin Android binding library for: https://github.com/h6ah4i/android-advancedrecyclerview
Apache License 2.0
0 stars 0 forks source link

Provide a method to define initial group expanded state in controller #7

Closed nastutusha closed 5 years ago

thefex commented 6 years ago

I don't feel like it should be merged. It looks like a in-app domain feature specific option (taking in account that GroupExpandController can always be accessed anyway)

If you are able to give some cons and examples why every/almost every grouping list need that then I will merge that

nastutusha commented 6 years ago

Yeah, I can inherit from MvxExpandableItemAdapter, override this method and then patch MvxAdvancedRecyclerViewExpandableAdapterController, but there are a lot of internal classes dependencies that leads to a long chain of source code copy-pasting. This feature is present in base library https://github.com/h6ah4i/android-advancedrecyclerview/blob/ef2a8a22f5425a7feec4da20196acff25373b647/library/src/main/java/com/h6ah4i/android/widget/advrecyclerview/utils/AbstractExpandableItemAdapter.java so I guess it should also be here, I've made it in the same way, so it won't break anything.

alexshikov commented 6 years ago

Hey @thefex

I understand your concern. But seems there is a problem with an additional layer of an abstract over original java library.

In short:

In details:

  1. We need to collapse only the first group of the list (true - this is specific to the app case)
  2. We know this case is supported by Java library (link)
  3. Current library has an internal implementation of an adapter - MvxExpandableItemAdapter
  4. Access to MvxExpandableItemAdapter is hidden inside of MvxAdvancedRecyclerViewExpandableAdapterController but configuration and event handling accessible through the new class MvxGroupExpandController
  5. To make a required feature available we need to create own MvxAdvancedExpandableRecyclerView that creates our own MvxAdvancedRecyclerViewExpandableAdapterController that creates our own MvxExpandableItemAdapter to simply override a method from the base library.
  6. We tried to make step 5 but there are too many internal methods and classes used, so we would need to completely copy the code of the library.

We believe, current PR extends the library in a simple way to make it possible to use an original feature.

Though, maybe we are missing something. Please, let us know if you see a proper way to access the origin method in the app. Or if the library should be patched in another area.

thefex commented 6 years ago

Thanks! That is perfectly fine, I will merge that then If that is exposed by orginal library - then sure, my bad, I have missed that

Not sure when I push the changes though as I am out-of-code for an upcoming week

alexshikov commented 6 years ago

hey @thefex, if you have time, it would be great to get this fix via NuGet too.

nastutusha commented 5 years ago

Hi @thefex just a friendly reminder that we really need this fix in NuGet. Any chance you can merge it soon?

thefex commented 5 years ago

will release new nuget today with other improvements

nastutusha commented 5 years ago

Thanks a lot!

thefex commented 5 years ago

@nastutusha sorry, not sure if I manage to release it today because I am finishing grouping swipe feature ...but anyway it will be released at worst case in next week because I need that urgently

nastutusha commented 5 years ago

@thefex How are the things going with your grouping swipe feature? It would be really great to have a nuget release this week.

thefex commented 5 years ago

@nastutusha This has been released, please check updated https://github.com/thefex/Xamarin.Bindings.AdvancedRecyclerView/blob/master/README.md