ANDROID-14098 set asset dimensions #322
Conversation
jeprubio
requested review from
a team,
yamal-alm,
pmartinbTEF and
jeslat
and removed request for
a team and
pmartinbTEF
| assetImageView.isVisible = assetType == TYPE_SMALL_ICON || assetType == TYPE_LARGE_ICON | ||
| } | ||
|
|
||
| fun setAssetType(@AssetType type: Int) { | ||
| fun setAssetType(@AssetType type: Int, dimensions: ImageDimensions? = null) { |
There was a problem hiding this comment.
Appart from programatic configuration we expose also configurable elements with xml attributes. Check ListRowView styleable in attrs_components.xml to add new asset type enum and height/width attributes. Ideally if databinding method matches attribute definition behaviour, its better, so there's no difference in the usage when using databinding and when not.
There was a problem hiding this comment.
Sure, I forgot doing that, I'll do that now. Thanks!
There was a problem hiding this comment.
Is it working the binding method with this new parameter? Or is it just ignored because is nullable?
There was a problem hiding this comment.
That one of the fun setAssetType will be null with binding and there are the other two of setAssetHeight and setAssetWidth to set the dimensions. That param is in order to use it when creating the element programmatically, but can be changed, it was thought as a shortcut.
|
Update also the READMEs that we have for each component |
| <attr name="listRowAssetHeight" format="integer"> | ||
| <enum name="undefined" value="64" /> | ||
| </attr> | ||
| <attr name="listRowAssetWidth" format="integer"> | ||
| <enum name="undefined" value="64" /> |
There was a problem hiding this comment.
should these attrs be of type "dimension" in case someone uses them as follows?
listRowAssetWidth="@dimen/some_width"There was a problem hiding this comment.
+1, I think it should be dimension to avoid putting just a raw number, we should use dp
There was a problem hiding this comment.
Yup, I can do this now. Good shout.
There was a problem hiding this comment.
Done, both in 1c1ee2b
| import com.telefonica.mistica.R | ||
|
|
||
| @RunWith(RobolectricTestRunner::class) | ||
| internal class ListRowItemKtTest: ScreenshotsTest() { |
There was a problem hiding this comment.
Just curious, why that Kt in the middle of the class name?
There was a problem hiding this comment.
When you create a test with Android Studio it creates them this way sometimes, I'm not sure which rules does it follow. I can remove that if you want.
|
📱 New catalog for testing generated: Download |
🎟️ Jira ticket
ANDROID-14098
🥅 What's the goal?
Be able to set asset dimensions in ListRow.
🚧 How do we do it?
Trying not to add breaking changes:
☑️ Checks
🧪 How can I test this?
Traditional views:
Compose: