Grid incorrectly overscroll's beyond last item

[venkat230278]

Occurs on master commit b81c1c6

Video can be found here: http://dai.ly/x1dj5iy

Test App to reproduce is at https://github.com/venkat230278/AndroidStaggeredGridViewOverscrollBug.git

Place app at same level as "library", "sample", import into Android studio and build.

Test App has a StaggeredGridView with 2 columns. Its adapter supplies 3 text views. Eventually, the three text views are laid out by the GridView as below.

A B
C

All text views (A, B, C) are of equal height H. H is greater than screen height. In above layout, A and B are clipped by the screen.

Slowly start scrolling towards and beyond C. Note that it is possible to scroll beyond C. Scroll in reverse towards A. Notice that B has disappeared. Notice after a while that A too disappears.

This issue is also encountered if the user flings on A or B towards C.

It appears that the grid view considers empty columns in scroll math. We tried to put in a quick hack fix for this issue for our product.

Patch can be found here: https://github.com/venkat230278/AndroidStaggeredGridViewOverscrollBug/blob/master/patch/sg.patch

Apply patch as follows:

[Some dir]/AndroidStaggeredGrid/library/src/main$ patch -p1 < sg.patch

Things to note about the above patch:

1. Tested only for 2 columns.
2. Not tested with other staggered grid features such as headers or footers.
3. Might introduce other bugs.
4. No guarantee / warranty of any kind. Use at your own risk.
5. Might make the staggered grid less efficient.

[venkat230278]

Could be related to #21

[denizmveli]

Thank you for a very detailed issue description. I'll be working on this and other small set issues for the next release 1.0.5.

[jbgill]

I applied @venkat230278 's patch to 1.0.4 and it improves matters, but doesn't solve it entirely. I still see issues with the views disappearing and other strangeness when scrolling to the bottom of the grid and back up - but only when I have items that are taller than the grid itself - like in landscape orientation

[venkat230278]

@jbgill

I would like to look into the problem you are experiencing after applying the patch. If possible, do you have a minimal app that reproduces the issue? Thanks.

[jbgill]

@venkat230278 Unfortunately, I don't have anything minimal I can send you right now. I'm attempting to put this view into a large app with dynamic content from our servers. If I get some time soon I'll see if I can trim it down to a small sample with some hard-coded content. The over-scrolling seems to be fixed, but I still see cases where if I scroll past a tall item, then back up, I get views in other columns disappearing and sometimes I have to scroll back up through the tall item twice before seeing the items above it. My failing examples have been 3 columns with maybe a dozen or so items of varying heights with one or two items that are taller than the grid view.

@venkat230278
Hi, I just tried version 1.0.4 and faced the problem described here. I put together a small demo app for debugging:
https://content.wuala.com/contents/muetzenflo/Sonstiges/Public/StaggeredGridViewDemo.zip/?dl=1

It starts in landscape mode. If you fling fast through the list and try to get back to the top some strange things happen. Sometimes it goes all the way up, but only with 1 column instead of 2, sometimes the listItems disappear.
I tried it on an HTC One mini. Maybe you need to make the hard-coded Strings a little bit longer to increase the height of each listItem.

[RuiRoque]

I had the same problems, even after applying the fix by venkat230278. Disappearing columns were a common issue.

So, instead of diving into the source code and get my hands dirty, I started to think what were we all doing different that Etsy isn't in their app. And I think I found out. I no longer have any problems with this issue.

There are two classes in the library:
com.etsy.android.grid.util.DynamicHeightTextView
com.etsy.android.grid.util.DynamicHeightImageView

They use these in the sample app, so you can see their usage. Basically, each class sets the height of the View in the onMeasure method. I was already using one of these classes in my app for the ImageView, but it was inside a LinearLayout with some other TextViews on the bottom. With
this setup, I was experiencing the reported problems. But when I removed the simple TextViews and had only my custom programmatic resizable view inside a FrameLayout, it worked perfectly. I assume that the views in the adapter item must all be "pre-measurable", and that dynamic layout properties like wrap_content and match_parent doesn't work very well. Hope this helps.

@RuiRoque are you sure about this? Did you try it with list items really higher than the available screen size? I just updated my DemoProject and each list item uses this simple layout:

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:orientation="vertical"
    android:layout_width="match_parent"
    android:layout_height="wrap_content">

<com.muetzenflo.sandbox.DynamicHeightTextView
    android:id="@+id/text"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:textSize="20sp"
    android:padding="10dp"
    />

</LinearLayout>

But I still cannot scroll the list back up from bottom to top without losing elements and/or having only one column filled and one empty. Would you mind taking a look at my demo? It's very simple, just an activity and the adapter.

https://content.wuala.com/contents/muetzenflo/Sonstiges/Public/StaggeredGridViewDemo.zip/?dl=1

Thanks for helping us here!

PS: If it really were the case that EVERY child of a list item has to be pre-measurable it would not be so cool, since I am using the official YouTubeThumbnailView from the YouTubePlayer API. This ThumbnailView is a final class and therefore cannot be overridden...

https://developers.google.com/youtube/android/player/reference/com/google/android/youtube/player/YouTubeThumbnailView?hl=ko

[RuiRoque]

Yes, this worked for me.

You are using the DynamicHeightTextView, but still not setting the height in the Adapter. The DynamicHeightTextView uses a ratio to determine the height, which was not very handy for me. So, I build my own DynamicHeightTextView (in my case, for an ImageView) to set the height and not the ratio:

public class DynamicHeightTextView extends TextView {

private int imageHeight;

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

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

public DynamicHeightTextView(Context context, AttributeSet attrs, int defStyle) {
    super(context, attrs, defStyle);
}

public void setHeight(int height) {
    if (imageHeight != height) {
        imageHeight = height;
        requestLayout();
    }
}

@Override
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
    if (imageHeight > 0) {
        int width = MeasureSpec.getSize(widthMeasureSpec);
        setMeasuredDimension(width, imageHeight);
    } else {
        super.onMeasure(widthMeasureSpec, heightMeasureSpec);
    }
}

}

With this, in the adapter, try to set the height. Something like:

holder.text.setHeight(someHeightInPixels);

And also:
holder.text.getLayoutParams().width = maximumImageWidth;
holder.text.getLayoutParams().height = someHeightInPixels;

The maximumImageWidth is something like:
maximumImageWidth = (maximumScreenWidthAccordingToOrientation / numberOfRows) - (staggeredGridViewMargin * numberOfRows);

And for the someHeightInPixels, build some random generator to get different values.

[Frank1234]

Thank you for the patch venkat.

@RuiRoque Thanks for your explanation and code! As far as I understand all the code examples I can find, they either

• use random height values for testing purposes or
• use height values which are known, because the height of the corresponding content (in your case an image) is fixed or calculated by a columnswidth:imageratio combination.

None of this is applicable for me. My list items look like this:

-----------------
| X | Title     |
-----------------
|               |
|   Image       |
|               |
-----------------
| Text of       |
| unknown       |
| length        |
-----------------

So, am I right when I understand it that I have to create:

• A DynamicHeightImageView for the icon next to the title and the image in the middle
• A DynamicHeightTextView for the title and the "text of unknown length"

These DynamicHeightXXViews both have a height value that I set in the getView()-method of my StaggeredGridViewAdapter. And to set this height value I have to calculate the exact height of each DynamicHeightXXXView?

If yes, this would bring two new question for me:

1. The image under the title is a YouTubeThumbnailView which is a final class and can therefore not be extended or modified. How would I handle this? Maybe an invisible fake View above the YouTubeThumbnailView with the same height?
2. How can I calculate the height of the "text of unknown length" before it is rendered on screen?

Thank you for your help!

In my case, @venkat230278 's patch fixed the problem. Thanks a lot!