Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PagedListAdapter with multiple ViewHolders scroll bug #375

Closed
kamgurgul opened this issue May 25, 2018 · 9 comments
Closed

PagedListAdapter with multiple ViewHolders scroll bug #375

kamgurgul opened this issue May 25, 2018 · 9 comments

Comments

@kamgurgul
Copy link

kamgurgul commented May 25, 2018

Hello,
I tried to add custom header (as a separeated ViewHolder) to the RecyclerView with PagedListAdapter but in that configuration loadInitial() (from DataSource) will caused automaticall scroll to the end of the loaded page (and it will trigger next load with loadRange()). This behaviour can be easily reproduced by changing this line from PostAdapter in PagingWithNetworkSample: private fun hasExtraRow() = true // networkState != null && networkState != NetworkState.LOADED.

I assume it is caused by wrong insertion notification from AsyncPagedListDiffer. Position on the data list is not the same as in the adapter because there I have one extra item.

How can I handle correctly this situation? Maybe some method in PagedListAdapter to calculate real data position would be helpful.

@ChrisCraik
Copy link
Contributor

You're right about the insert notification - If you want items above the PagedListAdapter, switch to using AsyncPagedListDiffer, and pass a custom ItemCallback that offsets the positions by one:

    final AdapterListUpdateCallback adapterCallback
            = new AdapterListUpdateCallback(/*Adapter*/ this);
    new AsyncPagedListDiffer<>(new ListUpdateCallback() {
        @Override
        public void onInserted(int position, int count) {
            adapterCallback.onInserted(position + 1, count);
        }

        @Override
        public void onRemoved(int position, int count) {
            adapterCallback.onRemoved(position + 1, count);
        }

        @Override
        public void onMoved(int fromPosition, int toPosition) {
            adapterCallback.onMoved(fromPosition + 1, toPosition + 1);
        }

        @Override
        public void onChanged(int position, int count, Object payload) {
            adapterCallback.onChanged(position + 1, count, payload);
        }
    }, config);

@kamgurgul
Copy link
Author

Working like a charm. I missed that overload.
Thank you.

@lucandr
Copy link

lucandr commented Feb 5, 2019

Hi guys @kamgurgul @ChrisCraik , I am having this problem too and I don't know how to solve it, could anyone help me with the code ?
I get the list automatically scrolled to the end of the loaded page.
I have this adapter with this three ViewHolders:

public class HomeAdapter extends PagedListAdapter<HomeItem, RecyclerView.ViewHolder> {

    private NetworkState networkState;
    private RetryCallback retryCallback;
    private RecommendedAdapter recommendedAdapter;

    @Inject
    public HomeAdapter() {
        super(feedDiffUtil);
    }

    public void setRetryCallback(RetryCallback retryCallback) {
        this.retryCallback = retryCallback;
    }

    public void setRecommendedAdapter(RecommendedAdapter recommendedAdapter) {
        this.recommendedAdapter = recommendedAdapter;
    }

    @NonNull
    @Override
    public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
        switch (viewType) {
            case R.layout.item_question:
                return QuestionsViewHolder.create(parent);
            case R.layout.view_network_state:
                return NetworkStateViewHolder.create(parent, retryCallback);
            case R.layout.item_list_recommended:
                return RecommendedViewHolder.Companion.create(parent);
            default:
                throw new IllegalArgumentException("unknown view type");
        }
    }

    @Override
    public void onBindViewHolder(@NonNull RecyclerView.ViewHolder holder, int position) {
        switch (getItemViewType(position)) {
            case R.layout.item_question:
                ((QuestionsViewHolder) holder).bindTo((QuestionResponse) getItem(position));
                break;
            case R.layout.view_network_state:
                ((NetworkStateViewHolder) holder).bindTo(networkState);
                break;
            case R.layout.item_list_recommended:
                ((RecommendedViewHolder) holder).bindTo((Recommended) getItem(position), recommendedAdapter);
                break;
        }
    }

    private boolean hasExtraRow() {
        return networkState != null && networkState != NetworkState.LOADED;
    }

    @Override
    public int getItemViewType(int position) {
        if (hasExtraRow() && position == getItemCount() - 1) {
            return R.layout.view_network_state;
        } else if(position == 0){
            return R.layout.item_list_recommended;
        } else {
            return R.layout.item_question;
        }
    }

    @Override
    public int getItemCount() {
        return super.getItemCount() + (hasExtraRow() ? 1 : 0);
    }

    public void setNetworkState(NetworkState newNetworkState) {
        NetworkState previousState = this.networkState;
        boolean hadExtraRow = hasExtraRow();
        this.networkState = newNetworkState;
        boolean hasExtraRow = hasExtraRow();
        if (hadExtraRow != hasExtraRow) {
            if (hadExtraRow) {
                notifyItemRemoved(super.getItemCount());
            } else {
                notifyItemInserted(super.getItemCount());
            }
        } else if (hasExtraRow && previousState != newNetworkState) {
            notifyItemChanged(getItemCount() - 1);
        }
    }

    @Override
    public void submitList(PagedList<HomeItem> pagedList) {
        super.submitList(pagedList);
    }

    @Override
    public void onCurrentListChanged(@Nullable PagedList<HomeItem> currentList) {
        super.onCurrentListChanged(currentList);
    }

    private static DiffUtil.ItemCallback<HomeItem> feedDiffUtil = new DiffUtil.ItemCallback<HomeItem>() {

        @Override
        public boolean areItemsTheSame(HomeItem oldItem, HomeItem newItem) {
            return oldItem == newItem;
        }

        @Override
        public boolean areContentsTheSame(HomeItem oldItem, HomeItem newItem) {
            return Objects.equals(oldItem, newItem);
        }
    };
}

@lucandr
Copy link

lucandr commented Feb 5, 2019

The scrolling problem is fixed with this:

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

    private NetworkState networkState;
    private RetryCallback retryCallback;
    private RecommendedAdapter recommendedAdapter;

    private AsyncPagedListDiffer<HomeItem> mDiffer;

    @Inject
    public HomeAdapterNew() {
        final AdapterListUpdateCallback adapterCallback = new AdapterListUpdateCallback(this);
        mDiffer = new AsyncPagedListDiffer<>(new ListUpdateCallback() {
            @Override
            public void onInserted(int i, int i1) {
                adapterCallback.onInserted(i + 1, i1);
            }

            @Override
            public void onRemoved(int i, int i1) {
                adapterCallback.onRemoved(i + 1, i1);
            }

            @Override
            public void onMoved(int i, int i1) {
                adapterCallback.onMoved(i + 1, i1 + 1);
            }

            @Override
            public void onChanged(int i, int i1, @Nullable Object o) {
                adapterCallback.onChanged(i + 1, i1, o);
            }
        }, new AsyncDifferConfig.Builder<HomeItem>(new DiffUtil.ItemCallback<HomeItem>() {
            @Override
            public boolean areItemsTheSame(@NonNull HomeItem homeItem, @NonNull HomeItem t1) {
                return false;
            }

            @Override
            public boolean areContentsTheSame(@NonNull HomeItem homeItem, @NonNull HomeItem t1) {
                return false;
            }
        }).build());
    }

    public void setRetryCallback(RetryCallback retryCallback) {
        this.retryCallback = retryCallback;
    }

    public void submitList(PagedList<HomeItem> pagedList) {
        mDiffer.submitList(pagedList);
    }

    public void setRecommendedAdapter(RecommendedAdapter recommendedAdapter) {
        this.recommendedAdapter = recommendedAdapter;
    }

    public PagedList<HomeItem> getCurrentList() {
        return mDiffer.getCurrentList();
    }

    @NonNull
    @Override
    public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
        switch (viewType) {
            case R.layout.item_question:
                return QuestionsViewHolder.create(parent);
            case R.layout.view_network_state:
                return NetworkStateViewHolder.create(parent, retryCallback);
            case R.layout.item_list_recommended:
                return RecommendedViewHolder.Companion.create(parent);
            default:
                throw new IllegalArgumentException("unknown view type");
        }
    }

    @Override
    public void onBindViewHolder(@NonNull RecyclerView.ViewHolder holder, int position) {
        switch (getItemViewType(position)) {
            case R.layout.item_question:
                ((QuestionsViewHolder) holder).bindTo((QuestionResponse) mDiffer.getItem(position));
                break;
            case R.layout.view_network_state:
                ((NetworkStateViewHolder) holder).bindTo(networkState);
                break;
            case R.layout.item_list_recommended:
                ((RecommendedViewHolder) holder).bindTo((Recommended) mDiffer.getItem(position), recommendedAdapter);
                break;
        }
    }

    private boolean hasExtraRow() {
        return networkState != null && networkState != NetworkState.LOADED;
    }

    @Override
    public int getItemViewType(int position) {
        if (hasExtraRow() && position == getItemCount() - 1) {
            return R.layout.view_network_state;
        } else if (position == 0) {
            return R.layout.item_list_recommended;
        } else {
            return R.layout.item_question;
        }
    }

    @Override
    public int getItemCount() {
        return mDiffer.getItemCount() + (hasExtraRow() ? 1 : 0);
    }

    public void setNetworkState(NetworkState newNetworkState) {
        NetworkState previousState = this.networkState;
        boolean hadExtraRow = hasExtraRow();
        this.networkState = newNetworkState;
        boolean hasExtraRow = hasExtraRow();
        if (hadExtraRow != hasExtraRow) {
            if (hadExtraRow) {
                notifyItemRemoved(mDiffer.getItemCount());
            } else {
                notifyItemInserted(mDiffer.getItemCount());
            }
        } else if (hasExtraRow && previousState != newNetworkState) {
            notifyItemChanged(getItemCount() - 1);
        }
    }
}

When I swipe to refresh I get this:

    java.lang.IndexOutOfBoundsException: Inconsistency detected. Invalid view holder adapter positionViewHolder{b46fd14 position=0 id=-1, oldPos=0, pLpos:-1 scrap [attachedScrap] tmpDetached no parent} android.support.v7.widget.RecyclerView{40c5213 VFED.V... ......I. 0,0-1080,1665 #7f0a0157 app:id/list_main}, adapter:com.android.features.home.home.feed.adapter.HomeAdapterNew@7f2ec50, layout:android.support.v7.widget.LinearLayoutManager@c4f2e49, context:com.android.features.home.HomeActivity@2be65fc
        at android.support.v7.widget.RecyclerView$Recycler.validateViewHolderForOffsetPosition(RecyclerView.java:5715)
        at android.support.v7.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline(RecyclerView.java:5898)
        at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5858)
        at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5854)
        at android.support.v7.widget.LinearLayoutManager$LayoutState.next(LinearLayoutManager.java:2230)
        at android.support.v7.widget.LinearLayoutManager.layoutChunk(LinearLayoutManager.java:1557)
        at android.support.v7.widget.LinearLayoutManager.fill(LinearLayoutManager.java:1517)
        at android.support.v7.widget.LinearLayoutManager.onLayoutChildren(LinearLayoutManager.java:612)
        at android.support.v7.widget.RecyclerView.dispatchLayoutStep1(RecyclerView.java:3875)
        at android.support.v7.widget.RecyclerView.dispatchLayout(RecyclerView.java:3639)
        at android.support.v7.widget.RecyclerView.onLayout(RecyclerView.java:4194)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.support.v4.widget.SwipeRefreshLayout.onLayout(SwipeRefreshLayout.java:625)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.widget.RelativeLayout.onLayout(RelativeLayout.java:1080)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.support.constraint.ConstraintLayout.onLayout(ConstraintLayout.java:1915)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1791)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1635)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1544)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1791)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1635)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1544)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at com.android.internal.policy.DecorView.onLayout(DecorView.java:944)
        at android.view.View.layout(View.java:20836)
        at android.view.ViewGroup.layout(ViewGroup.java:6401)
        at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:2948)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2635)

If I comment this lines, it doesn't crash but it's not refreshing:

 final AdapterListUpdateCallback adapterCallback = new AdapterListUpdateCallback(this);
        mDiffer = new AsyncPagedListDiffer<>(new ListUpdateCallback() {
            @Override
            public void onInserted(int i, int i1) {
                //adapterCallback.onInserted(i + 1, i1);
            }

            @Override
            public void onRemoved(int i, int i1) {
                //adapterCallback.onRemoved(i + 1, i1);
            }

            @Override
            public void onMoved(int i, int i1) {
                //adapterCallback.onMoved(i + 1, i1 + 1);
            }

            @Override
            public void onChanged(int i, int i1, @Nullable Object o) {
                //adapterCallback.onChanged(i + 1, i1, o);
            }
        }, new AsyncDifferConfig.Builder<HomeItem>(new DiffUtil.ItemCallback<HomeItem>() {
            @Override
            public boolean areItemsTheSame(@NonNull HomeItem homeItem, @NonNull HomeItem t1) {
                return false;
            }

            @Override
            public boolean areContentsTheSame(@NonNull HomeItem homeItem, @NonNull HomeItem t1) {
                return false;
            }
        }).build());

How can I solve this?

@lucandr
Copy link

lucandr commented Feb 5, 2019

@kamgurgul can u reopen this issue?

@kamgurgul
Copy link
Author

If your header isn't a part of "PagedList" then you have bad "getItemCount()" implementation. You don't have additional element for a header. Just try to add 1.

Anyway I think it isn't the right place for this issue :)

@lucandr
Copy link

lucandr commented Feb 5, 2019

@kamgurgul the header is part of the "PagedList", why do u think this is not the right place for the issue?
I am having the same scrolling problem. Thanks for the answer @kamgurgul

@lucandr
Copy link

lucandr commented Feb 5, 2019

I created a new issue @kamgurgul : Issue 548

Thanks for all

@danielwilson1702
Copy link

danielwilson1702 commented Sep 9, 2019

I have tried implementing a simple header but now all my items except the header have disappeared. If anyone could take a look at what could be missing I would love to know: https://stackoverflow.com/questions/57853321/how-to-add-a-header-to-a-pagedlist-using-android-paging-library

Edit: Ok I actually figured it out, I was using super.getItemCount() in the getItemCount() function when I should have been using differ.itemCount

feragusper pushed a commit to intive-FDV/TMDBAndroid that referenced this issue Sep 20, 2021
 - A few changes related to a workaround proposed here => android/architecture-components-samples#375
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants