RUM-6195: Improve background color on unchecked state

DataDog · Dec 17, 2024 · 2ef05ac · 2ef05ac
1 parent 661e57a
commit 2ef05ac
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 65 deletions.
diff --git a/...g/android/sessionreplay/compose/internal/mappers/semantics/CheckboxSemanticsNodeMapper.kt b/...g/android/sessionreplay/compose/internal/mappers/semantics/CheckboxSemanticsNodeMapper.kt
@@ -38,20 +38,20 @@ internal class CheckboxSemanticsNodeMapper(
     ): SemanticsWireframe {
         val globalBounds = resolveBounds(semanticsNode)
 
-        val checkableWireframes = if (parentContext.textAndInputPrivacy != TextAndInputPrivacy.MASK_SENSITIVE_INPUTS) {
+        val checkableWireframes = if (isCheckboxMasked(parentContext)) {
             listOf(
                 resolveMaskedCheckable(
                     semanticsNode = semanticsNode,
                     globalBounds = globalBounds
                 )
             )
         } else {
-            // Resolves checkable view regardless the state
-            resolveCheckable(
-                semanticsNode = semanticsNode,
+            createCheckboxWireframe(
                 parentContext = parentContext,
                 asyncJobStatusCallback = asyncJobStatusCallback,
-                globalBounds = globalBounds
+                semanticsNode = semanticsNode,
+                globalBounds = globalBounds,
+                currentIndex = 0
             )
         }
 
@@ -61,6 +61,9 @@ internal class CheckboxSemanticsNodeMapper(
         )
     }
 
+    private fun isCheckboxMasked(parentContext: UiContext): Boolean =
+        parentContext.textAndInputPrivacy != TextAndInputPrivacy.MASK_SENSITIVE_INPUTS
+
     private fun resolveMaskedCheckable(
         semanticsNode: SemanticsNode,
         globalBounds: GlobalBounds
@@ -75,72 +78,39 @@ internal class CheckboxSemanticsNodeMapper(
         )
     }
 
-    private fun resolveCheckable(
-        semanticsNode: SemanticsNode,
-        parentContext: UiContext,
-        asyncJobStatusCallback: AsyncJobStatusCallback,
-        globalBounds: GlobalBounds
-    ): List<MobileSegment.Wireframe> {
-        return if (isCheckboxChecked(semanticsNode)) {
-            createCheckedState(
-                parentContext = parentContext,
-                asyncJobStatusCallback = asyncJobStatusCallback,
-                semanticsNode = semanticsNode,
-                globalBounds = globalBounds,
-                currentIndex = 0
-            )
-        } else {
-            val borderColor =
-                semanticsUtils.resolveBorderColor(semanticsNode)
-                    ?.let { rawColor ->
-                        convertColor(rawColor)
-                    } ?: DEFAULT_COLOR_BLACK
-
-            listOf(
-                createUncheckedState(
-                    semanticsNode = semanticsNode,
-                    globalBounds = globalBounds,
-                    backgroundColor = DEFAULT_COLOR_WHITE,
-                    borderColor = borderColor,
-                    currentIndex = 0
-                )
-            )
-        }
-    }
-
-    private fun createCheckedState(
+    private fun createCheckboxWireframe(
         parentContext: UiContext,
         asyncJobStatusCallback: AsyncJobStatusCallback,
         semanticsNode: SemanticsNode,
         globalBounds: GlobalBounds,
         currentIndex: Int
     ): List<MobileSegment.Wireframe> {
+        val borderColor = resolveBorderColor(semanticsNode)
         val rawFillColor = semanticsUtils.resolveCheckboxFillColor(semanticsNode)
         val rawCheckmarkColor = semanticsUtils.resolveCheckmarkColor(semanticsNode)
-
-        val fillColorRgba = rawFillColor?.let {
-            convertColor(it)
-        } ?: DEFAULT_COLOR_WHITE
-        val checkmarkColorRgba = rawCheckmarkColor?.let {
-            convertColor(it)
-        } ?: getFallbackCheckmarkColor(DEFAULT_COLOR_WHITE)
-
+        val fillColorRgba = rawFillColor?.let { convertColor(it) } ?: DEFAULT_COLOR_WHITE
+        val checkmarkColorRgba = rawCheckmarkColor?.let { convertColor(it) }
+            ?: getFallbackCheckmarkColor(DEFAULT_COLOR_WHITE)
         val parsedFillColor = colorUtils.parseColorSafe(fillColorRgba)
-        val parsedCheckmarkColor = colorUtils.parseColorSafe(checkmarkColorRgba)
+        val isChecked = isCheckboxChecked(semanticsNode)
+        val checkmarkColor = resolveCheckmarkColor(isChecked, checkmarkColorRgba, parsedFillColor)
+
         val wireframes = mutableListOf<MobileSegment.Wireframe>()
 
-        if (parsedFillColor != null && parsedCheckmarkColor != null) {
-            val androidPath = semanticsUtils
-                .resolveCheckPath(semanticsNode)?.let { checkPath ->
-                    pathUtils.asAndroidPathSafe(checkPath)
-                }
+        if (parsedFillColor != null && checkmarkColor != null) {
+            val composePath = semanticsUtils
+                .resolveCheckPath(semanticsNode)
+
+            val androidPath = composePath?.let { checkPath ->
+                pathUtils.asAndroidPathSafe(checkPath)
+            }
 
             if (androidPath != null) {
                 parentContext.imageWireframeHelper.createImageWireframeByPath(
                     id = resolveId(semanticsNode, currentIndex),
                     globalBounds = globalBounds,
                     path = androidPath,
-                    strokeColor = parsedCheckmarkColor,
+                    strokeColor = checkmarkColor,
                     strokeWidth = STROKE_WIDTH_DP.toInt(),
                     targetWidth = CHECKBOX_SIZE_DP,
                     targetHeight = CHECKBOX_SIZE_DP,
@@ -155,7 +125,7 @@ internal class CheckboxSemanticsNodeMapper(
                         cornerRadius = CHECKBOX_CORNER_RADIUS
                     ),
                     border = MobileSegment.ShapeBorder(
-                        color = fillColorRgba,
+                        color = borderColor,
                         width = BOX_BORDER_WIDTH_DP
                     ),
                     customResourceIdCacheKey = null
@@ -173,25 +143,35 @@ internal class CheckboxSemanticsNodeMapper(
         return createManualCheckedWireframe(
             semanticsNode = semanticsNode,
             globalBounds = globalBounds,
-            backgroundColor = fillColorRgba
+            backgroundColor = fillColorRgba,
+            borderColor = borderColor
         )
     }
 
+    private fun resolveCheckmarkColor(isChecked: Boolean, checkmarkColorRgba: String, fillColor: Int?): Int? =
+        if (isChecked) {
+            colorUtils.parseColorSafe(checkmarkColorRgba)
+        } else {
+            fillColor
+        }
+
+    private fun resolveBorderColor(semanticsNode: SemanticsNode): String {
+        return semanticsUtils.resolveBorderColor(semanticsNode)
+            ?.let { rawColor ->
+                convertColor(rawColor)
+            } ?: DEFAULT_COLOR_BLACK
+    }
+
     private fun createManualCheckedWireframe(
         semanticsNode: SemanticsNode,
         globalBounds: GlobalBounds,
-        backgroundColor: String
+        backgroundColor: String,
+        borderColor: String
     ): List<MobileSegment.Wireframe> {
         val strokeColor = getFallbackCheckmarkColor(backgroundColor)
 
         val wireframesList = mutableListOf<MobileSegment.Wireframe>()
 
-        val borderColor =
-            semanticsUtils.resolveBorderColor(semanticsNode)
-                ?.let { rawColor ->
-                    convertColor(rawColor)
-                } ?: DEFAULT_COLOR_BLACK
-
         val background = createUncheckedState(
             semanticsNode = semanticsNode,
             globalBounds = globalBounds,

diff --git a/...droid/sessionreplay/compose/internal/mappers/semantics/CheckboxSemanticsNodeMapperTest.kt b/...droid/sessionreplay/compose/internal/mappers/semantics/CheckboxSemanticsNodeMapperTest.kt
@@ -407,7 +407,7 @@ internal class CheckboxSemanticsNodeMapperTest : AbstractSemanticsNodeMapperTest
         )
 
         val expectedBorder = MobileSegment.ShapeBorder(
-            color = fakeFillColorHexString,
+            color = fakeBorderColorHexString,
             width = BOX_BORDER_WIDTH_DP
         )
 

diff --git a/...g/android/sessionreplay/compose/internal/mappers/semantics/RootSemanticsNodeMapperTest.kt b/...g/android/sessionreplay/compose/internal/mappers/semantics/RootSemanticsNodeMapperTest.kt
@@ -71,6 +71,9 @@ class RootSemanticsNodeMapperTest {
     @Mock
     private lateinit var mockImageSemanticsNodeMapper: ImageSemanticsNodeMapper
 
+    @Mock
+    private lateinit var mockCheckboxSemanticsNodeMapper: CheckboxSemanticsNodeMapper
+
     @Mock
     private lateinit var mockComposeHiddenMapper: ComposeHiddenMapper
 
@@ -91,7 +94,8 @@ class RootSemanticsNodeMapperTest {
                 Role.RadioButton to mockRadioButtonSemanticsNodeMapper,
                 Role.Tab to mockTabSemanticsNodeMapper,
                 Role.Button to mockButtonSemanticsNodeMapper,
-                Role.Image to mockImageSemanticsNodeMapper
+                Role.Image to mockImageSemanticsNodeMapper,
+                Role.Checkbox to mockCheckboxSemanticsNodeMapper
             ),
             textSemanticsNodeMapper = mockTextSemanticsNodeMapper,
             containerSemanticsNodeMapper = mockContainerSemanticsNodeMapper,
@@ -204,6 +208,27 @@ class RootSemanticsNodeMapperTest {
         )
     }
 
+    @Test
+    fun `M use CheckboxSemanticsNodeMapper W map { role is Checkbox }`() {
+        // Given
+        val mockSemanticsNode = mockSemanticsNode(Role.Checkbox)
+
+        // When
+        testedRootSemanticsNodeMapper.createComposeWireframes(
+            mockSemanticsNode,
+            fakeMappingContext.systemInformation.screenDensity,
+            fakeMappingContext,
+            mockAsyncJobStatusCallback
+        )
+
+        // Then
+        verify(mockCheckboxSemanticsNodeMapper).map(
+            eq(mockSemanticsNode),
+            any(),
+            eq(mockAsyncJobStatusCallback)
+        )
+    }
+
     @Test
     fun `M use ComposeHideMapper W node is hidden`(forge: Forge) {
         // Given