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

GridDividerItemDecoration crashes on an empty list #33

Open
mobiRic opened this issue Feb 28, 2018 · 2 comments · May be fixed by #36
Open

GridDividerItemDecoration crashes on an empty list #33

mobiRic opened this issue Feb 28, 2018 · 2 comments · May be fixed by #36

Comments

@mobiRic
Copy link

mobiRic commented Feb 28, 2018

GridDividerItemDecoration.drawHorizontalDividers() assumes that the number of items in the list is at least the number of columns in the grid (see View child = parent.getChildAt(i);)

private void drawHorizontalDividers(Canvas canvas, RecyclerView parent) {
    int parentTop = parent.getPaddingTop();
    int parentBottom = parent.getHeight() - parent.getPaddingBottom();

    for (int i = 0; i < mNumColumns; i++) {
        View child = parent.getChildAt(i);
        RecyclerView.LayoutParams params = (RecyclerView.LayoutParams) child.getLayoutParams();

        int parentLeft = child.getRight() + params.rightMargin;
        int parentRight = parentLeft + mHorizontalDivider.getIntrinsicWidth();

        mHorizontalDivider.setBounds(parentLeft, parentTop, parentRight, parentBottom);
        mHorizontalDivider.draw(canvas);
    }
}

There needs to be a null check before the next line that assumes child is not null.

@mobiRic
Copy link
Author

mobiRic commented Feb 28, 2018

Updating to the latest code which is slightly different to v1.0.0 released on maven shows this method (which was introduced in 11a647f):

private void drawHorizontalDividers(Canvas canvas, RecyclerView parent) {
    int childCount = parent.getChildCount();
    int rowCount = childCount / mNumColumns;
    int lastRowChildCount = childCount % mNumColumns;

    for (int i = 1; i < mNumColumns; i++) {
        int lastRowChildIndex;
        if (i < lastRowChildCount) {
            lastRowChildIndex = i + (rowCount * mNumColumns);
        } else {
            lastRowChildIndex = i + ((rowCount - 1) * mNumColumns);
        }

        View firstRowChild = parent.getChildAt(i);
        View lastRowChild = parent.getChildAt(lastRowChildIndex);

        int dividerTop = firstRowChild.getTop();
        int dividerRight = firstRowChild.getLeft();
        int dividerLeft = dividerRight - mHorizontalDivider.getIntrinsicWidth();
        int dividerBottom = lastRowChild.getBottom();

        mHorizontalDivider.setBounds(dividerLeft, dividerTop, dividerRight, dividerBottom);
        mHorizontalDivider.draw(canvas);
    }
}

This still crashes on line int dividerTop = firstRowChild.getTop(); with an empty list, since all child views are null. This will in fact crash for any list with fewer items than columns, sing firstRowChild is trying to find the view to the right of the divider.

Since the commit is the same, I think this is related to #31

mobiRic added a commit to mobiRic/simple-item-decoration that referenced this issue Mar 2, 2018
Given a grid where the first row is not filled up to the end, `firstRowChild` will become null at some point.

Resolves: bignerdranch#33
@mobiRic
Copy link
Author

mobiRic commented Mar 2, 2018

Apologies - this is a duplicate of #28

I did not originally notice that as I was working with an empty list.

TonyTangAndroid pushed a commit to TonyTangAndroid/itemDecoration that referenced this issue Dec 28, 2018
Given a grid where the first row is not filled up to the end, `firstRowChild` will become null at some point.

Resolves: bignerdranch#33
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

Successfully merging a pull request may close this issue.

1 participant