27

I have a list with 13 items (although items may be added or removed), positions 0-12. When the fragment containing the RecyclerView is first shown, only positions 0 through 7 are visible to the user (position 7 being only half visible). In my adapter I Log every time a view holder is binded/bound (idk if grammar applies here) and record its position.

Adapter

@Override
public void onBindViewHolder(final ViewHolder holder, final int position) {
    Log.d(TAG, "onBindViewHolder() position: " + position);
    ...
}

From my Log I see that positions 0-7 are bound:

Log from Adapter

I have a selectAll() method that gets each ViewHolder by adapter position. If the returned holder is NOT null I use the returned holder to update the view to show it's selected. If the returned holder IS null I call selectOnBind() a method that flags the view at that position update to show it's selected when it's binded rather than in real time since it's not currently shown:

public void selectAll() {
    for (int i = 0; i < numberOfItemsInList; i++) {
        MyAdapter.ViewHolder holder = (MyAdapter.ViewHolder)
                mRecyclerView.findViewHolderForAdapterPosition(i);

        Log.d(TAG, "holder at position " + i + " is " + holder);

        if (holder != null) {
            select(holder);
        } else {
            selectOnBind(i);
        }
    }
}

In this method I Log the holder along with its position:

Log from selectAll()

So up to this point everything seems normal. We have positions 0-7 showing, and according to the Log these are the positions bound. When I hit selectAll() without changing the visible views (scrolling) I see that positions 0-7 are defined and 8-12 are null. So far so good.

Here's where it gets interesting. If after calling selectAll() I scroll further down the list positions 8 and 9 do not show they are selected.

When checking the Log I see that it's because they are never bound even though they were reported to be null:

Adapter Log after scroll

Even more confusing is that this does not happen every time. If I first launch the app and test this it may work. But it seems to happen without fail afterwards. I'm guessing it has something to do with the views being recycled, but even so wouldn't they have to be bound?

EDIT (6-29-16)
After an AndroidStudio update I cannot seem to reproduce the bug. It works as I expected it to, binding the null views. If this problem should resurface, I will return to this post.

young_souvlaki
  • 1,886
  • 4
  • 24
  • 28
  • it's not a good practice to select your row like you did ( and it's not working ), better approach is have a list ( or map ) of a select item, then in `onBind` check position is exists in your list or not. if it's exists change row to selected, else show normal row. – Shayan Pourvatan Jun 17 '16 at 16:09
  • That is what I have. But if you only have them update in `onBind` the user will not see the changes on the items visible until they are binded again (which may be `onResume` or scroll). I omitted excess code as to not complicate the question. Although I would like to find an even cleaner way than how I have it and you've mentioned. – young_souvlaki Jun 17 '16 at 17:06
  • you can call `notifyDataSetChanged` or `notifyItemChanged` to call `onBind` manually – Shayan Pourvatan Jun 17 '16 at 18:02
  • @shayanpourvatan thank you but I do not think that is a good solution. It is noted in the documentation to call `notifyDataSetChanged()` as a last result. And to use one of the more granular methods I would need to know which are not binded every time which is not a constant number. Either way it doesn't really solve the problem it just works around it. – young_souvlaki Jun 17 '16 at 18:33
  • I encountered this same problem and it appears I had the setHasStableIds option set to true. – Neon Warge Jul 05 '18 at 07:16

6 Answers6

20

This is happening because:

  • The views are not added to the recyclerview (getChildAt will not work and will return null for that position)
  • They are cached also (onBind will not be called)

Calling recyclerView.setItemViewCacheSize(0) will fix this "problem".

Because the default value is 2 (private static final int DEFAULT_CACHE_SIZE = 2; in RecyclerView.Recycler), you'll always get 2 views that will not call onBind but that aren't added to the recycler

Pedro Oliveira
  • 20,442
  • 8
  • 55
  • 82
  • recyclerView.setItemViewCacheSize(0) is not a good practice since we lose performance improvement. – Tin Tran Jun 28 '16 at 15:49
  • Yes I agree with @TinTran. Please see edit above and thank you for your contribution. – young_souvlaki Jun 29 '16 at 11:50
  • 2
    This was just an explanation of why this is happening. It seems weird that upgrading AndroidStudio changed a behavior that it's happening since recyclerview came out. – Pedro Oliveira Jun 29 '16 at 16:41
  • This is the correct answer to my problem. The issue was greatly describe here [link](https://stackoverflow.com/questions/49460762/whats-the-difference-between-recyclerview-setitemviewcachesize-and-recycledview/49460900#49460900) which helps with a good explaination. – Mihae Kheel Sep 28 '19 at 14:27
10

In your case views for positions 8 and 9 are not being recycled, they are being detached from the window and will be attached again. And for these detached view onBindViewHolder is not called, only onViewAttachedToWindow is called. If you override these function in your adapter, you can see what I am talking.

@Override
    public void onViewRecycled(ViewHolder vh){
        Log.wtf(TAG,"onViewRecycled "+vh);
    }

    @Override
    public void onViewDetachedFromWindow(ViewHolder viewHolder){
        Log.wtf(TAG,"onViewDetachedFromWindow "+viewHolder);
    }

Now in order to solve your problem you need to keep track of the views which were supposed to recycled but get detached and then do your section process on

@Override
    public void onViewAttachedToWindow(ViewHolder viewHolder){
        Log.wtf(TAG,"onViewAttachedToWindow "+viewHolder);
    }
Zaartha
  • 1,106
  • 9
  • 25
6

The answers by Pedro Oliveira and Zartha are great for understanding the problem, but I don't see any solutions I'm happy with.

I believe you have 2 good options depending on what you're doing:

Option 1

If you want onBindViewHolder() to get called for an off-screen view regardless if it's cached/detached or not, then you can do:

RecyclerView.ViewHolder view_holder = recycler_view.findViewHolderForAdapterPosition( some_position );

if ( view_holder != null )
{
    //manipulate the attached view
}
else //view is either non-existant or detached waiting to be reattached
    notifyItemChanged( some_position );

The idea is that if the view is cached/detached, then notifyItemChanged() will tell the adapter that view is invalid, which will result in onBindViewHolder() getting called.

Option 2

If you only want to execute a partial change (and not everything inside onBindViewHolder()), then inside of onBindViewHolder( ViewHolder view_holder, int position ), you need to store the position in the view_holder, and execute the change you want in onViewAttachedToWindow( ViewHolder view_holder ).

I recommend option 1 for simplicity unless your onBindViewHolder() is doing something intensive like messing with Bitmaps.

Kacy
  • 3,330
  • 4
  • 29
  • 57
1

When you have large number of items in the list you have passed to recyclerview adapter you will not encounter the issue of onBindViewHolder() not executing while scrolling.

But if the list has less items(I have checked on list size 5) you may encounter this issue.

Better solution is to check list size.

Please find sample code below.

private void setupAdapter(){
    if (list.size() <= 10){
        recycler.setItemViewCacheSize(0);
     }
     recycler.setAdapter(adapter);
     recycler.setLayoutManager(linearLayoutManager);
}
0

I think playing with view is not a good idea in recyclerview. The approach I always use to follow to just introduce a flag to the model using for RecyclerView. Let assume your model is like -

class MyModel{
    String name;
    int age;
}

If you are tracking the view is selected or not then introduce one boolean to the model. Now it will look like -

class MyModel{
    String name;
    int age;
    boolean isSelected;
}

Now your check box will be selected/un-selected on the basis of the new flag isSelected (in onBindViewHolder() ). On every selection on view will change the value of corresponding model selected value to true, and on unselected change it to false. In your case just run a loop to change all model's isSelected value to true and then call notifyDataSetChanged().

For Example, let assume your list is

ArrayList<MyModel> recyclerList;
private void selectAll(){
    for(MyModel myModel:recyclerList)
        myModel.isSelected = true;
    notifyDataSetChanged();
}

My suggestion, while using recyclerView or ListView to less try to play with views.

So in your case -

@Override
public void onBindViewHolder(final ViewHolder holder, final int position) {
   holder.clickableView.setTag(position);
   holder.selectableView.setTag(position);
   holder.checkedView.setChecked(recyclerList.get(position).isSelected);
    Log.d(TAG, "onBindViewHolder() position: " + position);
    ...
}

@Override
public void onClick(View view){
    int position = (int)view.getTag();
    recyclerList.get(position).isSelected = !recyclerList.get(position).isSelected;
}

@Override
public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
      int position = (int)buttonView.getTag();
      recyclerList.get(position).isSelected = isChecked;
}

Hope it will help you, Please let me know if you need any further explanation :)

Neo
  • 3,546
  • 1
  • 24
  • 31
  • This would work except what about when a user selects a view that is visible and thus, already binded? – young_souvlaki Jun 29 '16 at 12:15
  • bind the position with every clickable view (Using setTag) in onBindViewHolder(). So whenever you get a callback of onClick or onChangeListner just get the tagged position and change in the corresponding model. – Neo Jun 29 '16 at 12:23
  • I apologize but I'm not sure I follow – young_souvlaki Jun 29 '16 at 13:31
  • @YoungCoconutCode I will repeat again, always try to play with your data structure rather than playing views in adapters, it's among best code practices. Because adapter reuses the views always, in this case anything that is not connected to your data structure will either repeat or lost in case of re-use of particular view. – Neo Jun 29 '16 at 13:44
  • No I got that and I agree that would clean up my code a lot, but what I'm not clear on is how you propose to update an already binded view that has already gone through `onBind` and thus will not be checked for `isSelected()` after being selected – young_souvlaki Jun 29 '16 at 14:16
  • Okay got your problem, I have updated my post as per your usage. You need to just map as per your code. Let me know if you need more clarity :) – Neo Jun 29 '16 at 14:29
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/116046/discussion-between-neo-and-youngcoconutcode). – Neo Jun 30 '16 at 07:09
0

So I think you question is answered below by @Pedro Oliveira. The main sense of RecycleView, that he using special algorithms for caching ViewHolder in any time. So next onBindViewHolder(...) may not work, for ex. if view is static, or something else.

And about your question you think to use RecycleView for dynamic changed Views. DON'T DO IT! Because RecycleView invalidates views and has caching system, so you will have a lot of problems.

Use LinkedListView for this task!

GensaGames
  • 5,538
  • 4
  • 24
  • 53