ViewHolder Layout Updates don't apply from onBindViewHolder()

Question

The setup I've got a RecyclerView with a custom adapter and custom ViewHolders.

I understand the onCreateViewHolder() method is called when the RecyclerView is running out of ViewHolders to recycle and needs a new one. So I'm just inflating a layout in there and passing it to a new ViewHolder.

Furthermore, onBindViewHolder() is responsible for filling the ViewHolder with data as soon as a new ViewHolder has been created or recycled by the RecyclerView. So what I'm doing in there is calling my method holder.setNode() to pass a data object to the ViewHolder.

The behavior I'm seeing When the activity first launches, all entries are correct. When I'm adding new entries or deleting existing ones, however, things start to get a bit funny.

• the title TextView is always set correctly
• the background color of the main layout changes seemingly at will, I'm assuming because the RecyclerView is reusing old ones
• as does the custom view I have implemented, even though I'm invalidating it and passing it new values which change its appearance noticeably

So I'm wondering: Why aren't those values changed in onBindViewHolder() as soon as views get reused? Or if I'm wrong, what's the real reason for the random switching of layouts?

---

TaskListAdapter

class TaskListAdapter extends RecyclerView.Adapter<TaskListAdapter.TaskViewHolder> {

private ArrayList<NodeHandler.DbNode> dbNodeList = new ArrayList<>();
...

@Override
public TaskViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
    View v = LayoutInflater.from(parent.getContext()).inflate(R.layout.small_task_view, parent, false);
    return new TaskViewHolder(v);
}

@Override
public void onBindViewHolder(TaskViewHolder holder, int position) {
    final NodeHandler.DbNode dbNode = dbNodeList.get(position);
    holder.setNode(dbNode);
    holder.wrapper.findViewById(R.id.card_details).setVisibility(View.GONE);
}

...

public static class TaskViewHolder extends RecyclerView.ViewHolder implements ItemTouchHelperViewHolder {
    private FrameLayout wrapper;
    private TextView title;
    private NodeHandler.DbNode dbNode;

    public TaskViewHolder(View view) {
        ...
    }

    public void setTitle(String str) {
        title.setText(str);
    }

    public void setMarkers(@IntRange(from = 1, to = Node.MAX_URGENCY) int urgency, @IntRange(from = 1, to = Node.MAX_IMPORTANCE) int importance) {
        if(!dbNode.isAppointment()) {
            wrapper.setBackgroundColor(ContextCompat.getColor(wrapper.getContext(), R.color.lightGray));
        }
        ((QuadrantView) wrapper.findViewById(R.id.quadrant_view)).setDimensions(importance, urgency);
        // setDimensions will invalidate the view
    }

    public void setNode(NodeHandler.DbNode dbNodeObject) {
        this.dbNode = dbNodeObject;
        setTitle(dbNode.toString());
        setMarkers(dbNode.getUrgency(), dbNode.getImportance());
        setTips();
    }
}
}

Let me know if anything else could matter here. I'd be happy to update the question accordingly.

Answer 1

Values are indeed changed in onBindViewHolder as soon as views get reused.

The real reason for the seemingly random switching of layouts is that onBindViewHolder is currently implemented in a way that assumes that the ViewHolder was freshly created and is being bound for its first time. onBindViewHolder should instead be implemented in a way that assumes that the ViewHolder being bound is being reused so it should either:

1. reset all the values of the ViewHolder to default values first before setting them to other values or
2. make sure that everything is set inside onBindViewHolder, so one cannot tell that it was ever previously bound to something else.

Random background color changes:

You are right for suspecting that the random background color problem is caused by the RecyclerView reusing ViewHolders.

The reason why this is happening is because of the following code:

    if(!dbNode.isAppointment()) {
        wrapper.setBackgroundColor(ContextCompat.getColor(wrapper.getContext(), R.color.lightGray));
    }

It only sets the background if the ViewHolder is not an appointment. So if a ViewHolder that is being reused was previously not for an appointment, but is currently for one that is now an appointment, it's background color will be inappropriate.

to fix this, do any of the following:

• set the background color of the ViewHolder to some default color before the if statement is executed (as per solution 1 mentioned above):

  wrapper.setBackgroundColor(/* default background color */);
  if(!dbNode.isAppointment()) {
      wrapper.setBackgroundColor(ContextCompat.getColor(wrapper.getContext(), R.color.lightGray));
  }
  

• add an else block to the if statement to set the background color of the ViewHolder to the appropriate color (as per solution 2 mentioned above)

  if(!dbNode.isAppointment()) {
      wrapper.setBackgroundColor(ContextCompat.getColor(wrapper.getContext(), R.color.lightGray));
  }
  else
  {
      wrapper.setBackgroundColor(/* appointment background color */);
  }
  

• override the RecyclerView.Adapter's getItemViewType to return different view types based on dbNode.isAppointment(), and create different ViewHolder subclasses for displaying each of them

p.s. I don't know what the problem could be regarding the custom views...sorry