RecyclerView causes issue when recycling

Question

I have a list of items that I created using RecyclerView. When the user clicks on one of them I change the background color of that selected item. The problem is, when I scroll through my items, and they get recycled, some of the items get the selected item's background color (which is wrong). Here you can see my Adapter's code:

public class OrderAdapter extends RecyclerView.Adapter<OrderAdapter.ViewHolder> {

private static final String SELECTED_COLOR = "#ffedcc";

private List<OrderModel> mOrders;

public OrderAdapter() {
    this.mOrders = new ArrayList<>();
}

public void setOrders(List<OrderModel> orders) {
    mOrders = orders;
}

public void addOrders(List<OrderModel> orders) {
    mOrders.addAll(0, orders);
}

public void addOrder(OrderModel order) {
    mOrders.add(0, order);
}

@Override
public ViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
    Context context = parent.getContext();
    LayoutInflater inflater = LayoutInflater.from(context);

    // Inflate the custom layout
    View contactView = inflater.inflate(R.layout.order_main_item, parent, false);

    // Return a new holder instance
    ViewHolder viewHolder = new ViewHolder(contactView);
    return viewHolder;
}

@Override
public void onBindViewHolder(final ViewHolder viewHolder, final int position) {
    final OrderModel orderModel = mOrders.get(position);

    // Set item views based on the data model
    TextView customerName = viewHolder.customerNameText;

    SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MM/dd/yyyy'   'HH:mm:ss:S");
    String time = simpleDateFormat.format(orderModel.getOrderTime());
    customerName.setText(time);

    TextView orderNumber = viewHolder.orderNumberText;
    orderNumber.setText("Order No: " + orderModel.getOrderNumber());

    Button button = viewHolder.acceptButton;
    button.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            viewHolder.userActions.acceptButtonClicked(position);
        }
    });

    final LinearLayout orderItem = viewHolder.orderItem;
    orderItem.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            viewHolder.userActions.itemClicked(orderModel);
            viewHolder.orderItem.setBackgroundColor(Color.parseColor(SELECTED_COLOR));
        }
    });
}

@Override
public int getItemCount() {
    return mOrders.size();
}


public static class ViewHolder extends RecyclerView.ViewHolder implements OrderContract.View {

    public TextView customerNameText;
    public Button acceptButton;
    public TextView orderNumberText;
    public OrderContract.UserActions userActions;
    public LinearLayout orderItem;

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

        userActions = new OrderPresenter(this);

        customerNameText = (TextView) itemView.findViewById(R.id.customer_name);
        acceptButton = (Button) itemView.findViewById(R.id.accept_button);
        orderNumberText = (TextView) itemView.findViewById(R.id.order_number);
        orderItem = (LinearLayout) itemView.findViewById(R.id.order_item_selection);
    }

    @Override
    public void removeItem() {

    }
}

Answer 1

The problem is recyclerView recycling behavior which assign your out of screen ViewHolder items to new items coming to be displayed on screen. I would not suggest you to bind your logic based on ViewHolder object as in all above answers. It will really cause you problem. You should build logic based on the state of your data object not ViewHolder Object as you will never know when it gets recycled.

Suppose you save a state boolean isSelected in ViewHolder to check, but and if it is true, then the same state will be there for new Item when this viewHolder will be recycled.

Better way to do above is holding the any state in DataModel object. In your case just a boolean isSelected.

Sample example like

package chhimwal.mahendra.multipleviewrecyclerproject;

import android.content.Context;
import android.support.v7.widget.RecyclerView;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.support.v7.widget.CardView;
import android.widget.TextView;

import java.util.List;

/**
 * Created by mahendra.chhimwal on 12/10/2015.
 */
public class MyRecyclerViewAdapter extends RecyclerView.Adapter<MyRecyclerViewAdapter.ViewHolder> {

    private Context mContext;
    private List<DataModel> mRViewDataList;


    public MyRecyclerViewAdapter(Context context, List<DataModel> rViewDataList) {
        this.mContext = context;
        this.mRViewDataList = rViewDataList;
    }

    @Override
    public MyRecyclerViewAdapter.ViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
        LayoutInflater inflater = LayoutInflater.from(parent.getContext());
        View view = inflater.inflate(R.layout.item_recycler_view, parent, false);
        return new ViewHolder(view);
    }

    @Override
    public void onBindViewHolder(ViewHolder holder, int position) {
        holder.bindDataWithViewHolder(mRViewDataList.get(position));
    }

    @Override
    public int getItemCount() {
        return mRViewDataList != null ? mRViewDataList.size() : 0;
    }


    public class ViewHolder extends RecyclerView.ViewHolder {
        private TextView textView;
        private LinearLayout llView;
        private DataModel mDataItem=null;

        public ViewHolder(View itemView) {
            super(itemView);
            llView=(LinearLayout)itemView.findViewById(R.id.ll_root_view);
            textView = (TextView) itemView.findViewById(R.id.tvItemName);
            cvItemView.setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View v) {
                  // One should handle onclick of event here based on the dataItem i.e. mDataItem in this case.
                  // something like that..
                /* Intent intent = new Intent(mContext,ResultActivity.class);
                 intent.putExtra("MY_DATA",mDataItem);   //If you want to pass data.
                 intent.putExtra("CLICKED_ITEM_POSTION",getAdapterPosition()); // If one want to get selected item position
                 startActivity(intent);*/
                 Toast.makeText(mContext,"You clicked item number "+ViewHolder.this.getAdapterPosition(),Toast.LENTH_SHORT).show();
                }
            });
        }

        //This is clean method to bind data with viewHolder. Do all dirty things on View based on dataItem.
        //Must be called from onBindViewHolder(),with dataItem. In our case dataItem is String object.
        public void bindDataWithViewHolder(DataModel dataItem){
            this.mDataItem=dataItem;

            if(mDataItem.isSelected()){
                llView.setBackgroundColor(Color.ParseColor(SELCTED_COLOR);
            }else{
                llView.setBackgroundColor(Color.ParseColor(DEFAULT_COLOR);
            }
            //other View binding logics like setting text , loading image  etc.
            textView.setText(mDataItem);
        }
    }
}

As @Gabriel asked in comment,

what if one want to select a single item at time?

In that case, again one should not save selected item state in ViewHolder object, as the same it gets recycled and cause you problem. For that better way is have a field int selectedItemPosition in Adapter class not ViewHolder . Following code snippet show it.

public class MyRecyclerViewAdapter extends RecyclerView.Adapter<MyRecyclerViewAdapter.ViewHolder> {



        private Context mContext;
        private List<DataModel> mRViewDataList;

        //variable to hold selected Item position
        private int mSelectedItemPosition = -1;


        public MyRecyclerViewAdapter(Context context, List<DataModel> rViewDataList) {
            this.mContext = context;
            this.mRViewDataList = rViewDataList;
        }

        @Override
        public MyRecyclerViewAdapter.ViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
            LayoutInflater inflater = LayoutInflater.from(parent.getContext());
            View view = inflater.inflate(R.layout.item_recycler_view, parent, false);
            return new ViewHolder(view);
        }

        @Override
        public void onBindViewHolder(ViewHolder holder, int position) {
            holder.bindDataWithViewHolder(mRViewDataList.get(position),position);
        }

        @Override
        public int getItemCount() {
            return mRViewDataList != null ? mRViewDataList.size() : 0;
        }


        public class ViewHolder extends RecyclerView.ViewHolder {
            private TextView textView;
            private LinearLayout llView;
            private DataModel mDataItem=null;

            public ViewHolder(View itemView) {
                super(itemView);
                llView=(LinearLayout)itemView.findViewById(R.id.ll_root_view);
                textView = (TextView) itemView.findViewById(R.id.tvItemName);
                cvItemView.setOnClickListener(new View.OnClickListener() {
                    @Override
                    public void onClick(View v) {
                        //Handling for background selection state changed
                        int previousSelectState=mSelectedItemPosition;
                        mSelectedItemPosition = getAdapterPosition();
                        //notify previous selected item
                        notifyItemChanged(previousSelectState);
                        //notify new selected Item
                        notifyItemChanged(mSelectedItemPosition);

                        //Your other handling in onclick

                    }
                });
            }

            //This is clean method to bind data with viewHolder. Do all dirty things on View based on dataItem.
            //Must be called from onBindViewHolder(),with dataItem. In our case dataItem is String object.
            public void bindDataWithViewHolder(DataModel dataItem, int currentPosition){
                this.mDataItem=dataItem;
                //Handle selection  state in object View.
                if(currentPosition == mSelectedItemPosition){
                    llView.setBackgroundColor(Color.ParseColor(SELCTED_COLOR);
                }else{
                    llView.setBackgroundColor(Color.ParseColor(DEFAULT_COLOR);
                }
                //other View binding logics like setting text , loading image  etc.
                textView.setText(mDataItem);
            }
        }
    }

If you only have to maintain selected Item state, I strongly discourage use of notifyDataSetChanged() method of Adapter class as RecyclerView provides much more flexibility for these cases.

Answer 2

You should modify your logic assign the value inside the item (object) not the view:

orderItem.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
           orderItem.setSelected(xxxx);
        }
    });

Then in your onBindViewHolder method you have to assing the color according to this value in the item.

if (orderItem.isSelected()){
   viewHolder.orderItem.setBackgroundColor(xxxx);
} else {
  viewHolder.orderItem.setBackgroundColor(xxxx);
}

Answer 3

This is quite a common mistake that has an easy solution.

Quick answer: add this line in your onBindViewHolder method:

if (orderItem.isSelected()){
    viewHolder.orderItem.setBackgroundColor(Color.parseColor(SELECTED_COLOR));
} else {
    viewHolder.orderItem.setBackgroundColor(Color.parseColor(DEFAULT_COLOR));
}

(with DEFAULT_COLOR the color that the viewholder has by default)

Explained answer: when the system recycles a viewholder it just calls onBindViewHolder method so if you have changed anything of that viewholder you'll have to reset it. This will happen if you change background, item's position, etc. Any change that is not related to content per se should be reset in that method