Why does OnClickListener on a ViewHolder don't work?

Question

I'm trying to implement a way to handle item selection on a RecyclerView. I personally don't like the way suggested in some answers on SO of passing through gestures, and I thought that implementing an OnClickListener, as suggested here and here, was waaay cleaner.

The fact is that... this pattern doesn't actually work! I'm really not able to understand why my OnClickListener.onClick is never called. It's kinda like another method intercepts the click before onClick can take care of it.

This is my code:

    public class ViewHolder extends RecyclerView.ViewHolder implements View.OnClickListener {
        TextView tvName;
        ImageView star;

        public ViewHolder(View itemView) {
            super(itemView);

            tvName = (TextView) itemView.findViewById(R.id.CHAT_ITEM_name);
            star = (ImageView) itemView.findViewById(R.id.CHAT_ITEM_star);

            Fonts.setTypeface(tvName, regular);
        }

        @Override
        public void onClick(View view) {
            int position = getLayoutPosition();
            select(position);
        }
    }

Unfortunately it's very important for me to able to access the position of the clicked item in the whole dataset, in order to remove it, so doing something like indexOfChild isn't acceptable too: I tried, but this method gives you the position of the item in the visibile part of the list, thus making list.remove(position) impossible.

But it works. Can you post all the code for your ViewHolder so we can see what is missing? — mthandr, Jul 25 '15 at 13:43

score 64 · Accepted Answer · answered Jul 25 '15 at 13:52

64

Looking at the updated code: you are not setting the onClickListener to any of the views in the ViewHolder. It is an understandable mistake to forget the click listener.

Just use:

tvName.setOnClickListener(this);
star.setOnClickListener(this);

You can set to both or just one of them. You can also simply get the parent layout of these two views, so that the whole item itself in the adapter can be clickable.

itemView.setOnClickListener(this);

answered Jul 25 '15 at 13:52

mthandr

3,062
2
23
33

1

What a terrible design that you have to actually set this and just overriding `onClick` doesn't work. Very intuitive :-\ – t3chb0t Oct 21 '20 at 09:40

user2167877 · Answer 2 · 2018-01-16T02:42:21.593

3

You can do it in your onBindViewHolder

         @Override
        public void onBindViewHolder(ReportViewHolder holder, int position {
      holder.itemView.setOnClickListener(new View.OnClickListener() {
        @Override 
        public void onClick(View arg0) { 
        // handle your click here. 
    } }); 
}

edited Jan 16 '18 at 02:42

answered Jan 16 '18 at 02:36

user2167877

1,676
1
14
15

2

This method is not optimal for performance.!!! Click listeners more efficiantly setup in constructor of ViewHolder. – kovac777 Nov 21 '18 at 12:53
2

The onBindViewHolder() method is called every single time a new item is shown in the RecyclerView, so if you have a very large list you end up needlessly creating hundreds or even more click listeners. Creating the listeners in the ViewHolder limits the number of listeners created to a reasonable amount. (roughly the number of items on the screen) – Vishnu M. Dec 05 '18 at 17:02
1

I agree especially if the number of items is dynamic not static. – user2167877 Dec 06 '18 at 17:34

score 0 · Answer 3 · answered May 24 '20 at 11:33

Simplely Click Handler your ViewHolder. Recycler View don't have special attaching click handlers like ListView which has the method setOnItemClickListener().

** public class ViewHolder extends RecyclerView.ViewHolder implements View.OnClickListener

** in public ViewHolder(Context context, View itemView) set public void onClick(View view)

** get position by: int position = getLayoutPosition(); User user = users.get(position);

Why does OnClickListener on a ViewHolder don't work?

3 Answers3

Linked

Related