Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added getSpanSize and getItemId functionality. #27

Closed
wants to merge 3 commits into from

Conversation

YeungKC
Copy link

@YeungKC YeungKC commented Nov 19, 2016

Added getSpanSize and getItemId functionality.

@YeungKC YeungKC changed the title Added getSpanSize functionality. Added getSpanSize and getItemId functionality. Nov 20, 2016
@sockeqwe
Copy link
Owner

sockeqwe commented Nov 21, 2016

Hi, thanks for your pull request.
Adding getItemId() looks good to me.
I'm not sure if we should add this the spansize lookup directly to the AdapterDelegate.
What if we want to have differen spanSize lookps depending on different screen sizes? i.e. Phone or Tablet? Would you then return R.integer.fooSpanSize and have different definitions in res folder for phone and tablet?
Also SpanSizeLookup is bound to GridLayoutManager and is not a general RecyclerView API.

#25 suggests to follow a different approach. What do you think?

@YeungKC
Copy link
Author

YeungKC commented Nov 21, 2016

Hi, I just think this library can do better.

We can do that with your question:

final GridLayoutManager gridLayoutManager = new GridLayoutManager(this, getResources().getInteger(R.integer.spanCount));
gridLayoutManager.setSpanSizeLookup(new GridLayoutManager.SpanSizeLookup() {
  @Override
  public int getSpanSize(int position) {
    return adapter.delegatesManager.getSpanSize(adapter.items, position, gridLayoutManager.getSpanCount());
  }
});

if AdapterDelegate need return different spanSize , just let the AdapterDelegate hanlde.

for example:

  public  int getSpanCount(@NonNull T items, int position, int spanCount){
    return CustomApplication.contenxt.getInteger().getInteger(R.integer.fooSpanSize);
  }

I should add these(T items, int position) parameters.

and

maybe create method in AbsDelegationAdapter for ease of use:

public GridLayoutManager.SpanSizeLookup getSpanSizeLookup(final int spanCount) {
  return new GridLayoutManager.SpanSizeLookup() {
    @Override
    public int getSpanSize(int position) {
      return delegatesManager.getSpanSize(items, position, spanCount);
    }
  };
}

About #25 suggestion, I think the specific parameters(T items, int position, int spanCount) provided by getSpanSize() can satisfy the requirement in most cases, so it unnecessary to add SpanSizeManager and SpanSizeDelegate if cannot find a perfect solution for it. Maybe keep it in a simple way is better.

then about ItemDecoration issue, I agree with your opinion.

Finally, I'm not a native English speaker, so I do not even have a comment to write ... I hope you do not mind.

add getSpanSizeLookup() to AbsDelegationAdapter
change getSpanCount() and getItemId() parameters
change getSpanCount() and getItemId() public to protected
@YeungKC YeungKC closed this Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants