-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
speaker list avatar fix #28
base: master
Are you sure you want to change the base?
speaker list avatar fix #28
Conversation
Fixes 360Conferences#26: When loading the speaker list, avatar images would pop in late, or have a "ghost" from the previous bind. The viewholder was requesting a `downloadUrl` for each avatar on bind. This switches to using coroutines to map the avatar download url into the `Speaker` model itself. Doing this lets the recycler have all the data it needs to bind its viewholders.
}) | ||
override suspend fun getAvatarUri(speaker: Speaker) = suspendCoroutine<String> { c -> | ||
val id = speaker.id ?: "" | ||
if (id.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can just be id == null
to get rid of the ?:
above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, I think we're ready to coroutine all the things ;)
@@ -10,7 +10,8 @@ class Speaker(val id: String? = null, | |||
val email: String? = null, | |||
val twitter: String? = null, | |||
val github: String? = null, | |||
val bio: String? = null) { | |||
val bio: String? = null, | |||
val downloadUrl: String? = null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For clarity could you please rename this to something like avatarUrl
?
8e54b87
to
987a1fa
Compare
All ready |
Fixes #26