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

Handling different View Types in the FirebaseRecyclerAdapter #305

Closed
giansegato opened this issue Sep 14, 2016 · 10 comments
Closed

Handling different View Types in the FirebaseRecyclerAdapter #305

giansegato opened this issue Sep 14, 2016 · 10 comments
Labels

Comments

@giansegato
Copy link

I'm working on a messaging app, and I think that overriding the original getItemViewType(int) in order to handle different view types (eg. sent vs received text layouts) can complicate code with no clear benefit. You can also have a single layout file and hide/show different elements in the populateViewHolder(VH, int, int) method, but I find it an even worse choice.

I'd like to change this, introducing a new constructor: The adapter will handle the possibility of having more than just one view model.

@puf
Copy link
Contributor

puf commented Sep 14, 2016

I'm generally not in favor of adding more overloads to the constructors,
unless those will be used by a reasonable subset of the developers using
FirebaseUI. But a builder might be a way out there, but more likely this
could be a dedicate subclass of the existing adapter(s).

Can you (in this issue) show how you would use this expanded class with
what type of data structure and how it would decide which view type to
instantiate?

On Tue, Sep 13, 2016 at 11:54 PM giansegato notifications@github.com
wrote:

I'm working on a messaging app, and I think that overriding the original
getItemViewType(int) in order to handle different view types (eg. sent vs
received text layouts) can complicate code with no clear benefit. You can
also have a single layout file and hide/show different elements in the populateViewHolder(VH,
int, int) method, but I find it an even worse choice.

I'd like to change this, introducing a new constructor: The adapter will
handle the possibility of having more than just one view model.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#305, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AA3w31KkboMmmgpSZggU9aXAl_RAwcMUks5qp5oTgaJpZM4J8dEb
.

@giansegato
Copy link
Author

giansegato commented Sep 14, 2016

I see your point. Maybe a dedicated subclass might be a very good solution, in order to avoid unnecessary boilerplate.

My original idea was a constructor like this

public FirebaseRecyclerAdapter(Class<T> modelClass, List<Integer> modelsLayout, List<Class<VH>> viewHolderClasses, Query ref)

in which I would set mModelLayout = -1; and mViewHolderClass = null;

In this way, I can change onCreateViewHolder and getItemViewType in order to handle both the functionalities. Something like this:

@Override
public VH onCreateViewHolder(ViewGroup parent, int viewType) {
    ViewGroup view = (ViewGroup) LayoutInflater.from(parent.getContext()).inflate(viewType, parent, false);
    try {
        Constructor<VH> constructor = mViewHolderClass != null ?
                mViewHolderClass.getConstructor(View.class) :
                mViewHolderClasses.get(mModelsLayout.indexOf(viewType)).getConstructor(View.class);
        return constructor.newInstance(view);
    } catch (IndexOutOfBoundsException e) {
        throw new RuntimeException(e);
    } /* [...] */
}
@Override
public int getItemViewType(int position) {
    if (mModelLayout == -1)
        throw new RuntimeException("You must override getItemViewType if you provide more than one view types.");
    return mModelLayout;
}

I assume that the items in the list of layouts have the same index of the items in the list of ViewHolders. And I also assume that the developers might want to add their own logic when deciding which view type to apply, depending on the data.

However, a subclass might be a cleaner solution, using the same pattern applied by onBindViewHolder:

@Override
public int getItemViewType(int position) {
    return decideViewType(position);
}

abstract protected void decideViewType(int position);

A third approach could be seeing the multiple-views case as an extension of the particular case of a single view. In this sense, the single-view constructor could create a list with just a single item in it.

Edit: On a second thought, I think that instead of two lists, it would be better to just provide a map: Map<Integer, VH>.

@Sottti
Copy link

Sottti commented Nov 30, 2016

This would be great to have! I think it's quite a common thing.

@morphine9
Copy link

+1 and hopefully transitively forwarded to FirebaseIndexRecyclerAdapter
Overriding getItemViewType(int position), populateViewHolder in FirebaseIndexRecyclerAdapter doesn't help since getItemViewType is called before populateViewHolder which doesn't have the model information untill the next pass when a new element inserted because of the index query. As shown above, should work well

@Override
public int getItemViewType(int position) {
    return decideViewType(position);
}

abstract protected void decideViewType(int position);

@brad007
Copy link

brad007 commented Feb 7, 2017

This needs to be done! If it won't be done here, has anyone done it in their own code and are they willing to share it?

@SourceCipher
Copy link

I have been hitting my head trying to work my way out to have 2 different styles inside FirebaseRecyclerAdapter. I hope they will add this future soon!

@SUPERCILEX
Copy link
Collaborator

@samtstern I believe this issue can be closed since it falls under the category of "the developer can do this themselves fairly easily and implementing it would make things too complicated for users who just want a quick solution".

@gwidaz @brad007 @Sottti I know this issue has been open for a while, but if any of you are still in need of a fix, here's how you would do it:

public class ScoutAdapter extends FirebaseRecyclerAdapter<ScoutMetric, ScoutViewHolderBase> {
    public ScoutAdapter(Query query) {
        super(ScoutUtils.METRIC_PARSER, 0 /* modelLayoutLayoutId: can be anything, it doesn't matter since we are overriding it */, ScoutViewHolderBase.class, query);
        // ...
    }

    @Override
    public void populateViewHolder(ScoutViewHolderBase viewHolder,
                                   ScoutMetric metric,
                                   int position) {
        //noinspection unchecked
        viewHolder.bind(metric, /* ... */);
    }

    /** 
      * This will require a refactor on your part, but the basic idea is to inflate your layout and
      * pass that into your ViewHolder for each different view.
      */
    @Override
    public ScoutViewHolderBase onCreateViewHolder(ViewGroup parent, @MetricType int viewType) {
        LayoutInflater inflater = LayoutInflater.from(parent.getContext());
        switch (viewType) {
            case MetricType.BOOLEAN:
                return new CheckboxViewHolder(
                        inflater.inflate(R.layout.scout_checkbox, parent, false));
            case MetricType.NUMBER:
                return new CounterViewHolder(
                        inflater.inflate(R.layout.scout_counter, parent, false));
            case MetricType.TEXT:
                return new EditTextViewHolder(
                        inflater.inflate(R.layout.scout_notes, parent, false));
            case MetricType.LIST:
                return new SpinnerViewHolder(
                        inflater.inflate(R.layout.scout_spinner, parent, false));
            case MetricType.STOPWATCH:
                return new StopwatchViewHolder(
                        inflater.inflate(R.layout.scout_stopwatch, parent, false));
            case MetricType.HEADER:
                return new ScoutViewHolderBase<Void, TextView>(
                        inflater.inflate(R.layout.scout_header, parent, false)) {};
            default:
                throw new IllegalStateException();
        }
    }

    @Override
    @MetricType
    public int getItemViewType(int position) {
        // Here you are simply returning an integer to use in onCreateViewHolder.
        // In my case, I'm getting the view type from the database, but you could do it anyway you like.
        return getItem(position).getType(); // This returns one of @MetricType e.g. MetricType.BOOLEAN, etc.
    }
}

If anyone wants to look at my source in a little bit more detail to get a better understanding of how to handle different view types, here's the full ScoutAdapter.java and the package where I keep my ViewHolders.

@samtstern
Copy link
Contributor

@SUPERCILEX I agree with your assessment. While we could do more to support this use case I have not yet seen a way that would be worth the cost.

Additionally, the upcoming changes to FirebaseArray will make it much easier for developers to implement customized adapters on top of our code, so it should enable many use cases.

Thank you to everyone who posted in this issue, I am going to close it to reflect that we don't intend to make any more changes here.

@WillieCubed
Copy link
Contributor

Hey, @SUPERCILEX, may I use the code snippet you provided in a library?

@SUPERCILEX
Copy link
Collaborator

@TheCraftKid yeah of course, don't see why not. 😀

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

No branches or pull requests

10 participants