Expandable Sticky Header becomes invalid after updateDataSet

[omgreel]

Hi again.
I'm facing strange bug.

There is my case.
First of all, I create adapter passing null into adapter's constructor.
Then data loads from network and after data was loaded I map it into Expandable and Section items and call updateDataSet(mappedItems). Everything works fine so far.
The problem appears after data loaded from network, mapped and updateDataSet called again.
If there were sticky header, it is never collapsing onClick, until it become unsticky.

I logged holder.toString() to see if there is different holders after update.
For example, before updateDataSet called second time, sticky header has something like this:
{3CA0C98E POSITION=2 ID=-1, OLDPOS=-1, PLPOS:-1, SCRAP[ATTACHEDSCRAP] TMPDETACHED NO PARENT}

After updateDataSet was called:
{3CA0C98E POSITION=2 ID=-1, OLDPOS=-1, PLPOS:-1, INVALID UPDATE NOT RECYCLABLE(1) UNDEFINED ADAPTER POSITION NO PARENT}

And if I scroll a bit to made header unsticky I can see that this Expandable item has different hashCode's hex string, but it is still the same item.

Sorry for kinda messy question. I just spent two hours to find the problem and ended up with nothing.
Hope you can say me what I missed.

Here is the snippets of ExpandableItem and SectionItem:

ExpandableItem

public class CategoryHeaderItem extends BaseAdapterItem<CategoryHeaderItem.CategoryHeaderViewHolder>
        implements IExpandable<CategoryHeaderItem.CategoryHeaderViewHolder, ISectionable<?, CategoryHeaderItem>>, IHeader<CategoryHeaderItem.CategoryHeaderViewHolder> {

    private static final int DEFAULT_SUBITEMS_SIZE = 20;

    private boolean expanded;

    private Category category;

    private List<ISectionable<?, CategoryHeaderItem>> sectionItems;

    private boolean hasMore;
    private MoreProductsSubItem moreProductsItem;

    public CategoryHeaderItem(Category category) {
        setSelectable(false);
        this.category = category;
        this.sectionItems = new ArrayList<>();
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;

        CategoryHeaderItem that = (CategoryHeaderItem) o;

        return category.equals(that.category);
    }

    @Override
    public int hashCode() {
        return category.hashCode();
    }

    @Override
    public int getSpanSize(int spanCount, int position) {
        return 2;
    }

    @Override
    public int getLayoutRes() {
        return R.layout.list_item_category1;
    }

    public Category getCategory() {
        return category;
    }

    @Override
    public boolean isExpanded() {
        return expanded;
    }

    @Override
    public void setExpanded(boolean expanded) {
        this.expanded = expanded;
        if (!expanded) {
            onCollapse();
        }
    }

    @Override
    public int getExpansionLevel() {
        return 0;
    }

    @Override
    public List<ISectionable<?, CategoryHeaderItem>> getSubItems() {
        return sectionItems;
    }

    public void addSubItems(List<CatalogProductItem> subItems) {
        if (!hasMore && subItems.size() > DEFAULT_SUBITEMS_SIZE) {
            hasMore = true;
            sectionItems.addAll(subItems.subList(0, DEFAULT_SUBITEMS_SIZE));
            moreProductsItem = new MoreProductsSubItem(this, subItems.subList(DEFAULT_SUBITEMS_SIZE, subItems.size()));
            sectionItems.add(moreProductsItem);
        } else {
            sectionItems.addAll(subItems);
        }
    }

    @Override
    public CategoryHeaderViewHolder createViewHolder(View view, FlexibleAdapter adapter) {
        return new CategoryHeaderViewHolder(view, adapter);
    }

    @Override
    public void bindViewHolder(FlexibleAdapter adapter, CategoryHeaderViewHolder holder, int position, List payloads) {
        %bindCode%
    }

    public void setHasMore(boolean hasMore) {
        this.hasMore = hasMore;
    }

    private void onCollapse() {
        if (!hasMore && moreProductsItem != null) {
            hasMore = true;
            sectionItems.removeAll(moreProductsItem.getMoreProducts());
            sectionItems.add(moreProductsItem);
            moreProductsItem.setHidden(false);
        }
    }

    class CategoryHeaderViewHolder extends ExpandableViewHolder {

        TextView name;
        ImageView expandedIcon;
        ImageView image;

        CategoryHeaderViewHolder(View view, FlexibleAdapter adapter) {
            super(view, adapter, true);
            name = (TextView) view.findViewById(R.id.category_name);
            expandedIcon = (ImageView) view.findViewById(R.id.category_expanded_icon);
            image = (ImageView) view.findViewById(R.id.category_image);
        }

        @Override
        protected boolean isViewCollapsibleOnLongClick() {
            return false;
        }

        @Override
        protected boolean shouldNotifyParentOnClick() {
            return true;
        }

        @Override
        protected void collapseView(int position) {
            super.collapseView(position);
            onCollapse();
        }
    }

SectionItem

abstract public class BaseProductItem<VH extends BaseProductViewHolder>
 extends BaseAdapterItem<VH> {

    protected Product product;

    public BaseProductItem(Product product) {
        this.product = product;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        BaseProductItem baseProductItem = (BaseProductItem) o;
        return product.equals(baseProductItem.product);
    }

    @Override
    public int hashCode() {
        return product.hashCode();
    }

    @Override
    public int getLayoutRes() {
        return R.layout.list_item_product1;
    }

    @Override
    public int getSpanSize(int spanCount, int position) {
        return 1;
    }

    @Override
    public void bindViewHolder(FlexibleAdapter adapter, VH holder, int position, List payloads) {
        %bindCode%
    }

    public Product getProduct() {
        return product;
    }
}

[davideas]

@omgreel, can you enable Verbose logs and see what happens when you try to collapse? Did you try to debug as well? Do you somewhere call holder.getAdapterPosition() ?

[omgreel]

I will provide logs a bit later.
I don't manually call holder.getAdapterPosition() anywhere.
When I click sticky header the ViewHolder's onClick() method is called. But inside this method mAdapter.isEnabled() returns false.

This

@Override
	public void onClick(View view) {
		if (mAdapter.isEnabled(getFlexibleAdapterPosition()) && isViewExpandableOnClick()) {
			toggleExpansion();
		}
		super.onClick(view);
	}

come to this

public final int getFlexibleAdapterPosition() {
		int position = getAdapterPosition();
		if (position == RecyclerView.NO_POSITION) {
			position = mBackupPosition;
		}
		return position;
	}

and this method returns -1.
getAdapterPosition() retruns -1 and mBackupPosition is also equals -1.

[davideas]

mBackupPosition is not supposed to be invalid. That is set in StickyHeaderHelper when VH is created again. Probably the update called makes to loose that value.

[davideas]

@omgreel, can you debug that part of code and see what happens to the instance of that VH?
In my demoApp, when I refresh I am still able to collapse items and header doesn't swap. Yours?

[omgreel]

Hello there. Sorry for late response.
I made some debug to check why backup position is -1.
As I can see there is only one place where setBackupPosition() might be called.
It's here:

private FlexibleViewHolder getHeaderViewHolder(int position) {
		// Find existing ViewHolder
		FlexibleViewHolder holder = (FlexibleViewHolder) mRecyclerView.findViewHolderForAdapterPosition(position);
		if (holder == null) {
			// Create and binds a new ViewHolder
			holder = (FlexibleViewHolder) mAdapter.createViewHolder(mRecyclerView, mAdapter.getItemViewType(position));
			mAdapter.bindViewHolder(holder, position);

			// Restore the Adapter position
			holder.setBackupPosition(position);

			// Calculate width and height
			int widthSpec;
			int heightSpec;
			if (mAdapter.getFlexibleLayoutManager().getOrientation() == OrientationHelper.VERTICAL) {
				widthSpec = View.MeasureSpec.makeMeasureSpec(mRecyclerView.getWidth(), View.MeasureSpec.EXACTLY);
				heightSpec = View.MeasureSpec.makeMeasureSpec(mRecyclerView.getHeight(), View.MeasureSpec.UNSPECIFIED);
			} else {
				widthSpec = View.MeasureSpec.makeMeasureSpec(mRecyclerView.getWidth(), View.MeasureSpec.UNSPECIFIED);
				heightSpec = View.MeasureSpec.makeMeasureSpec(mRecyclerView.getHeight(), View.MeasureSpec.EXACTLY);
			}

			// Measure and Layout the stickyView
			final View headerView = holder.getContentView();
			int childWidth = ViewGroup.getChildMeasureSpec(widthSpec,
					mRecyclerView.getPaddingLeft() + mRecyclerView.getPaddingRight(),
					headerView.getLayoutParams().width);
			int childHeight = ViewGroup.getChildMeasureSpec(heightSpec,
					mRecyclerView.getPaddingTop() + mRecyclerView.getPaddingBottom(),
					headerView.getLayoutParams().height);

			headerView.measure(childWidth, childHeight);
			headerView.layout(0, 0, headerView.getMeasuredWidth(), headerView.getMeasuredHeight());
		}
		return holder;
	}

However, it's never called because holder is never null.
Well, I can't reproduce this behaviour on sample app too. But it never calls setBackupPosition either.
So it might be a miss logic, but it is not a reason for my bug.

Any ideas what else could be wrong?

[davideas]

@omgreel, ok I really don't know what happens to the position, I believed it is counted from the LayoutManager, so in that logic if we are able to retrieve the VH from a position, we can guess that we can retrieve the position from the VH, but probably is not as we suppose.

I can try to force that backup position even if the holder is found, they should match. As you can read from the Javadoc that position is lost because the VH is generated out of the LayoutManager when is scrolled down (because the header is much higher than the showed and recycled items), so the RV doesn't know about that holder yet.

I will move outsite that backup setting, I think it should work then.
I think it's the time to create a Snapshot version, so you will have to use it, until RC3 will be released, at least.

I believe instead that is the cause, because when it is checked if an item is enabled, the position -1 will make it return false, so the click event is interrupted.

PS: Unfortunately, you can't even override the onClick, because indeed you don't have the position...

[omgreel]

There is one more thing that I noticed.
Headers became broken only when it sticky and it's holder didn't scrolled enough to be recycled.
If header is sticky and I do enough scroll then everything works.