Block clicks on views that are being removed

[elihart]

We can check that position != NO_POSITION in our click listener wrapper.

I am also considering adding some debouncing support

[niccorder]

I have some free time I'll hop on this. Let's save the debounce support for another issue, but I'm all for supporting debouncing!

[elihart]

Great!

The easiest way to do this now would be to change WrappedEpoxyModelClickListener#onClick to check the adapter position before calling onClick of the nested listener.

That would be a very easy change, but it only helps people that used OnModelClickListener. I'm considering changing the annotation processor to wrap all click listeners with WrappedEpoxyModelClickListener so everything gets this protection. A downside is it creates extra click listener objects.

When we want to do the debouncing we would also change WrappedEpoxyModelClickListener#onClick and do something like https://github.com/JakeWharton/butterknife/blob/master/butterknife/src/main/java/butterknife/internal/DebouncingOnClickListener.java

Butterknife uses a post, but I'm wondering if just checking system clock is more efficient, and also if we may want to delay slightly longer, since requests to rebuild epoxy models are async and posted a frame later, so any changes do the adapter due to a click would take a frame to get notified (even longer if we add diffing)

[niccorder]

Thanks for the detailed feedback! I'll touch on the two topics here.

Blocking Clicks

The code change itself is minute, but the interesting aspect here is refactoring to allow for click tests to pass.

I'll throw up my code once I'm back on my laptop for your guidance, and to give clarity into what I mean.

Debouncing Clicks

I was planning on doing it the ButterKnife way originally but you bring up a great point with using the system clock + the extra required delay.

I think we can solve this by creating a custom handler and using a combination of both ideas. I think it would look like:

public class DebounceHandler extends Handler {
    private long debounceTime;
    private long lastPosted;

    @Override
    public void post(Runnable r) {
         // If lastPosted - currentTime < debounceTime -> remove message
         // Update last posted time.
         // Store runnable in map
         // PostDelay message by debounce time
   }
}

On my mobile phone and haven't thought it out filly yet but this might suffice

[elihart]

Closed this since it is fixed, lets take the debounce discussion to #305

[elihart]

A handler does synchronization and other in depth stuff under the hood to post, just checking the system clock might be good enough