mikepenz / FastAdapter

The bullet proof, fast and easy to use adapter library, which minimizes developing time to a fraction...
https://mikepenz.dev
Apache License 2.0
3.83k stars 492 forks source link

isOnlyOneExpandedItem not working #992

Closed jakoss closed 3 years ago

jakoss commented 3 years ago

About this issue

I'm not sure if it's my fault or the expanded implementation is broken somehow.

I implemented parent item like that:

class TimecardExpanderItem(val timecard: Timecard) :
    AbstractBindingItem<ViewHolderTimecardExpanderHeaderBinding>(),
    IExpandable<BindingViewHolder<ViewHolderTimecardExpanderHeaderBinding>>,
    IClickable<TimecardExpanderItem> {
    override val type: Int
        get() = R.id.timecard_expander_item

    override var identifier = timecard.stableId.hashCode().toLong()

    private var mOnClickListener: ClickListener<TimecardExpanderItem>? = null

    @Suppress("SetterBackingFieldAssignment")
    override var onItemClickListener: ClickListener<TimecardExpanderItem>? =
        { v, adapter, item, position ->
            if (v != null && item.timecard.entries.isNotEmpty()) {
                val binding = ViewHolderTimecardExpanderHeaderBinding.bind(v)

                if (!item.isExpanded) {
                    ViewCompat.animate(binding.headerIndicator).rotation(180f).start()
                } else {
                    ViewCompat.animate(binding.headerIndicator).rotation(0f).start()
                }
            }
            mOnClickListener?.invoke(v, adapter, item, position) ?: true
        }
        set(onClickListener) {
                this.mOnClickListener = onClickListener // on purpose
            }

    override var onPreItemClickListener: ClickListener<TimecardExpanderItem>?
        get() = null
        set(_) {}

    override val isAutoExpanding = true
    override var isExpanded = false
    override var parent: IParentItem<*>? = null
    override var subItems = mutableListOf<ISubItem<*>>()

Then i'm adding items to adapter like that:

val items = timecards
                        .map { timecard ->
                            val parent = TimecardExpanderItem(timecard)
                            if (timecard.entries.isNotEmpty()) {
                                val subItems = mutableListOf<ISubItem<*>>()
                                subItems.addAll(
                                    timecard.entries
                                        .map { timecardRecord ->
                                            val entry = TimecardEntryItem(timecardRecord)
                                            entry.parent = parent
                                            entry
                                        }
                                )
                                parent.subItems = subItems
                            }
                            parent
                        }
                    itemAdapter.set(items)

The issue is that if one item is expanded and i'm trying to expand other one - it won't collapse the old one. I checked under debugger that the flag isOnlyOneExpandedItem is true. I also traced the onClick call.

In file ExpandableExtension, on line 173, the line val expandedItems = getExpandedItemsSameLevel(pos) is always empty. The getExpandedItemsRootLevel always get's called (as expected i think) but here's the issue. We have check fastAdapter.getItem(i).ifExpandableParent, so the body won't ever be called, since this is root level and the parent is null. Am i doing something wrong here?

Details

Checklist

mikepenz commented 3 years ago

Could you maybe please try to reproduce this in the sample app? and maybe provide full sample code so I can also debug and get a better view what you see?

thanks

jakoss commented 3 years ago

Sure, minimal setup reproducing issue attached

ExpanderIssue.zip

mikepenz commented 3 years ago

@jakoss thanks for the sample.

The problem is, that the subitem is only an ISubItem and not a IExpandable. Causing this cast to fail: https://github.com/mikepenz/FastAdapter/blob/develop/fastadapter-extensions-expandable/src/main/java/com/mikepenz/fastadapter/expandable/ExpandableExtension.kt#L42

Will look if that's something we can improve

mikepenz commented 3 years ago

v5.4.1 will fix this for you :)

jakoss commented 3 years ago

Awesome, thanks. Implementing IExpandable in subitem seems counterintuitive if this subitem isn't really expandable :)

mikepenz commented 3 years ago

Yeah indeed. In the sample we use the AbstractExpandableItem as base as such this was not seen.

Thanks again for the report

jakoss commented 3 years ago

5.4.1 works perfectly, thanks