Allow default layout res to be specified in class annotation #109
Conversation
elihart
force-pushed
the
eli/generate_default_layout
branch
from
d1afdf5 to
3396066
Compare
| ", addedToAdapter=" + addedToAdapter + | ||
| '}'; | ||
| return getClass().getSimpleName() + "{" | ||
| + "id=" + id |
There was a problem hiding this comment.
Is this the correct Airbnb java open source formatting? I can start using this in Lottie if that's the case.
There was a problem hiding this comment.
Yes, this is the formatting I got from Felipe. Although I think I may have changed a few things to my preference. In general we should follow the google java style guide.
| * A layout resource that should be used as the default layout for the model. If you set this you | ||
| * don't have to implement `getDefaultLayout`; it will be generated for you. | ||
| */ | ||
| @LayoutRes int value() default 0; |
There was a problem hiding this comment.
Is it possible to have this be a named annotation property but also be the default?
So, you could do:
@EpoxyModelClass(R.layout.hello_world)
or
@EpoxyModelClass(layoutRes=R.layout.hello_world, futureProperty=true)
?
There was a problem hiding this comment.
No unfortunately :( If you want to omit the parameter name it has to be called value http://stackoverflow.com/questions/11786354/how-i-can-do-java-annotation-like-nameluke-with-no-attribute-inside-parenth
There was a problem hiding this comment.
@elihart what would it look like in the future if you want to add more parameters?
There was a problem hiding this comment.
Hm, the easy option would be @EpoxyModelClass(value=R.layout.hello_world, futureProperty=true) which isn't great.
The other option is to rename the param from value to layoutRes in the future if we other params. That'd be a breaking change we'd have to force on people. Not too bad to upgrade if we provide a structural replace pattern, but also not great.
Another option is to create a whole new annotation. That could work depending on what the goal of the feature is.
Or we just do @EpoxyModelClass(layoutRes=R.layout.hello_world) now in the anticipation of adding future params.
That last option is the safest. It's not terrible to require the param name, but it would be a bit annoying. I supposed the param name would make the purpose clearer.
Thanks for bringing this up, hadn't considered it.
There was a problem hiding this comment.
@elihart Personally I'm a fan because it's the most explicit and is future proof. Luckily IntelliJ should be able to autocomplete the name as well.
There was a problem hiding this comment.
Yeah, I think you're right. I'll change it. Thanks!
elihart
force-pushed
the
eli/generate_default_layout
branch
from
3396066 to
f4e387b
Compare
This lets us get rid of even more boilerplate by not writing the
getDefaultLayoutmethod in models.