-
Notifications
You must be signed in to change notification settings - Fork 742
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
Updating use case icons to match designs #5025
Conversation
- programmatically creates a layer drawable to avoid introducing new colours into the palette
Matrix SDKIntegration Tests Results:
|
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.
Thanks for this new PR.
Just some small remarks, not blocking!
|
||
fun Context.singletonEntryPoint(): SingletonEntryPoint { | ||
return EntryPoints.get(applicationContext, SingletonEntryPoint::class.java) | ||
} | ||
|
||
fun Context.getResTintedDrawable(@DrawableRes drawableRes: Int, @ColorRes tint: Int, alpha: Float? = null): Drawable? { |
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.
Nit: you could annotate the param alpha
with @FloatRange(from = 0.0, to = 1.0)
Nit2: maybe defaulting the param alpha
to the value 1.0
to avoid making it nullable?
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.
} | ||
|
||
private fun Float.toAndroidAlpha(): Int { | ||
return round(this * 255).toInt() |
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.
Call to round
could probably be omitted, but I am not sure.
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've merged with the toInt
57be0ba
@@ -125,7 +123,6 @@ | |||
android:padding="16dp" | |||
android:text="@string/ftue_auth_use_case_option_three" | |||
android:textColor="?vctr_content_primary" | |||
app:drawableStartCompat="@drawable/ic_communities" |
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.
Could be nice to keep those attributes but using the namespace tools
, for a better later preview.
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.
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.
Why not using ic_use_case_communities
and the 2 other drawables for the preview?
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.
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 ic_use_case_*
drawables have intrinsic padding which can be a little misleading, I guess it depends if we would prefer the design preview to show relative sizing or the context of the image
no preference from my side!
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 see, OK
return getTintedDrawable(drawableRes, ContextCompat.getColor(this, tint), alpha) | ||
} | ||
|
||
fun Context.getTintedDrawable(@DrawableRes drawableRes: Int, @ColorInt tint: Int, alpha: Float? = null) = ContextCompat.getDrawable(this, drawableRes) |
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.
Same remark.
- this is only the background layer though as the real image is programatically constructed
52dc530
to
203c7e0
Compare
(CI is not happy: |
Part of #4585