Add ViewGroup.inflate extension

[Bhullnatik]

Sorry for opening a PR straight away, but it was so simple I just wrote it.

This is a method I add to every one of my Kotlin projects. The easiest/simplest way is to use it in RecyclerView.Adapter.createViewHolder like so:

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder {
    return RecyclerView.ViewHolder(parent.inflate(R.layout.item, false))
}

but can be used anywhere.

The points I am not sure about:

• It doesn't handle the case where the ViewGroup can be null as it would require passing another Context as a parameter since we can't use the one from the ViewGroup. But that's out of the scope-ish since it's an extension method for ViewGroup?
• Defaulting attachToRoot to true. Mainly to mirror the Android API but as I said the main use case is to use inside a RecyclerView.Adapter in which case it should be false, but I don't think it's a good enough reason/too confusing.

Anyway thanks for the library, very cool. 👍🏾

[romainguy]

Can you be more specific and explain that the inflated view is added as a child to this ViewGroup?

[romainguy]

Incorrect indentation. See https://github.com/android/android-ktx/blob/master/src/androidTest/res/layout/test_attrs.xml for example

[romainguy]

I like attachToRoot being true by default. I would however rename it here to attachToSelf or attachToThis instead? (or any other better idea you might have)

[antoniolg]

Just for consideration. Methods that take a boolean as default are a bit confusing, because one never knows what's the default. This one is specially confusing, as the gut feeling is thinking that the default value is false (as it's mostly used in adapters), but sticking to Android SDK would make one think that the default is true.

What do you think about having two separate methods?

viewGroup.inflate(R.layout.item)
viewGroup.inflateAndAttach(R.layout.item)

The method name says what it's doing without having to go to the code / reference and check what's the default.

[romainguy]

I don't see why a default value for a boolean is any different than for other types? The fact that the default matches Android's existing methods makes sense for consistency (and the default value should be documented).

[antoniolg]

Booleans tend to be confusing for method arguments in general, that was point of my suggestion. There's quite some literature about it: https://martinfowler.com/bliki/FlagArgument.html

But I also see your point, people used to the SDK methods will easily understand this too, and for this library consistency may be better.

[romainguy]

I agree that booleans tend to be confusing but this particular API has been around since 1.0 so I think consistency wins here.

[Bhullnatik]

I agree that the attachToRoot parameter is the original method is not very clear, but having 2 different methods mean that documentation for these methods must be more throroughly written, whereas here a reference to LayoutInflater.inflate documentation is enough imho.

[antoniolg]

Agreed, was a random idea, but true that consistency wins in this case.

[fada21]

I think attachToRoot parameter should be false by default. As @antoniolg mentioned that would be mainly used in Adapters where we want false. Being forced to always add that extra parameter will just make me to add my own extension as for many people. This one might be consistent but not so much concise and pleasant as in project description. I think there is no harm with adding some extra documentation to explain why usability here should win over consistency here.

[romainguy]

I really don't like having inflate methods use different default behaviors.

[tetedoie]

I Agree that we should keep constancy with the platform, attachToRoot is by default true if the viewGroup is not null. In the mean time we need to be able to override this behavior (especially for adapter). Actually, We should go deeper, we only need a context to inflate a view, so we should have three extensions like:

fun Context.inflate(@LayoutRes layoutId: Int, root: ViewGroup? = null, attachToRoot: Boolean = root != null): View = LayoutInflater.from(this).inflate(layoutId, root, attachToRoot)

fun View.inflate(@LayoutRes layoutId: Int, root: ViewGroup? = null, attachToRoot: Boolean = root != null): View = context.inflate(layoutId, root, attachToRoot)

fun ViewGroup.inflate(@LayoutRes layoutId: Int, attachToRoot: Boolean = true): View = inflate(layoutId, this, attachToRoot)

[antoniolg]

As @antoniolg mentioned that would be mainly used in Adapters where we want false

Must say that I use it a lot too in custom views, where the value is usually true.

[clhols]

An alternative is to automatically cast to the receiver type:

@Suppress("UNCHECKED_CAST")
fun <T : View> ViewGroup.inflate(@LayoutRes layoutRes: Int, attachToRoot: Boolean = true): T =
        LayoutInflater.from(context).inflate(layoutRes, this, attachToRoot) as T

[Bhullnatik]

@tetedoie That sounds good! Could allow handling every case, one of the problems I talked about in my comment. But maybe in different PRs so that it can be discussed more easily?

@clhols Not sure if that's very useful, but I don't really see why not. @romainguy How does it look for you?

[antoniolg]

Not sure if that's very useful, but I don't really see why not

In my experience, it's helpful in adapters, where you need to use the result of the inflation (to add it to a VH for instance), but not so much in custom views. Kotlin forces you to specify the type, so if you're not assigning the result to a variabe, you need to make it explicit.

It would look like this on a custom view:

inflate<View>(R.layout.view_custom)

[clhols]

It would be consistent with what was done for findViewById:
https://developer.android.com/about/versions/oreo/android-8.0-changes.html#fvbi-signature

[Bhullnatik]

@clhols Agreed, it makes sense.

@romainguy Still looks good to you?

[ataulm]

@Bhullnatik can you update with the latest master pls?

[Bhullnatik]

@ataulm Done! The api/current.txt is not up-to-date due to #235 though.