Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

Support GridLayoutManager #106

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Support GridLayoutManager #106

wants to merge 4 commits into from

Conversation

Mariovc
Copy link

@Mariovc Mariovc commented Feb 1, 2016

Quick fix to support GridLayoutManager (without reverse mode), but with the condition of the adapter must draw the first item of each section in a new row.

@@ -205,12 +205,9 @@
<item>Egyptian Mau</item>
<item>Electric Eel</item>
<item>Elephant</item>
<item>Elephant Seal</item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a need to remove sample data to make this work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah because each header needs to have a multiple of 3 items?

@jacobtabak
Copy link
Contributor

Cool - definitely a good start. We will need to overcome the condition you mentioned before merging this, though. That implementation detail should be abstracted from users of the library.

@Mariovc
Copy link
Author

Mariovc commented Feb 2, 2016

I have removed the condition. The header of the items will correspond to the first item of the row, even if it is not reordered with the method "reorderItems" I've created.

@jacobtabak
Copy link
Contributor

Hi, sorry for the delay in reviewing this. Your code for reordering these items lives within the sample, not the library itself, so the burden of all this extra logic still falls upon the developer.

@Mariovc
Copy link
Author

Mariovc commented Feb 13, 2016

Like I said before. The sticky-headers will work even if the items are not reordered. I considered that reorder or not the items, is a desicion that falls upon the developer, but in both cases you will have sticky-headers

@MartinRajniak
Copy link

Hi, @Mariovc thanks for the fix, I am using it in our project and it works fine. The only thing that I also had to do is to add empty items to create sections before headers (something like you do in your reorder code). Maybe this is the last thing missing for it to be merged.

That would unfortunately mean that we would need to extend some BaseAdapter class instead of just implementing interface. I don't know if this is something with which would author, @jacobtabak, agree with, but it would be great to switch to stable master again :)

Copy link

@mrlem mrlem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I may, in StickyRecyclerHeadersDecoration, instead of:
if (position == RecyclerView.NO_POSITION || position % mAdapter.getNumColumns() > 0) {

Couldn't you just do:
if (position == RecyclerView.NO_POSITION || itemView.getLeft() != 0) {

(not 100% sure if the left position of the left most item can be something else, but it does work for me)

This way you'd avoid all the nasty numcolumns business, sparing your user a lot of work.

@hendrawd
Copy link

hendrawd commented Jun 8, 2017

Too bad it's not maintained anymore, since this functionality is a good feature to implement i think. I will try it, since i need for grid layout too

@hendrawd
Copy link

hendrawd commented Jun 8, 2017

@MartinRajniak yes, unfortunatelly i got the same problem as you. It would be great if this pull request can also fix the problem.

Expected

What happened instead

@Mariovc
Copy link
Author

Mariovc commented Jun 8, 2017

@hendrawd Maybe I can do that, but this pull request will not be merged probably.

Anyway the user has to provide two criteria to the library:

  1. The criteria to order (date order, recent first)
  2. The criteria to create a new sticky header (grouped by day)

@hendrawd
Copy link

hendrawd commented Jun 8, 2017

@Mariovc yes too bad :(. Actually i was cloning your github repo directly and import it as a module since this repo is inactive

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants