Skip to content

Commit

Permalink
[EoS] Fix scrolling issues caused by LinearLayoutManager
Browse files Browse the repository at this point in the history
RecyclerViews inherently have two data models that are kept in "sync":
* The child views that are actually on the RecyclerView
* the model provided by the Adapter.

We use LinearLayoutManager#scrollToPosition when the model is loaded
to scroll to a particular card.  Normally this scrolls the card to the
top of the view.  However, if the model provided by the Adapter has
more items than the recyclerview has children, and the scroll request
is for an item out of range of the child count, then the layout manager
will only scroll the view to the bottom of the RecyclerView ! (see
https://cs.chromium.org/chromium/src/third_party/android_tools/sdk/sources/android-25/android/support/v7/widget/LinearLayoutManager.java?rcl=e958d6ea74442d4e0849bb8a018d215a0e78981d&l=850)
for details)

We can work around this by using LinearLayoutManager#scrollToPositionWithOffset
with an offset of 0, which disables the only-scroll-into-view logic.

Bug: 915378
Change-Id: Ic25e7e90558c5bed953995e359cb03dff8010219
Reviewed-on: https://chromium-review.googlesource.com/c/1396704
Reviewed-by: Cathy Li <chili@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620410}
  • Loading branch information
mrdewitt authored and Commit Bot committed Jan 7, 2019
1 parent 70fdf14 commit 56e2f55
Showing 1 changed file with 13 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import android.content.Context;
import android.support.annotation.IntDef;
import android.support.annotation.Nullable;
import android.support.v7.widget.RecyclerView;
import android.support.v7.widget.LinearLayoutManager;
import android.text.SpannableString;
import android.text.SpannableStringBuilder;

Expand Down Expand Up @@ -49,10 +49,10 @@ class CategoryCardAdapter extends ForwardingListObservable<Void>
private final NativePageNavigationDelegate mNavDelegate;
private final Profile mProfile;

private RecyclerView.LayoutManager mLayoutManager;
private LinearLayoutManager mLayoutManager;
private PropertyModel mCategoryModel;

public CategoryCardAdapter(PropertyModel model, RecyclerView.LayoutManager layoutManager,
public CategoryCardAdapter(PropertyModel model, LinearLayoutManager layoutManager,
RoundedIconGenerator iconGenerator, ContextMenuManager contextMenuManager,
NativePageNavigationDelegate navDelegate, Profile profile) {
mCategoryModel = model;
Expand Down Expand Up @@ -133,8 +133,16 @@ public void onPropertyChanged(
}
}
if (key == ExploreSitesPage.SCROLL_TO_CATEGORY_KEY) {
mLayoutManager.scrollToPosition(
mCategoryModel.get(ExploreSitesPage.SCROLL_TO_CATEGORY_KEY));
// NOTE: LinearLayoutManager#scrollToPosition has strange behavior if the scrolling
// happens between the time that the adapter has an item and the time that the view has
// actually added its children. In that case, the LinearLayoutManager will only scroll
// the requested position /into view/.
//
// To work around that, we use LinearLayoutManager#scrollToPositionWithOffset, and set
// the offset to 0. This allows us to always scroll the desired view to the top of the
// screen.
mLayoutManager.scrollToPositionWithOffset(
mCategoryModel.get(ExploreSitesPage.SCROLL_TO_CATEGORY_KEY), 0);
}
}
}

0 comments on commit 56e2f55

Please sign in to comment.