RecyclerView checks wrong items as i scroll down

Question

I decided to use RecyclerView to display my list of data. There's a checkbox, 2 TextViews and one ImageView in each item/row. I have created my Adapter to handle everything, and to add the checked items to my final list.

The problem is that when I check a few items and scroll down some of the items will appear as checked and when I scroll up the ones I had checked previously appears as unchecked. The Images and the Textviews are OK, but the checkbox messes up when scrolling down and up.

This is my Adapter:

public class TeamAdapter extends RecyclerView.Adapter<TeamAdapter.MyViewHolder> {

    private Context mContext;
    private List<UserProject> UserList;
    private UserProject user;


    public class MyViewHolder extends RecyclerView.ViewHolder implements View.OnClickListener{
        public TextView name, description;
        public CircleImageView profile_image;
        public CheckBox check;

        public MyViewHolder(View view) {
            super(view);
            name = (TextView) view.findViewById(R.id.name);
            description = (TextView) view.findViewById(R.id.description);
            profile_image = (CircleImageView) view.findViewById(R.id.profile_image);
            check = (CheckBox) view.findViewById(R.id.check);
        }
        @Override
        public void onClick(View v) {
            if (mItemClickListener != null) {
                mItemClickListener.onItemClickListener(v, getAdapterPosition());
            }
        }
    }


    public TeamAdapter(Context mContext, List<UserProject> userList) {
        this.mContext = mContext;
        this.UserList = userList;

    }
    @Override
    public TeamAdapter.MyViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
        View itemView = LayoutInflater.from(parent.getContext())
                .inflate(R.layout.adapter_teams, parent, false);

        return new MyViewHolder(itemView);
    }

    @Override
    public void onBindViewHolder(final TeamAdapter.MyViewHolder holder, final int position) {

        user = UserList.get(position);

        holder.name.setText(user.getName());
        holder.description.setText(user.getCompany());

        if(user.getImageURL() != null && !user.getImageURL().isEmpty()) {
            Picasso.with(mContext).load(user.getImageURL()).into(holder.profile_image);
        }

        holder.check.setOnCheckedChangeListener(new CompoundButton.OnCheckedChangeListener() {
            @Override
            public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
                if(isChecked){
                    ActivityTeams.newList.add(UserList.get(position));
                }else {
                    ActivityTeams.newList.remove(UserList.get(position));
                }
            }
        });

    }


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

    private TeamAdapter.onRecyclerViewItemClickListener mItemClickListener;

    public void setOnItemClickListener(TeamAdapter.onRecyclerViewItemClickListener mItemClickListener) {
        this.mItemClickListener = mItemClickListener;
    }

    public interface onRecyclerViewItemClickListener {
        void onItemClickListener(View view, int position);
    }
}

Possible duplicate of [CheckBox in RecyclerView keeps on checking different items](https://stackoverflow.com/questions/32427889/checkbox-in-recyclerview-keeps-on-checking-different-items) — TofferJ, Sep 14 '17 at 20:17
It didn't work for me using the solutions from this question. — Marcos Guimaraes, Sep 14 '17 at 20:24

Dave Thomas · Accepted Answer · 2017-09-14T20:46:57.783

As in the comment about the duplicate answer, your holder is retaining the check box view. You just need to update the check boxes state based on your backing data.

Add this before assigning the check listener in onBindViewHolder:

holder.check.setOnCheckedChangeListener(null);    
holder.check.setChecked(ActivityTeams.newList.contains(UserList.get(position));
holder.check.setOnCheckedChangeListener(new ... "you know what to do here"

Based on CompoundButton source code from here https://github.com/android/platform_frameworks_base/blob/master/core/java/android/widget/CompoundButton.java setChecked:

/**
 * <p>Changes the checked state of this button.</p>
 *
 * @param checked true to check the button, false to uncheck it
 */
@Override
public void setChecked(boolean checked) {
    if (mChecked != checked) {
        mCheckedFromResource = false;
        mChecked = checked;
        refreshDrawableState();
        notifyViewAccessibilityStateChangedIfNeeded(
                AccessibilityEvent.CONTENT_CHANGE_TYPE_UNDEFINED);

        // Avoid infinite recursions if setChecked() is called from a listener
        if (mBroadcasting) {
            return;
        }

        mBroadcasting = true;
        if (mOnCheckedChangeListener != null) {
            mOnCheckedChangeListener.onCheckedChanged(this, mChecked);
        }
        if (mOnCheckedChangeWidgetListener != null) {
            mOnCheckedChangeWidgetListener.onCheckedChanged(this, mChecked);
        }
        final AutofillManager afm = mContext.getSystemService(AutofillManager.class);
        if (afm != null) {
            afm.notifyValueChanged(this);
        }

        mBroadcasting = false;
    }
}

As you can see calling setChecked also calls the currently attached listener.

Thanks for the reply. I added this line to the Adapter, now it's not checking randomly anymore but it keeps adding the items to my list when I scroll down and back up. — Marcos Guimaraes, Sep 14 '17 at 20:32
I hope you ignored my comment before I edited. To optimize out the setting of the listener requires more work, please ignore that part. You didn't implement that too right? Because it was broken wrong code. — Dave Thomas, Sep 14 '17 at 20:36
I only added this line after the setOnCheckedChangeListener method: holder.check.setChecked(ActivityTeams.newList.contains(UserList.get(position)); — Marcos Guimaraes, Sep 14 '17 at 20:38
I managed to fix that by only adding a new object to the list if it's not already there. Thanks for your answer, it helped me! — Marcos Guimaraes, Sep 14 '17 at 20:40
Yup that makes sense, just realized that is broken too. That will cause your listener to fire. — Dave Thomas, Sep 14 '17 at 20:40

score 2 · Answer 2 · answered Sep 14 '17 at 20:29

You did not specify whether the CheckBox is checked or not inside your onBindViewHolder() method. Hence, the RecyclerView has no way of knowing whether the CheckBox is checked or not when Views are created/recycled.

You could try saving the checked/unchecked state for each corresponding CheckBox inside a separate ArrayList or merge with your UserProject class and keep the info there.

After that, you can simply say holder.check.setChecked(...); Where the ... is your condition for the CheckBox.

RecyclerView checks wrong items as i scroll down

2 Answers2