FlexibleItemDecoration withOffset not working correctly for section headers

[omgreel]

Hi there.
I'm facing a simple bug when trying to add offset for my section headers with GridLayoutManager.
The problem is that offset won't applied for headers with
position == itemsCount - grid.spanCount - 1.

For example, if GridLayoutManager spanCount == 2, there is no offset between last and before last item.
As I can see the problem is in isLastRowOrColumn() method.

Specially in this part:
int nextRowPos = position < itemCount - (spanCount - spanIndex) ? position + (spanCount - spanIndex) : -1;

Assume we have 10 items and Grid's columnCount == 2.
Since headers take whole width the spanCount will be equals 2, and spanIndex will be 0.
So, for the before last position the position will be 8.
So we have 8 < 10 - (2 - 0) which is false, and the ternary result is -1.
In method return statement there is ... || nextRowPos == -1 || ... condition, so we get true for position before last.

I'm using 5.0.0rc2 version of lib.

I would like any quick(!) suggestion how to fix it. Since I can't see the way to extend FlexibleItemDecoration and override relative methods.
Thanks.

[davideas]

Hi @omgreel, that calculation comes from the library which I imported, you can see it in #382.
You can of course take the code of this decoration and create your own decoration with the fix you need.

If you find the fix please let me know.

[davideas]

@omgreel, are you applying a sectionOffset with GridLayout?
So you mean that the items of the last row of each section do not receive the section gap but only the last item of the section?

[omgreel]

@davideas well, i'm not sure about gap, for me withOffset() seems to be broken.

Here the code I used to reproduce this bug in your sample app.
In FragmentExpandableSections:
mRecyclerView.addItemDecoration(new FlexibleItemDecoration(getActivity()) .addItemViewType(R.layout.recycler_expandable_header_item) .withOffset(4));

If you collapse all expandable headers and scroll to bottom you'll see no offset between last three headers.

[davideas]

Ok thanks, to have noticed it.
It is necessary a new calculation method. And for that I need more free time 😢
It is indeed coming from the other library.

[omgreel]

I will try to find some time to take a closer look to help you with it.

My first suggestion is to use spanSize of item.
Something like this:
int nextRowPos = position < itemCount - (spanCount/spanSize - spanIndex) ? position + (spanCount/spanSize - spanIndex) : -1;

I ran it on sample app in FragmentExpandableSection and it works in my case. But it's hard for me to say whether I broke something else or not. For example, there might be situation when spanCount = 3
and spanSize = 2.

[davideas]

@omgreel, 🆒 For the moment I do not see inconvenience. Let's use it.

However, I also tried to remove completely the calculation of nextRowPos and it seems to work anyway.
The first 3 conditions should be more than enough. because the next row should be covered by next position viewType condition... what do you think?

[omgreel]

My suggestion requires lots of tests for different viewTypes, spanCounts, spanSizes and etc.
Same for removing calculation. But I think there might be less side effects with no calculating nextRowPos at all.

Maybe I'll write test for isLastRowOrColumn method later.

[davideas]

The proposed fix will be available in the next snapshot.