Epoxy: Why SwipeRefreshLayout dont work, when first model is hidden.

[frank-fan]

I use Epoxy to manage RecycleView inside SwipeRefreshLayout, like below:

 <android.support.v4.widget.SwipeRefreshLayout
      android:id="@+id/srLayout"
      android:layout_width="match_parent"
      android:layout_height="match_parent">

      <android.support.v7.widget.RecyclerView
          android:id="@+id/rvList"
          android:layout_width="match_parent"
          android:layout_height="match_parent"/>
  </android.support.v4.widget.SwipeRefreshLayout>

It is weird when I hide first model of RecycleView by EpoxyModel.hide(), SwipeRefreshLayout doesn't work anymore.
I create a TestProject, https://github.com/fanxu123/EpoxyTest, specially for demonstration.
Please help, thanks.

[elihart]

Thanks for reporting this and for the project to reproduce it.

When a model is hidden it is actually still added to the recycler view, but a Space object is used as the view with a size of 0. https://github.com/airbnb/epoxy/blob/master/epoxy-adapter/src/main/res/layout/view_holder_empty_view.xml is the layout that is used.

This must be messing up the swipe to refresh layout somehow, but I don't have a good guess as to why off the top of my head. I would need to dig into this a bit, but won't be able to get to it for a few days. Maybe somebody else can look in the meantime.

The first thing I would sanity check is that in the empty layout mentioned above the width is zero. I would try changing it to match_parent to see if that makes a difference.

[frank-fan]

@elihart cool, thanks for update.
I will try "changing space to match_parent" when I got some time.

[SamThompson]

Hi, also ran across this issue.

I was able to inspect the layout via stetho and it looks like the width of the empty view is zero.

I tried changing it to match_parent but it didn't seem to fix things.

[elihart]

@SamThompson the width of 0 is expected. Thanks for trying match_parent, that was just a hypothesis but it's too bad it doesn't work. Could you maybe try change the height to 1dp? I'm curious if that fixes it - it wouldn't be a good long term solution but it may tell us more about the problem.

[elihart]

I haven't had time to look into this myself yet

[SamThompson]

Interesting. It works when the height is 1dp (unfortunately didn't have time this weekend to dive any deeper).

Here's a demo branch on my fork which incorporates @fanxu123's example: https://github.com/SamThompson/epoxy/tree/bug

[elihart]

Good to know that 1dp works, thanks for testing that. I'm not sure what that means yet, but it's a start. I'm guessing any non zero value would work, so maybe 1px could work as a short term solution for anybody that really needs it. That should be barely noticeable, but is definitely not ideal.

This may be related to https://github.com/airbnb/epoxy/issues/61. I unfortunately haven't had time to look deeper at this myself, and I am fairly busy the next two weeks, but I welcome anybody else to help with it.

[SamThompson]

I was able to track the bug down to LinearLayoutManager.

Before showing the refresh spinner, SwipeRefreshLayout makes a check to whether or not RecyclerView can be scrolled in the negative direction:

        // from SwipeRefreshLayout#onNestedScroll
        final int dy = dyUnconsumed + mParentOffsetInWindow[1];
        if (dy < 0 && !canChildScrollUp()) {  // <-- evaluates to false and never shows the spinner 
                                              // b/c canChildScrollUp() == true
            mTotalUnconsumed += Math.abs(dy);
            moveSpinner(mTotalUnconsumed);
        }

the conditional dy < 0 && !canChildScrollUp() evaluates to false. Tracing canChildScrollUp(), it eventually leads us to this code in LinearLayoutManager:

    private int computeScrollOffset(RecyclerView.State state) {
        if (getChildCount() == 0) {
            return 0;
        }
        ensureLayoutState();
        return ScrollbarHelper.computeScrollOffset(state, mOrientationHelper,
                findFirstVisibleChildClosestToStart(!mSmoothScrollbarEnabled, true),
                findFirstVisibleChildClosestToEnd(!mSmoothScrollbarEnabled, true),
                this, mSmoothScrollbarEnabled, mShouldReverseLayout);
    }

The call to findFirstVisibleChildClosestToStart will return the first non-hidden item (since the height of a hidden item is 0dp and isn't considered visible by the layout manager). However, because there is still technically an item before the first non-hidden item, the layout manager still considers the RecyclerView scrollable in the negative direction due to this math:

// final return in ScrollbarHelper#computeScrollOffset
return Math.round(itemsBefore * avgSizePerRow + (orientation.getStartAfterPadding()
                - orientation.getDecoratedStart(startChild)))

Let me know if that explanation is unclear and I can do my best to clarify. I can imagine a similar thing is happening in #61

[elihart]

@SamThompson Thanks for the great analysis. Sounds like a complex situation - can you pinpoint what a fix in the layout manager would look like? Is it worth reporting as a support library bug?

As far as a fix for our purposes these are the options I see:

• Instead of a zero sized empty layout, have an option for 1px. Seems workable, maybe the easiest, but not ideal.
• Use a custom layout manager with a fix. Doesn't seem practical.
• Come up with a way to work around the layout manager's measuring bug. Maybe we can think of something clever?
• Change how the hidden model system works. Instead of adding empty views for a hidden model we could add no view. This seems like maybe the best long term solution since it is a bit hacky to use an empty view and we may not be able to fix this bug otherwise. The reason I didn't do this from the start is because we would need to maintain more state in the adapter and keep a separate list of just the items that are shown.

Any other ideas?

[SamThompson]

One easy fix might be to change the if check in LinearLayoutManager's findOneVisibleChild from if (childStart < end && childEnd > start) to if (childStart < end && childEnd >= start). I tested it locally and it seems to work, but it might have unintended consequences. It might be worth creating a bug for that. The worst they could do is close it and tell us to change how the model system works 😝

I think those options look good. The only issue with the 1px fix is it accumulates if there are a lot of hidden models, leading to unintended spaces in the ui.

[elihart]

Unfortunately I think the standard layout managers don't play well with views of zero size, and it is dangerous to rely on that behavior to make hidden models work.

In the upcoming 2.0 release and the new EpoxyController there is no need to hide models so this won't be a problem. The original EpoxyAdapter will continue to be available but I have no plans to try to change the hidden models implementation to fix these issues.

To summarize the issues and fixes:

1. Your top model can't be hidden if you are using pull to refresh.
2. On a grid layout consecutive hidden models that don't take up the full span count break scrolling. You can work around this by setting the models span size to the total span size when they are hidden. Something like this should work.

@Override public int getSpanSize(int spanCount, int position, int itemCount) {
    return isShown() ? whateverSpanSizeIWant : spanCount;
  }

Hiding models is just a slightly easier way of dealing with models in some cases. If you are having trouble with hidden models you can just not add them to the adapter instead, or use the EpoxyController available since the 2.0 release

If any additional problems or solutions arise, please post here.

[sotic]

@SamThompson Hi Sam, why not just trying to extend SwipeRefreshLayout and modify canChildScrollUp() to fill your need?

public class SwipeRefreshLayoutCustom extends SwipeRefreshLayout {

    private ListCallback mListCallback;

    public SwipeRefreshLayoutCustom(Context context) {
        super(context);
    }

    public SwipeRefreshLayoutCustom(Context context, AttributeSet attrs) {
        super(context, attrs);
    }

    @Override
    public boolean canChildScrollUp() {
        return super.canChildScrollUp() && mListCallback != null && !mListCallback.isScrolledToTop();
    }

    public void setListCallback(ListCallback callback) {
        mListCallback = callback;
    }

    public interface ListCallback {

        boolean isScrolledToTop();

    }

} 

The mListCallback should be the recyclerView in use in this case, and you can implement isScrolledToTop() to check if the recyclerView has actually or not reached to top in mRecyclerView.addOnScrollListener() by invoking LinearLayoutManager.findFirstCompletelyVisibleItemPosition().

The following code can be as reference

private static final int NUM_HIDDEN_ITEM = 1;

mRecyclerView.addOnScrollListener(new RecyclerView.OnScrollListener() {
            @Override
            public void onScrollStateChanged(RecyclerView recyclerView, int newState) {
                super.onScrollStateChanged(recyclerView, newState);
                int firstCompletely = ((LinearLayoutManager) mLayoutManager).findFirstCompletelyVisibleItemPosition();
                int firstVisible = ((LinearLayoutManager) mLayoutManager).findFirstVisibleItemPosition();
                isScrolledToTop = firstCompletely == NUM_HIDDEN_ITEM || firstCompletely == -1 && firstVisible == NUM_HIDDEN_ITEM;
            }

            @Override
            public void onScrolled(RecyclerView recyclerView, int dx, int dy) {
                super.onScrolled(recyclerView, dx, dy);
            }
        });

mSwipeRefreshLayout.setListCallback(new SwipeRefreshLayoutCustom.ListCallback() {
                @Override
                public boolean isScrolledToTop() {
                    return isScrolledToTop;
                }
            });

[SiarheiSm]

workaround:

<androidx.swiperefreshlayout.widget.SwipeRefreshLayout
    android:layout_width="match_parent"
    android:layout_height="match_parent">

    <FrameLayout
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <androidx.recyclerview.widget.RecyclerView
            android:layout_width="match_parent"
            android:layout_height="match_parent" />
    </FrameLayout>
</androidx.swiperefreshlayout.widget.SwipeRefreshLayout>

[matthewrkula]

@SiarheiSm Thanks this worked for me!