tjerkw / Android-SlideExpandableListView

A better ExpandableListView, with animated expandable views for each list item
Apache License 2.0
1.98k stars 741 forks source link

the OnItemExpandCollapseListener not work! #100

Closed chenchongyu closed 9 years ago

chenchongyu commented 9 years ago

I want to change the icon of the toggleButton when I click the btn.So I set a OnItemExpandCollapseListener on the SlideExpandableListAdapter,but not work. When I debug,the listener is null.Why?

lorenzos commented 9 years ago

Same problem here. The listener is null, and I can't get why! Why is it closed?

lorenzos commented 9 years ago

Ok, I think I got it. In SlideExpandableListView.java line 44 a new SlideExpandableListAdapter is initialized, but instead documentation says that we are supposed to instantiate and pass it.

In README.md line 93:

Wrap your ListAdapter list.setAdapter(new SlideExpandableListAdapter(adapter, ...));

But in code:

public void setAdapter(ListAdapter adapter) {
    this.adapter = new SlideExpandableListAdapter(adapter);
    super.setAdapter(this.adapter);
}

Basically our custom SlideExpandableListAdapter, the one with our expand/collapse listener, is wrapper into another, new SlideExpandableListAdapter, which has no listener obviously.

This is a major issue in my opinion. If you think about it, the list adapter is wrapped two times with no reason at all. We should change the code (i.e. assigning the SlideExpandableListAdapter directly, without wrapping it) or the documentation (i.e. it should be said that we must pass the normal list adapter, not the SlideExpandableListAdapter, but then provide a getSlideExpandableListAdapter() method, so we can grab it and set our listener).

I'm ready to provide a pull request, but there's a choice to be made, and it must be made by @tjerkw

MarcoMirisola commented 8 years ago

Hi @lorenzos could you please show us the solution? I can't figure out how to resolve this issue. Thanks

lorenzos commented 8 years ago

Hi @MarcoMirisola

I didn't provide a pull request because I proposed two solutions that work both, but are different in architecture. I was waiting for a reply by @tjerkw on which is its favourite, and then I forgot this at all.

First proposed solution

Remove both the setAdapter methods in the SlideExpandableListView class and replace them with:

public void setAdapter(SlideExpandableListAdapter adapter) {
    this.adapter = adapter;
    super.setAdapter(this.adapter);
}

Second proposed solution

If I remeber well, I ended up chosing this one. Add the following method in the SlideExpandableListView class:

public SlideExpandableListAdapter getSlideExpandableListAdapter() {
    return adapter;
}

Change the setup code in your application from:

list.setAdapter(new SlideExpandableListAdapter(adapter, ...));

To:

list.setAdapter(adapter, ...);

Now, when you want to attach the listener, you do on it the instance returned by the new method, not on your original adapter:

list.getSlideExpandableListAdapter().setItemExpandCollapseListener(...)
MarcoMirisola commented 8 years ago

Thanks @lorenzos! I've used your second solution and it works perfectly. I hope @tjerkw replays you.