-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
181512365 airtime bonus #477
Conversation
alexosugo
commented
May 17, 2022
- Adds support for promo channels (bonus airtime)
- Minor UI fixes and improvements.
…n from channel list
import kotlinx.coroutines.launch | ||
import timber.log.Timber | ||
|
||
class BonusViewModel(val repo: BonusRepo, private val dbRepo: DatabaseRepo) : ViewModel() { |
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.
Really like your code style and naming
import kotlinx.coroutines.launch | ||
import timber.log.Timber | ||
|
||
class BonusViewModel(val repo: BonusRepo, private val dbRepo: DatabaseRepo) : ViewModel() { |
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.
Really like your code style.
fun ImageView.loadImage(fragment: Fragment, url: String) = GlideApp.with(fragment) | ||
.load(url) | ||
.placeholder(R.drawable.icon_bg_circle) | ||
.circleCrop() | ||
.into(imageView) | ||
.override(100) | ||
.into(this) | ||
|
||
fun loadImage(context: Context, url: String, imageView: ImageView) = GlideApp.with(context) | ||
fun ImageView.loadImage(context: Context, url: String) = GlideApp.with(context) | ||
.load(url) | ||
.placeholder(R.drawable.icon_bg_circle) | ||
.circleCrop() |
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.
Nice!
fun ImageView.loadImage(fragment: Fragment, url: String) = GlideApp.with(fragment) | ||
.load(url) | ||
.placeholder(R.drawable.icon_bg_circle) | ||
.circleCrop() | ||
.into(imageView) | ||
.override(100) | ||
.into(this) | ||
|
||
fun loadImage(context: Context, url: String, imageView: ImageView) = GlideApp.with(context) | ||
fun ImageView.loadImage(context: Context, url: String) = GlideApp.with(context) | ||
.load(url) | ||
.placeholder(R.drawable.icon_bg_circle) | ||
.circleCrop() |
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.
Nice!
app:layout_constraintEnd_toStartOf="@id/icon" | ||
app:layout_constraintStart_toStartOf="parent" | ||
app:layout_constraintTop_toBottomOf="@id/title" | ||
tools:text="Get 8% bonus when you buy airtime with Stax." /> |
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.
Minor, but maybe to use a string resource.
app:layout_constraintEnd_toStartOf="@id/icon" | ||
app:layout_constraintStart_toStartOf="parent" | ||
app:layout_constraintTop_toBottomOf="@id/title" | ||
tools:text="Get 8% bonus when you buy airtime with Stax." /> |
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.
Minor, but maybe to use a string resource.
|
||
val hniList = mutableSetOf<String>() | ||
bonusChannels.forEach { channel -> | ||
channel.hniList.split(",").forEach { |
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.
I understand what this is, but can we split have this in its own function, and use a declarative name so that it's easier to understand what's going on.
|
||
val hniList = mutableSetOf<String>() | ||
bonusChannels.forEach { channel -> | ||
channel.hniList.split(",").forEach { |
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.
I understand what this is, but can we split have this in its own function, and use a declarative name so that it's easier to understand what's going on.
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.
The code looks good. Left a couple of comments. But not a blocker