android / android-ktx

A set of Kotlin extensions for Android app development.
https://android.github.io/android-ktx/core-ktx/
7.47k stars 563 forks source link

TextView.setCompoundDrawable... with default arguments #147

Open tomaszrykala opened 6 years ago

tomaszrykala commented 6 years ago

eg.

@RequiresApi(Build.VERSION_CODES.JELLY_BEAN_MR1)
fun TextView.setCompoundDrawablesRelative(
        start: Drawable? = null,
        top: Drawable? = null,
        end: Drawable? = null,
        bottom: Drawable? = null
) {
    setCompoundDrawablesRelative(start, top, end, bottom)
}
// same for the other, similar methods
JakeWharton commented 6 years ago

Name won't work. Extensions cannot override members.

tomaszrykala commented 6 years ago

True, thanks. So replace set with update, eg. fun TextView.updateCompoundDrawablesRelative(...) ?

JakeWharton commented 6 years ago

Yeah. And that's unfortunate. We should do non-relative too.

PRs welcome.

On Mon, Feb 5, 2018 at 5:34 PM Tomasz Rykała notifications@github.com wrote:

True, thanks. So replace set with update, eg. fun TextView.updateCompoundDrawablesRelative(...) ?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/android/android-ktx/issues/147#issuecomment-363244600, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEXrZJpv__FV90srSiasjiRYV7gm3ks5tR4H1gaJpZM4R6L2x .

tomaszrykala commented 6 years ago

Will do, thanks.

fredy-mederos commented 6 years ago

The compound drawables extensions could be "var" extensions like this?

var TextView.drawableTop: Drawable?
    get() = compoundDrawables[1]
    set(value) {
        setCompoundDrawables(
            compoundDrawables[0],
            value,
            compoundDrawables[2],
            compoundDrawables[3]
        )
    }

Thanks

consp1racy commented 6 years ago

@fredy-mederos That's var TextView.compoundDrawableTop.

consp1racy commented 6 years ago

@tomaszrykala null is a valid argument for setting compound drawables. Perhaps we should use a marker object.

private object DoNotChange : Drawable() {
    override fun draw(canvas: Canvas) {
    }

    override fun setAlpha(alpha: Int) {
    }

    override fun getOpacity(): Int = PixelFormat.TRANSPARENT

    override fun setColorFilter(colorFilter: ColorFilter?) {
    }
}

fun TextView.updateCompoundDrawablesWithIntrinsicBounds(
        left: Drawable? = DoNotChange,
        top: Drawable? = DoNotChange,
        right: Drawable? = DoNotChange,
        bottom: Drawable? = DoNotChange
) {
    val compoundDrawables = compoundDrawables
    val newLeft = if (left != DoNotChange) left else compoundDrawables[0]
    val newTop = if (top != DoNotChange) top else compoundDrawables[1]
    val newRight = if (right != DoNotChange) right else compoundDrawables[2]
    val newBottom = if (bottom != DoNotChange) bottom else compoundDrawables[3]
    setCompoundDrawablesWithIntrinsicBounds(newLeft, newTop, newRight, newBottom)
}

It's not as fast and pretty, probably not a great candidate for inlining. getCompoundDrawables() involves cloning so we get just one local copy ahead of time.

And do that for all the intrinsic and non-intrinsic and relative and absolute variants, of course.