RecyclerView causes issue when recycling
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() {
}
}
Solution 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.
Solution 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);
}
Solution 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