Adding optional images for left-to-right languages#1
Conversation
capezzbr
left a comment
capezzbr
left a comment
There was a problem hiding this comment.
Good stuff, left some comments, let me know what you think
| final int index = i; | ||
| Glide.with(mTagContainerLayout1.getContext()) | ||
| .asBitmap() | ||
| .load("https://d1marr3m5x4iac.cloudfront.net/images/block/movies/17214/17214_aa.jpg") |
There was a problem hiding this comment.
If the image is a big one, remember to ask Glide to resize it for you, so that this will get cached resized, which will improve performances
| .into(new SimpleTarget<Bitmap>() { | ||
| @Override | ||
| public void onResourceReady(Bitmap resource, Transition<? super Bitmap> transition) { | ||
| mTagContainerLayout1.getTagView(index).setImage(resource); |
There was a problem hiding this comment.
I think you might want to update the code to:
- move the
forloop within this block
or
- add a
sleepafter each iteration
since as it is right now, I'm afraid can be inclined to some concurrency issues
There was a problem hiding this comment.
Leaving for loop as is, as I meant to showcase various images - I've updated the code to do this.
I added a sleep in, and refactored this code a bit (because that function was getting giant).
Can you explain a bit more about why you're concerned about concurrency here?
There was a problem hiding this comment.
I guess I was just worried about the fact that we're asking Glide to fetch the same assets in a for loop. For this demo purpose I really like your updated solution, 👍
|
LGTM 👍 thanks for updating 🙌 |
2 participants
Please review @capezzbr
Attn: @Expensify/mobile
This adds images for our Android Tag View.
TagContainerLayouttakes a default image, which then propagates to theTagViews.TagView, leaving space for the border around them.Please make sure to try changing the image URL in the test project - I was having some issues in with image requests from new servers (that the simulator had not used before) failing, which may be related to some sort of caching from Glide. (If I relaunched the simulator, it would resolve itself for those same images.) I've seen reports of similar behavior when searching for a solution, but after a clean build it started working just fine. This should still be well-tested, though.