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

Dynamically add and remove listeners #427

Closed
rusxakep opened this issue Aug 23, 2017 · 8 comments
Closed

Dynamically add and remove listeners #427

rusxakep opened this issue Aug 23, 2017 · 8 comments
Milestone

Comments

@rusxakep
Copy link

FlexibleAdapter have addListener function, it's good. What do you think about removeListener(this) function?

@davideas davideas added this to the 5.0.0-rc3 milestone Aug 23, 2017
@davideas
Copy link
Owner

@rusxakep, yes, we can add this feature.
Thanks!

@davideas
Copy link
Owner

davideas commented Aug 23, 2017

Listeners can be cleared by providing 1 or more Class objects to a unique method: User cannot pass the instance because several listeners can be attached to that instance, so it will remove them all and is not good. If instead, I provide several remove methods, it will be then possible, but it is much better to specify the .class property of the interface so it can be removed from everywhere even without instance!

For instance:

mAdapter.removeListener(
        FlexibleAdapter.OnItemLongClickListener.class,
        FlexibleAdapter.OnUpdateListener.class);

Also, I will fix the set of the click and long click in the ViewHolder based of the existence of the click and long click listeners respectively.

@rusxakep
Copy link
Author

FYI, this functionality for example need for working with one adapter instance with more than one fragment, if you need, for example, different logic onclick/onlongclick in fragment's

@davideas
Copy link
Owner

davideas commented Aug 23, 2017

If you remove in a second moment and at runtime a long click listeners, the bound views will still have the listener attached! The event will be fired anyway, but blocked inside the event callback because listener instance inside the adapter will be empty.

So, do you want also to remove the event from the bound ViewHolders?
The same problem will occur when you will assign again the listener, the current bound ViewHolders will then don't receive the callback! What do you think?

PS. Currently I already have all the bound ViewHolders in memory.

@rusxakep
Copy link
Author

rusxakep commented Aug 23, 2017

About this code:

mAdapter.removeListener(
        FlexibleAdapter.OnItemLongClickListener.class,
        FlexibleAdapter.OnUpdateListener.class);

I thinked about function like addListener:

function void removeListener() {
        onItemClickListener = null;
        onItemLongClickListener = null;
}

Or you want change all logic for listeners?

@davideas
Copy link
Owner

davideas commented Aug 23, 2017

@rusxakep, if I add this new method it must support all types, not only these 2 and the current state on already bound ViewHolders must be updated too, as I mentioned.

Fine. I will remove and add the listeners from the ViewHolders too.

@davideas davideas changed the title Dynamically add and remove listener's Dynamically add and remove listeners Aug 23, 2017
@rusxakep
Copy link
Author

rusxakep commented Aug 23, 2017

second moment
you meant - second fragment?

Anyway, I meant about removing listeners capability from adapter, not in viewholders, because viewholder onclick/onlongclick interface do not support addListener feature.

I am using one recyclerview in common activity for many fragments. Every fragment add/remove own custom items from common activity's recyclerview. In this architecture, I want with replace fragment and prepare (clear) recyclerview in activity -> replace action for click-click events too.

@davideas
Copy link
Owner

davideas commented Aug 23, 2017

I understood your use case, for that you don't need to remove the listener but just to update it: the addListener(), in the end, is a set XYZ listener grouped in one function.

However, your idea to remove the listener only, I have to implement it as I explained because the current ViewHolders were created with that listener and the removal of it must be propagated to the visible Views as well for completeness otherwise the event is fired however.

Check the code I will commit now.

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

No branches or pull requests

2 participants