Open technicalflaw opened 5 years ago
davideas
commented
5 years ago @technicalflaw, I don't have time to analyse all this. Can you debug it?
Are you scrolling down I guess, so the sticky header will disappear at top, because the view is indeed recycled manually.
Please check if the position at line 1814 in FlexibleAdapter.onViewRecycled() is a valid one and belongs to the expected Item and ViewHolder. Which is the position of the sticky header and which the position found?
technicalflaw
commented
5 years ago Sorry for the delay. I've had another look, and whilst I've 'fixed' it I don't know if it's fixing a problem I caused, or something else. Anyway, the main issue seems to be FlexibleAdapter.onViewRecycled() referencing the adapter position of items that have moved, or been deleted, depending on when it is called. If it's called from StickyHeaderHelper when it manually recycles its ViewHolder it's very likely to be wrong, because that class doesn't listen for data change events, but I've also had it happen via delayed recycling after a layout (see RecyclerView#mUpdateChildViewsRunnable).
That said, here's how I've worked around it.
Change FlexibleAdapter.onViewRecycled()
public void onViewRecycled(@NonNull RecyclerView.ViewHolder holder) {
super.onViewRecycled(holder);
if (areHeadersSticky()) {
// #297 - Empty (Invisible) Header Item when Using Sticky Headers
holder.itemView.setVisibility(View.VISIBLE);
}
int position = holder.getAdapterPosition();
// the only public way to check for private 'FLAG_REMOVED' set on holder
if (position != RecyclerView.NO_POSITION) {
// not sure of the logic for this check, but it seems to work
if (holder.getOldPosition() != RecyclerView.NO_POSITION) {
position = holder.getLayoutPosition();
}
T item = getItem(position);
if (item != null) {
item.unbindViewHolder(this, holder, position);
}
}
}Remove view recycling from StickyHeaderHelper.swapHeader(). Unless the position is fixed up in the mStickyHeaderViewHolder when an update occurs it will always point to the old item. Personally I don't use unbindViewHolder() for headers, so it doesn't matter for me.
// if (mHeaderPosition > oldHeaderPosition) {
// mAdapter.onViewRecycled(mStickyHeaderViewHolder);
// }Make FlexibleAdapter.AdapterDataObserver.updateStickyHeader() update after layout, rather than an arbitrary delay (plus fix for fixed size translation).
private void updateStickyHeader(int positionStart) {
int stickyPosition = getStickyPosition();
// #499 - Bulk operation properly updates the same sticky header once, while each
// independent operation (multiple events) updates the sticky header multiple times.
if (stickyPosition >= 0 && stickyPosition == positionStart) {
log.d("updateStickyHeader position=%s", stickyPosition);
// this is needed to translate initial sticky header correctly, when state restoration scrolls the list
if (mRecyclerView.hasFixedSize() && !mRecyclerView.isLayoutRequested()) {
mRecyclerView.requestLayout();
}
// #320 - To include adapter changes just notified we need a new layout pass:
// We must give time to LayoutManager otherwise the findFirstVisibleItemPosition()
// will return wrong position!
if (!mIsLayoutChangeListenerAdded) {
mRecyclerView.addOnLayoutChangeListener(new View.OnLayoutChangeListener() {
@Override
public void onLayoutChange(View v, int left, int top, int right, int bottom, int oldLeft, int oldTop, int oldRight, int oldBottom) {
v.removeOnLayoutChangeListener(this);
mIsLayoutChangeListenerAdded = false;
if (areHeadersSticky()) {
mStickyHeaderHelper.updateOrClearHeader(true);
}
}
});
mIsLayoutChangeListenerAdded = true;
}
}
}This will need a lot of testing (a previous solution seemed to work for a couple of days, but ultimately failed), and I'm still not sure if it's right. So far, so good, however.
davideas
commented
5 years ago @technicalflaw thank you. I should have a look, but I do not know when: I have important nicely events in my life now :-D
PS. The ideal is to get over of this solution and develop a custom LayoutManager (solution more complicated but correct).
I'm having an issue when I update the adapter's data set (via
updateDataSet()), but only when sticky headers are enabled, AND a new header is introduced as part of the update, AND a scroll occurs, such that previously sticky header's position changes. I'm not using expandable headers. In addition, it's very hard to reproduce. It can take hours, but eventually it will occur. Here's the stack:As you can see, when the
StickyHeaderHelpermanually recycles its sticky header (normally via its scroll handler, as it scrolls out of the view), it thinks its item is aMySectionedIteminstance (i.e. a subclass ofAbstractSectionableItem, notAbstractHeaderItem. So theViewHolderpassed toAbstractSectionableItem#unbindViewHolder()is that of the header.As far as I can tell (I'm in no way sure), this is because the
StickyHeaderHelperkeeps a reference to theViewHolderfor the stuck item, so when the data set is updated, and the time comes to swap out them item (i.e. recycle it), and it requests its position in the adapter (viaFlexibleAdapter#onViewRecycled()'s call toRecyclerView.ViewHolder#getAdapterPosition()) it receives the position it was at before the update, which is still a valid index, but no longer referencing the header's actual position in the new data set.I should also add that my data set is unusual in that its updates tend to be at the head of the list, so new headers tend to be created at index 0, thus offsetting all items after it. If headers were to be created at the end of the list it would be less likely to occur.
As I say, I'm not 100% sure my explanation is correct, but it's the best I can think of. It might also be related to
FlexibleAdapter.AdapterDataObserver#updateStickyHeader()'s delayed call toStickyHeaderHelper#updateOrClearHeader(), which might explain why it doesn't happen every time.I've checked that my item layout ids are unique, and that
Object#equalsworks OK. I think it might be related to issues #568 and #575. Any help (or just a workaround) would be greatly appreciated.