Color downloaded torrents differently #58
Conversation
acheronfail
force-pushed
the
feat/color-completed-torrents
branch
2 times, most recently
from
fae1166
to
fae15dc
Compare
| val progress = (torrent.percentDone * 100).toInt() | ||
| progressBar.progress = progress | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { | ||
| progressBar.progressTintList = if (progress == 100) { |
There was a problem hiding this comment.
AFAIK minimum API here is L, not M.
For pre-Lollipop versions we could probably use the same solution as in ProgressBar.fixPreLollipopColor() extension function in ui/utils/Extensions.kt (and make it more generic function).
There was a problem hiding this comment.
On the other hand, I think I'm ready to drop Android 4.x support.
There was a problem hiding this comment.
AFAIK minimum API here is L, not M.
This is what Android Studio told me... 🤷♂️ 😅 Happy to change it if you'd like.
On the other hand, I think I'm ready to drop Android 4.x support.
So should I leave it as is?
There was a problem hiding this comment.
This is what Android Studio told me... man_shrugging sweat_smile Happy to change it if you'd like.
I think it was for getColor() call, which indeed requires API 23. Luckily, there is ContextCompat.getColor() which handles this for us.
So should I leave it as is?
I already raised minSdkVersion to 21, so with ContextCompat.getColor() we can just drop version check.
There was a problem hiding this comment.
Also, we need to call setProgressTintList() only when isFinished changes, since AFAIK it can perform some expensive operations even when passed ColorStateList is the same as before. It is especially important since update() is called from onBindViewHolder() which will be called frequently when scrolling (we can just cache isFinished value inside TorrentsViewHolder).
acheronfail
force-pushed
the
feat/color-completed-torrents
branch
2 times, most recently
from
fae193b
to
fae1543
Compare
| val progress = (torrent.percentDone * 100).toInt() | ||
| progressBar.progress = progress | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { | ||
| progressBar.progressTintList = if (progress == 100) { |
There was a problem hiding this comment.
This is what Android Studio told me... man_shrugging sweat_smile Happy to change it if you'd like.
I think it was for getColor() call, which indeed requires API 23. Luckily, there is ContextCompat.getColor() which handles this for us.
So should I leave it as is?
I already raised minSdkVersion to 21, so with ContextCompat.getColor() we can just drop version check.
| progressBar.progressTintList = if (progress == 100) { | ||
| ColorStateList.valueOf(context.getColor(R.color.color_downloaded_torrents)) | ||
| } else { | ||
| ColorStateList.valueOf(context.getColor(R.color.color_primary)) |
There was a problem hiding this comment.
You can just use null here, to reset progressTintList.
There was a problem hiding this comment.
It seems that Android sets default tint with primary color so we indeed need to restore it.
Anyway, we need to get the color from R.attr.colorPrimary theme attribute instead of raw color resource since there is two themes with different primary colors. We can do it either by using MaterialColors helper or directly from theme.
| @@ -125,7 +127,16 @@ class TorrentsAdapter(private val fragment: TorrentsListFragment) : | |||
| } | |||
| etaTextView.text = FormatUtils.formatDuration(context, torrent.eta) | |||
|
|
|||
| progressBar.progress = (torrent.percentDone * 100).toInt() | |||
| val progress = (torrent.percentDone * 100).toInt() | |||
There was a problem hiding this comment.
I think Torrent.isFinished is better suited for this check.
| val progress = (torrent.percentDone * 100).toInt() | ||
| progressBar.progress = progress | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { | ||
| progressBar.progressTintList = if (progress == 100) { |
There was a problem hiding this comment.
Also, we need to call setProgressTintList() only when isFinished changes, since AFAIK it can perform some expensive operations even when passed ColorStateList is the same as before. It is especially important since update() is called from onBindViewHolder() which will be called frequently when scrolling (we can just cache isFinished value inside TorrentsViewHolder).
I'm not familiar with Android development, so I'm not sure if there are any APIs I can use that support our minimum build target...
Let me know if there's anything you'd like me to do. 🙂