viewholder init for onItemClick is not working

Question

I am researching on how to add an onClick event on my recyclerview properly,

currently I am using the interface inside my customAdapter

class CategoryAdapter(val categoryList : List<CategoryObject>, val context: Context, val mItemClickListener: MainInterface) : RecyclerView.Adapter<CategoryAdapter.ViewHolder>() {

interface MainInterface {
    fun onCategoryItemClick(categoryKey: Int)
}

override fun getItemCount(): Int {
    return categoryList.size
}

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder {
    return ViewHolder(LayoutInflater.from(context).inflate(R.layout.listview_category, parent, false))
}

override fun onBindViewHolder(holder: ViewHolder, position: Int) {
    holder?.categoryName?.text = categoryList[position].categoryName
//        holder?.categoryName.setOnClickListener{
//            mItemClickListener.onCategoryItemClick(position)
//        }
//        holder?.categoryName?.setOnClickListener { listener(categoryList[position]) }
}

inner class ViewHolder (view: View) : RecyclerView.ViewHolder(view) {
    val categoryName = view.lv_txt_category_name

    init {
        view.setOnClickListener {
            mItemClickListener.onCategoryItemClick(adapterPosition)
        }
    }
}

}

viewHolder's onClick does not register on my activity ovveride function.

but putting the onClick inside onBindViewHolder works perfectly,

I'm not sure which is more efficient, if the onBindViewHolder onClick is the right answer, then I'll stick with it, but if the viewHolder is the right one, why it does not work properly?

Thanks in advance!

Update
This is the stackoverflow post I'm using to research things

RecyclerView itemClickListener in Kotlin

have you tried to set click listener on itemView instead of view??? — hamid_c, Feb 27 '20 at 20:11
you mean you used `itemView.setOnClickListener { }` instead of `view.setOnClickListener { }` in `init` but the result was the same??? — hamid_c, Feb 27 '20 at 20:17
@hamid_c, yes, but still, my override function in activity is not called. — MaChee Neraid, Feb 27 '20 at 20:18
can you add a `Log` in your `view.setOnClickListener` and check it in Logcat to see whether it get called or not??? — hamid_c, Feb 27 '20 at 20:23
I am also using breakpoint to see if it will stop on my click. but it won't stop. btw, I updated my post, I included the post that I am using as my reference. — MaChee Neraid, Feb 27 '20 at 20:26

score 0 · Answer 1 · answered Feb 27 '20 at 20:19

0

tried android:clickable="true" in your xml?

answered Feb 27 '20 at 20:19

Viatcheslav Ehrmann

716
2
5
11

I think you don't need to add this but it won't harm to try, still... not working – MaChee Neraid Feb 27 '20 at 20:23

score 0 · Answer 2 · answered Feb 27 '20 at 20:23

0

As the documentation suggest for:

RecyclerView.Adapter.html#onBindViewHolder

Called by RecyclerView to display the data at the specified position. This method should update the contents of the itemView to reflect the item at the given position.

And for the RecyclerView.Adapter.html#onCreateViewHolder

Called when RecyclerView needs a new RecyclerView.ViewHolder of the given type to represent an item.

This new ViewHolder should be constructed with a new View that can represent the items of the given type. You can either create a new View manually or inflate it from an XML layout file.

As onCreateViewHolder is a method where we return the type of ViewHolder only while in onBindViewHolder we update or initialise the data to display.

It means no data is binded in onCreateViewHolder and binding anything in this method will have no effect.

answered Feb 27 '20 at 20:23

Waqar UlHaq

6,144
2
34
42

so, does it means even I put my onClicklistener inside my OnBindViewHolder, it won't create memory-leak-ish events?? – MaChee Neraid Feb 27 '20 at 20:27
@MaCheeNeraid very nice question indeed. Although many articles/blogs use this approach and I would say it's a best practice to set clickListener's in `onBindViewHolder` as there are hardly any chances of memory leak unless the listener is defined as `static`. I would suggest you, to go over a very nice explanation about this in the following link https://stackoverflow.com/questions/35086967/how-can-an-asynctask-still-use-an-activity-if-the-user-has-already-navigated-awa/35087188#35087188 – Waqar UlHaq Feb 27 '20 at 20:36
Thanks for the link! just to add, actually I also used the lambda expression "(category) -> Unit?". This is the quickest and simpliest way but android profiler shows that my activity is not garbage collected even I forced it. That's why I'm avoiding the lambda. – MaChee Neraid Feb 27 '20 at 20:40
The RecyclerView Adapter is not going to leak your Activity if it is referenced only by the Activity itself. A lambda compiles to an implementation of an interface, so that wasn't the cause of your leak, if you indeed had one. – Tenfour04 Feb 27 '20 at 21:14
I don't think this explanation is the reason for it not working. There is nothing in RecyclerView.Adapter that is going to go through all the views and remove click listeners that you have set before `onBindView`. – Tenfour04 Feb 27 '20 at 21:37

viewholder init for onItemClick is not working

2 Answers2