luizgrp / SectionedRecyclerViewAdapter

An Adapter that allows a RecyclerView to be split into Sections with headers and/or footers. Each Section can have its state controlled individually.
MIT License
1.68k stars 372 forks source link

Support for resource-less View creation #98

Closed Philipp91 closed 6 years ago

Philipp91 commented 6 years ago

PR TEMPLATE

Use case

Currently, the SectionedRecyclerViewAdapter holds the @LayoutRes IDs for the different view types (item/header/footer/loading/failed/empty) and inflates them whenever necessary. However, there are use cases when views need to be created with more or less logic than this simple inflation:

android.view.InflateException: Binary XML file line #0: <merge /> can be used only with a valid ViewGroup root and attachToRoot=true
  Caused by: android.view.InflateException: <merge /> can be used only with a valid ViewGroup root and attachToRoot=true
  at android.view.LayoutInflater.inflate(LayoutInflater.java:488)
  at android.view.LayoutInflater.inflate(LayoutInflater.java:426)
  at io.github.luizgrp.sectionedrecyclerviewadapter.SectionedRecyclerViewAdapter.getItemViewHolder(SectionedRecyclerViewAdapter.java:84)
  at io.github.luizgrp.sectionedrecyclerviewadapter.SectionedRecyclerViewAdapter.onCreateViewHolder(SectionedRecyclerViewAdapter.java:59)

Design

The proposed design only has minimal impact on the user-facing API of the library. In the majority of use cases, the above does not apply and no additional logic is necessary. Therefore, the proposed design does not affect existing users and does not change the API for the regular use case.

No new user-facing functions are added/changed. But when the user supplies the special value 0 as the layout resource ID (note that null already means "no footer/header/...), the SectionRecyclerViewAdapter should not inflate anything and leave the view creation entirely up to the Section.getXyzViewHolder() implementation.

In addition, an internal helper method could be useful to encapsulate this common logic (detection of 0 and reacting accordingly).

How to use

Adapted from the first example in README.md:

class MySection extends StatelessSection {
    List<String> itemList = Arrays.asList("Item1", "Item2", "Item3");

    public MySection() {
        // call constructor with layout resources for this Section header and items
        super(new SectionParameters.Builder(0) // <----------- NOTE THE ZERO HERE
                .headerResourceId(R.layout.section_header)
                .build());
    }

    @Override
    public int getContentItemsTotal() {
        return itemList.size(); // number of items of this section
    }

    @Override
    public RecyclerView.ViewHolder getItemViewHolder(View view) {
        // NOTE: Now the `view` is just the parent.
        return new RecyclerView.ViewHolder(new TextView(view.getContext()));
    }

    @Override
    public void onBindItemViewHolder(RecyclerView.ViewHolder holder, int position) {
        ((TextView) holder.itemView).setText(itemList.get(position));
    }
}
luizgrp commented 6 years ago

Hi @Philipp91 ,

Thanks for your contribution!

Use case: I'm sold, thanks for all examples.

Design: I appreciate you designed it thinking in having minimal impact on the user-facing API but:

I would suggest the following:

Design

Users can opt to be responsible for providing the instance of the views themselves instead of letting the adapter inflate the views based on the views id. They will indicate it explicitly through SectionParameters.Builder. They will provide the instance of the views overriding get*View methods of Section class.

SectionParameters.Builder
SectionParameters
Section
StatelessSection
SectionedRecyclerViewAdapter

How to use

Adapted from your "How to use":

class MySection extends StatelessSection {
    List<String> itemList = Arrays.asList("Item1", "Item2", "Item3");

    public MySection() {
        // call constructor with layout resources for this Section header and items
        super(new SectionParameters.Builder()
                .itemViewWillBeProvided()
                .headerResourceId(R.layout.section_header)
                .build());
    }

    @Override
    public int getContentItemsTotal() {
        return itemList.size(); // number of items of this section
    }

    @Override
    public View getItemView(ViewGroup parent) {
        return new TextView(parent.getContext());
    }

    @Override
    public RecyclerView.ViewHolder getItemViewHolder(View view) {
        return new RecyclerView.ViewHolder(view);
    }

    @Override
    public void onBindItemViewHolder(RecyclerView.ViewHolder holder, int position) {
        ((TextView) holder.itemView).setText(itemList.get(position));
    }
}

Let me know your thoughts.

Philipp91 commented 6 years ago

Sounds good. Please clarify the question below and I'll try and implement it.

// EDIT: I earlier had considerations about my own surrounding code design in this comment, but those turned out to be wrong, so I removed that part. Either solution works for me.

I'm not sure how you meant the RecyclerView.ViewHolder getItemView() method. What you wrote wouldn't compile.

A) Do you mean this:

public RecyclerView.ViewHolder getItemView(ViewGroup parent) {
    return new RecyclerView.ViewHolder(new TextView(parent.getContext()));
}

This approach is a more flexible, as it allows the implementor to delegate both view and holder creation to a single place (where some mingled magic happens). Multiple views might be created and put into the ViewHolder separately (where only one of them is the top-level itemView), and so on.

But I'd say, for this variant, the function name is confusing and we should try and find a better one. E.g. createItemViewAndHolder() or sth like that.

B) Or do you mean this:

public View getItemView(ViewGroup parent) {
    return new TextView(parent.getContext());
}

and then the SectionedRecyclerViewAdapter combines both methods like this (pseudo-code): getItemViewHolder(provided ? getItemView(parent) : inflate(...)).

This variant is less flexible, as it forces the implementor to first (and independently) create a single View, and to then wrap it in a ViewHolder.

luizgrp commented 6 years ago

I meant B, just updated my comment in order to avoid confusion.

Are you sure it wouldn't work for your case? If so, your initial proposal wouldn't work either. Section.getItemView would be called at the same point of when Section.getItemViewHolder is called, from SectionedRecyclerViewAdapter.getItemViewHolder. We are just splitting into two methods to avoid the confusion with the parent view. Let me know if it makes sense.

Philipp91 commented 6 years ago

Makes sense. I was able to change my (slightly messy) application design so that it works. For properly designed applications, this shouldn't be a problem in the first place.

I made the following changes to your proposal:

Unit tests have run for a long time (30m+) without progressing from 20%, so I'm just uploading and hoping for CI in the cloud to figure out if anything is going wrong.