Skip to content
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

Add support for theme-attribute-containing CSLs in colorFilter on Lollipop #1551

Closed
ataulm opened this issue Apr 21, 2020 · 0 comments · Fixed by #1708
Closed

Add support for theme-attribute-containing CSLs in colorFilter on Lollipop #1551

ataulm opened this issue Apr 21, 2020 · 0 comments · Fixed by #1708

Comments

@ataulm
Copy link
Contributor

ataulm commented Apr 21, 2020

** If you don't use this template, your issue will be closed **

Is your feature request related to a problem? Please describe.
The colorFilter attr is read using typedArray.getColor. This accommodates both single colors and color state lists (CSLs), with the caveat that if a CSL is used, it'll return the default color.

<selector xmlns:android="http://schemas.android.com/apk/res/android">
    <item android:alpha="0.54" android:color="?attr/colorOnBackground" />
</selector>

On Marshmallow+, the above works great. It's a recommended pattern for define alpha-variants of theme colors, without having to create custom theme attributes (and specifying each of them literally, with the alpha baked in).

On Lollipop, CSLs don't work well if they contain theme attributes - they'll render as red. This issue is to add support for Lollipop by using AppCompatResources to load the color.

Describe the solution you'd like
Read the colorFilter attribute as a resource id, then use AppCompatResources to load the resource in a backwards compatible fashion. This will work for both regular color resources and also for CSLs.

Describe alternatives you've considered
I've wrapped the view and re-apply the color filter - I'm not sure whether using addValueCallback (in addition to the superclass calling it for colorfilter) will mean that there's multiple passes.

class LottieLoadingSpinnerView(context: Context, attrs: AttributeSet) : LottieAnimationView(
        context,
        attrs,
        DEF_STYLE_ATTR
) {
    init {
        // we're reading the colorFilter attr so we can reapply it with support for CSLs on API 21/22
        context.obtainStyledAttributes(attrs, R.styleable.LottieAnimationView, DEF_STYLE_ATTR, DEF_STYLE_RES).apply {
            if (hasValue(R.styleable.LottieAnimationView_lottie_colorFilter)) {
                val colorFilterResId = getResourceId(R.styleable.LottieAnimationView_lottie_colorFilter, -1)
                val colorFilterColorInt = AppCompatResources.getColorStateList(context, colorFilterResId).defaultColor
                val filter = SimpleColorFilter(colorFilterColorInt)
                val keyPath = KeyPath("**")
                val callback = LottieValueCallback<ColorFilter>(filter)
                addValueCallback(keyPath, LottieProperty.COLOR_FILTER, callback)
            }
            recycle()
        }
    }
}

Additional context
Happy to raise a PR if this issue is accepted.

Before After
spinner-before spinner-after

Lottie is supported and developed on nights and weekends. Please consider sponsoring Lottie to prioritize this issue and help support this issue as well as future investments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant