search

ebarrenechea / header-decor

A couple of sticky header decorations for android's recycler view.
Apache License 2.0
878 stars 159 forks source link

Add ability to make headers optional for some items #48

Closed starkej2 closed 8 years ago

starkej2 commented 8 years ago

This is basically the same as pull request #32 with a few modifications that made it a little easier to understand as well as a bug fix from @chendrak.

starkej2 commented 8 years ago

This will resolve issue #34

ebarrenechea commented 8 years ago

Awesome! Thanks for the contribution! 👍

stravadave commented 7 years ago

@eduardobarrenechea @edubarr this changed introduced some pretty severe performance issues. showHeaderAboveItem is looping through all previous elements in the adapter if the item before itemAdapterPosition has the same headerId. I noticed this because I found that my getHeaderId method was being called 37000 times after initial loading for a dataset of 2700 items. After making this change that number was reduced to 24 method calls. Here is the chunk of code I used to fix this.

private boolean showHeaderAboveItem(int itemAdapterPosition) { if (itemAdapterPosition == 0) { return true; } return mAdapter.getHeaderId(itemAdapterPosition - 1) != mAdapter.getHeaderId(itemAdapterPosition); }

starkej2 commented 7 years ago

Nice catch @stravadave! I didn't notice that.

stravadave commented 7 years ago

@starkej2 no worries. We didn't notice it on at first RecyclerView we added it onto. It wasn't until we started using it with some really large datasets that it became apparent. Besides this one issue, we have been really happy with the ease of use and looking forward to converting away our ListViews that we weren't able to do before without pinned headers.

starkej2 commented 7 years ago

I added that fix in #51 - it works fine in my testing and is logically equivalent.

(btw I ❤️ the Strava app @stravadave)

ebarrenechea commented 7 years ago

@stravadave thanks for finding that out! I haven't used this library with any datasets that large so it would go largely unnoticed for a while.

@starkej2 thanks for getting a fix so quickly! 👍

I'll get a new version out tonight still.