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

Fix CompoundButton mapper drawable clone issue #2365

Merged
merged 1 commit into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ internal abstract class CheckableCompoundButtonMapper<T : CompoundButton>(
null
}
}
return originCheckableDrawable?.let { cloneCheckableDrawable(view, it) }
return originCheckableDrawable
}

private fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable? {
override fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable? {
return drawable.constantState?.newDrawable(view.resources)?.apply {
// Set state to make the drawable have correct tint.
setState(view.drawableState)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import android.widget.Checkable
import android.widget.TextView
import androidx.annotation.UiThread
import com.datadog.android.api.InternalLogger
import com.datadog.android.sessionreplay.internal.recorder.resources.DrawableCopier
import com.datadog.android.sessionreplay.model.MobileSegment
import com.datadog.android.sessionreplay.recorder.MappingContext
import com.datadog.android.sessionreplay.recorder.mapper.TextViewMapper
Expand Down Expand Up @@ -109,6 +110,7 @@ internal abstract class CheckableTextViewMapper<T>(
view,
mappingContext.systemInformation.screenDensity
)
val drawableCopier = DrawableCopier { _, _ -> cloneCheckableDrawable(view, it) }
mappingContext.imageWireframeHelper.createImageWireframe(
view = view,
imagePrivacy = mapInputPrivacyToImagePrivacy(mappingContext.textAndInputPrivacy),
Expand All @@ -117,6 +119,7 @@ internal abstract class CheckableTextViewMapper<T>(
y = checkBoxBounds.y,
width = it.intrinsicWidth,
height = it.intrinsicHeight,
drawableCopier = drawableCopier,
drawable = it,
shapeStyle = null,
border = null,
Expand All @@ -127,6 +130,8 @@ internal abstract class CheckableTextViewMapper<T>(
}
}

abstract fun cloneCheckableDrawable(view: T, drawable: Drawable): Drawable?

@UiThread
abstract fun getCheckableDrawable(view: T): Drawable?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ internal open class CheckedTextViewMapper(

return (view.checkMarkDrawable?.constantState as? DrawableContainer.DrawableContainerState)?.getChild(
checkableDrawableIndex
)?.constantState?.newDrawable(view.resources)?.apply {
)
}

override fun cloneCheckableDrawable(view: CheckedTextView, drawable: Drawable): Drawable? {
return drawable.constantState?.newDrawable(view.resources)?.apply {
// Set state to make the drawable have correct tint according to the state.
setState(view.drawableState)
// Set tint list to drawable if the button has declared `checkMarkTint` attribute.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

package com.datadog.android.sessionreplay.internal.recorder.mapper

import android.content.res.Resources
import android.graphics.drawable.Drawable
import androidx.annotation.UiThread
import androidx.appcompat.widget.SwitchCompat
import com.datadog.android.api.InternalLogger
Expand Down Expand Up @@ -75,14 +73,12 @@ internal open class SwitchCompatMapper(
)
return trackBounds?.let {
val trackDrawable = view.trackDrawable
val drawableCopier = object : DrawableCopier {
override fun copy(originalDrawable: Drawable, resources: Resources): Drawable? {
return originalDrawable.constantState?.newDrawable(view.resources)?.apply {
setState(view.trackDrawable.state)
bounds = view.trackDrawable.bounds
view.trackTintList?.let {
setTintList(it)
}
val drawableCopier = DrawableCopier { originalDrawable, resources ->
originalDrawable.constantState?.newDrawable(resources)?.apply {
setState(view.trackDrawable.state)
bounds = view.trackDrawable.bounds
view.trackTintList?.let { tintList ->
setTintList(tintList)
}
}
}
Expand Down Expand Up @@ -118,17 +114,16 @@ internal open class SwitchCompatMapper(
)

val thumbDrawable = view.thumbDrawable
val drawableCopier = object : DrawableCopier {
override fun copy(originalDrawable: Drawable, resources: Resources): Drawable? {
return originalDrawable.constantState?.newDrawable(view.resources)?.apply {
val drawableCopier =
DrawableCopier { originalDrawable, resources ->
originalDrawable.constantState?.newDrawable(resources)?.apply {
setState(view.thumbDrawable.state)
bounds = view.thumbDrawable.bounds
view.thumbTintList?.let {
setTintList(it)
}
}
}
}
return thumbBounds?.let {
mappingContext.imageWireframeHelper.createImageWireframe(
view = view,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import com.datadog.android.lint.InternalApi
* Interface of copying drawable to a new one.
*/
@InternalApi
interface DrawableCopier {
fun interface DrawableCopier {

/**
* Called to copy the drawable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,6 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
@Mock
lateinit var mockConstantState: DrawableContainer.DrawableContainerState

@Mock
lateinit var mockCheckedConstantState: DrawableContainer.DrawableContainerState

@IntForgery(min = 0, max = 0xffffff)
var fakeCurrentTextColor: Int = 0

Expand All @@ -108,35 +105,27 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
@Mock
lateinit var mockNotCheckedDrawable: Drawable

@Mock
lateinit var mockClonedDrawable: Drawable

@Mock
lateinit var mockDrawableCopier: DrawableCopier

@IntForgery
var mockCloneDrawableIntrinsicHeight: Int = 0
var fakeDrawableIntrinsicHeight: Int = 0

@IntForgery
var mockCloneDrawableIntrinsicWidth: Int = 0
var fakeDrawableIntrinsicWidth: Int = 0

@Forgery
lateinit var fakeImagePrivacy: ImagePrivacy

@BeforeEach
fun `set up`() {
mockClonedDrawable = mock {
whenever(it.intrinsicHeight) doReturn mockCloneDrawableIntrinsicHeight
whenever(it.intrinsicWidth) doReturn mockCloneDrawableIntrinsicWidth
}
mockCheckedConstantState = mock {
whenever(it.newDrawable(mockResources)) doReturn mockClonedDrawable
}
mockCheckedDrawable = mock {
whenever(it.constantState) doReturn mockCheckedConstantState
whenever(it.intrinsicHeight) doReturn fakeDrawableIntrinsicHeight
whenever(it.intrinsicWidth) doReturn fakeDrawableIntrinsicWidth
}
mockNotCheckedDrawable = mock {
whenever(it.constantState) doReturn mockCheckedConstantState
whenever(it.intrinsicHeight) doReturn fakeDrawableIntrinsicHeight
whenever(it.intrinsicWidth) doReturn fakeDrawableIntrinsicWidth
}
mockConstantState = mock {
whenever(it.getChild(CHECK_BOX_CHECKED_DRAWABLE_INDEX)) doReturn mockCheckedDrawable
Expand Down Expand Up @@ -226,8 +215,8 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
currentWireframeIndex = anyInt(),
x = eq(expectedX),
y = eq(expectedY),
width = eq(mockCloneDrawableIntrinsicWidth),
height = eq(mockCloneDrawableIntrinsicHeight),
width = eq(fakeDrawableIntrinsicWidth),
height = eq(fakeDrawableIntrinsicHeight),
usePIIPlaceholder = anyBoolean(),
drawable = any(),
drawableCopier = any(),
Expand Down Expand Up @@ -270,10 +259,10 @@ internal abstract class BaseCheckableTextViewMapperTest<T> :
currentWireframeIndex = anyInt(),
x = eq(expectedX),
y = eq(expectedY),
width = eq(mockCloneDrawableIntrinsicWidth),
height = eq(mockCloneDrawableIntrinsicHeight),
width = eq(fakeDrawableIntrinsicWidth),
height = eq(fakeDrawableIntrinsicHeight),
usePIIPlaceholder = anyBoolean(),
drawable = eq(mockClonedDrawable),
drawable = eq(mockNotCheckedDrawable),
drawableCopier = any(),
asyncJobStatusCallback = eq(mockAsyncJobStatusCallback),
clipping = eq(MobileSegment.WireframeClip()),
Expand Down