airbnb / epoxy

Epoxy is an Android library for building complex screens in a RecyclerView
https://goo.gl/eIK82p
Apache License 2.0
8.51k stars 728 forks source link

ClassCastException: click listener on EpoxyModel within a EpoxyModelGroup #687

Closed eboudrant closed 5 years ago

eboudrant commented 5 years ago

I am not sure it is a new or old issue but when we set a click listener on an EpoxyModel which is part of an EpoxyModelGroup we receive ClassCastException on click :

java.lang.ClassCastException: x.x.ModelGroup cannot be cast to x.x.NestedModel
    at x.x.EpoxyController$buildModels$$inlined$apply$lambda$1.onClick(EpoxyController.kt:274)
    at com.airbnb.epoxy.WrappedEpoxyModelClickListener.onClick(WrappedEpoxyModelClickListener.java:52)
    at android.view.View.performClick(View.java:6597)
    at android.view.View.performClickInternal(View.java:6574)
        at ...
val nestedModel = ModelGroup_()
    .id(...)
    .imageClickListener { model, _, clickedView, _ ->
        ...
    }

// then nestedModel get added to an EpoxyModelGroup
elihart commented 5 years ago

Thanks for the report, I think this has existed for a long time. It will only happen if you use the model click listener.

The code in WrappedEpoxyModelClickListener does

@Override
  public void onClick(View v) {
    EpoxyViewHolder epoxyHolder = getEpoxyHolderForChildView(v);
    if (epoxyHolder == null) {
      throw new IllegalStateException("Could not find RecyclerView holder for clicked view");
    }

    final int adapterPosition = epoxyHolder.getAdapterPosition();
    if (adapterPosition != RecyclerView.NO_POSITION) {
      //noinspection unchecked
      originalClickListener
          .onClick((T) epoxyHolder.getModel(), (V) epoxyHolder.objectToBind(), v, adapterPosition);
    }
  }

which assumes that the top level view/model should be used. It needs to be updated to check for group handling somehow.

I will try to look into it soon, unless someone else can take it

elihart commented 5 years ago

Potentially the generated code needs to change the WrappedEpoxyModelClickListener constructor to also take the model it is associated with, then when we find a model group we can check whether the listener's model is the top level group, or any nested models (potentially recursively)

TheLester commented 5 years ago

Any update on this?

elihart commented 5 years ago

No, but I'd love to see somebody put up a PR.

It may be as simple as modifying WrappedEpoxyModelClickListener to check if the top level view holder model is a group and if so pulling out the right child model

roncbird commented 5 years ago

We've started using MvRx and Epoxy for a new app we're building here at work, and the libraries are great, but I just recently received an feature request that requires a fairly complicated UI design for rows, which has required me to use an EpoxyModelGroup. We are using databindings so I've added the clickListener in the xml. This has caused me to run into this ClassCastException error as well, so I wanted to check and see if there is a fix in the works for this, or should I avoid using the model onClickListener?

elihart commented 5 years ago

Thanks for the reminder, just fixed it finally https://github.com/airbnb/epoxy/pull/767

roncbird commented 5 years ago

Oh wow! Awesome. Thank you so much for the quick reply and fix. I really appreciate it.