Skip to content

Conversation

@MaciejCiemiega
Copy link
Contributor

Hello,
in current implementation this cast prevents from inflating just simple TextView as a list item (such as android.R.layout.simple_list_item_1 etc.).

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@MaciejCiemiega
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@samtstern
Copy link
Contributor

@puf I remember a while ago you explained why it had to be this way. Is that still the case? If so could you enlighten us?

@puf
Copy link
Contributor

puf commented Sep 16, 2016

@samtstern that was in #96 . But I'm wondering if that case wasn't different, since here the View/Viewgroup seem local to onCreateViewHolder()

@MaciejCiemiega
Copy link
Contributor Author

It looks like it was like that since the beginning:

ViewGroup view = (ViewGroup) LayoutInflater.from(parent.getContext()).inflate(mModelLayout, parent, false);

@samtstern
Copy link
Contributor

@puf I suggest merging this one, we immediately do this:

Constructor<VH> constructor = mViewHolderClass.getConstructor(View.class);

Which means we are passing the ViewGroup to a method that takes a View, and we don't use the ViewGroup for any other purpose so I see no reason not to relax this.

@puf
Copy link
Contributor

puf commented Oct 7, 2016

I've been starting at this one for a while now, but can't remember why it seemed a bad idea before. The ViewGroup doesn't even escape from the method. Maybe that was another change after all. @samtstern: can you merge?

@puf puf assigned samtstern and unassigned puf Oct 7, 2016
@samtstern samtstern merged commit a86fdb5 into firebase:master Oct 7, 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.

4 participants