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 support to double header without sub header #55

Closed paulocns closed 6 years ago

paulocns commented 7 years ago

Added support to create a list with double sticky header with some items having sub-headers and some don't. Making sub-header optional

starkej2 commented 7 years ago

I'll take a look at this pull request sometime this week. Thanks for the contribution!

paulocns commented 7 years ago

No problem, if you have any questions feel free to contact me. Best regards

On Mon, Nov 14, 2016, 9:28 PM Jeffrey Starke notifications@github.com wrote:

I'll take a look at this pull request sometime this week. Thanks for the contribution!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/edubarr/header-decor/pull/55#issuecomment-260497858, or mute the thread https://github.com/notifications/unsubscribe-auth/AIUZA0D-JBr-Xu4wyUsu2_KVZ0QDsM20ks5q-O6wgaJpZM4KpbLU .

paulocns commented 7 years ago

Sorry if i took so long to address your comments, please re check and if you find more problems please let me know Best Regards

andacaydin commented 7 years ago

This pullrequest is gold! Thx paulocns!

emwno commented 7 years ago

@paulocns I found a problem. When scrolling from sub-header to header, the sub overlays the header and doesn't stick under it. Example:

screen shot 2017-01-03 at 5 14 04 am
paulocns commented 7 years ago

@emwno I'm sorry I couldn't see the issue in the picture, could you give me more information?

emwno commented 7 years ago

@paulocns yeah sure. (I'm using the doubleheader btw) When scrolling the sub-header is supposed to "stick" to the bottom of the header and not scroll over it. But actually, the sub-header is taking the place of the header. Here's a gif.

paulocns commented 7 years ago

@emwno please check with the new code, there was a bug for inline header

emwno commented 7 years ago

@paulocns the issue still exists and there is a new one too, the header does no longer animates the subheader out of view when scrolling

paulocns commented 7 years ago

@starkej2 @emwno I fixed for double header inline, i added that case to the sample app. I think we are good to go!

emwno commented 7 years ago

@paulocns The animations now work. But it still does not act like a double-header, rather just a single sticky header. Any ideas?

paulocns commented 7 years ago

@emwno can you send me a gif because I tested and it was working like a double header, take a look at the sample app

emwno commented 7 years ago

@paulocns the only difference between my implementation vs sample is that I am not using the divider decoration. Here's a gif of what I meant

paulocns commented 7 years ago

@emwno ok i saw it, definitely there is some off with your implementation. are you sure that you are setting the header and subheader id for your items? if you see in the gif your header doesn't have a subheader, are you setting your item with subheader but without header?

emwno commented 7 years ago

@paulocns I am not sure what you mean by the last part but all my items have a designated type and this type is checked like so:

if (get(position).getPartType() == TYPE_HEADER) {
   return (long) position;
} else {
   return DoubleHeaderDecoration.NO_HEADER_ID;
}

and its more or less the same for the sub header. Maybe I am assigning the IDs wrong?

paulocns commented 7 years ago

yes, your sub header, must have the same header id, for exemple: item 1 : headerId= 1 subheaderId = NO_SUBHEADER item 2 : headerid= 1 subheaderId = 1 item 3 : headerid= 1 subheaderId = 2 item 4 : headerId= 2 subheaderId = NO_SUBHEADER item 5 : headerid= 2 subheaderId = 3 item 6 : headerid= 2 subheaderId = 4

emwno commented 7 years ago

@paulocns oh, I understand now, thanks. I have made the changes to the assignment of the header IDs but there is this new animation glitch. Here's a gif. Is this related to the assignment of the sub header IDs (my imp)?

paulocns commented 7 years ago

@emwno i think so, i did some tests on the sample and wasn`t able to reproduce the issue, if you prefer give me read permission to your project and let me see what is the problem.

emwno commented 7 years ago

@paulocns I do not have the project on github. I have attached the adapter, let me know if you require any additional material (thank you) TestAdapter.txt

paulocns commented 7 years ago

Hello @emwno I took a look at you code and the IDs should be fine, did you get the latest code I did a commit to fix a problem similar to that one just after I fixed the animation with all items with header and subheader

emwno commented 7 years ago

@paulocns I thought I did, but I guess I didn't because its working perfectly now! Thanks a lot for taking out the time to help me out!

paulocns commented 7 years ago

@emwno Great, glad to help!!!!

emwno commented 7 years ago

Hi @paulocns, I found another bug: if the height of the sub-header view is near the same height as the sub-header (in my case 32 - 30dp), the sub-header is not animated properly. Here's a gif (same as the first bug I posted here)

paulocns commented 7 years ago

@emwno sorry the description is not clear, could you elaborate a little more?

emwno commented 7 years ago

@paulocns Here's a gif explaining what I mean. This only happens if the height of a sub-header view is less than a certain amount.

starkej2 commented 6 years ago

Closing this for now until someone gets a chance to confirm/fix the bug that was found.