Skip to content

Commit 453078c

Browse files
committed
address review feedback
1 parent 7ce0984 commit 453078c

3 files changed

Lines changed: 70 additions & 6 deletions

File tree

posthog-android-surveys-compose/src/main/java/com/posthog/android/surveys/compose/internal/ui/EmojiRating.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,9 @@ internal fun EmojiRating(
6060
) {
6161
val appearance = localAppearance()
6262
val emojis = emojisForScale(question.scaleUpperBound - question.scaleLowerBound + 1)
63-
// Record the rating against the question's real scale bounds (matching NumberRating and
64-
// iOS), not a synthetic 1..size range. Emoji scales are 1-based on the server, so for
65-
// thumbs this is 1 = 👍, 2 = 👎 — the same values iOS records and the dashboard expects.
66-
val values = (question.scaleLowerBound..question.scaleUpperBound).toList()
63+
// Drive the layout off the emoji list (always 2/3/5 faces) rather than the scale range, so an
64+
// unexpected scale can never index past the available faces. The recorded rating uses the
65+
// question's 1-based scale bounds: lowerBound + index (e.g. thumbs → 1 = 👍, 2 = 👎).
6766
// Thumbs up/down (a 2-point emoji scale) hides the bound labels — thumbs
6867
// don't need "lower/upper" captions.
6968
val showBoundLabels = emojis.size > 2
@@ -78,7 +77,8 @@ internal fun EmojiRating(
7877
horizontalArrangement = arrangement,
7978
verticalAlignment = Alignment.CenterVertically,
8079
) {
81-
values.forEachIndexed { index, value ->
80+
emojis.forEachIndexed { index, emoji ->
81+
val value = question.scaleLowerBound + index
8282
val isSelected = selectedValue == value
8383
// Non-selected emojis use a muted version of the input text color (a gray on a
8484
// light background); selected ones use the active rating color.
@@ -99,7 +99,7 @@ internal fun EmojiRating(
9999
contentAlignment = Alignment.Center,
100100
) {
101101
Canvas(modifier = Modifier.size(40.dp)) {
102-
drawPath(emojis[index].buildPath(size.width, size.height), color = tint)
102+
drawPath(emoji.buildPath(size.width, size.height), color = tint)
103103
}
104104
}
105105
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package com.posthog.android.surveys.compose.internal.theme
2+
3+
import androidx.compose.ui.graphics.Color
4+
import androidx.compose.ui.graphics.toArgb
5+
import org.junit.Assert.assertEquals
6+
import org.junit.Test
7+
8+
internal class SurveyColorTest {
9+
private fun argb(input: String?): Int = parseSurveyColor(input).toArgb()
10+
11+
@Test
12+
fun `named colors resolve case-insensitively`() {
13+
assertEquals(0xFFFF0000.toInt(), argb("red"))
14+
assertEquals(0xFFFF0000.toInt(), argb("RED"))
15+
assertEquals(0xFFFFFFFF.toInt(), argb("white"))
16+
assertEquals(0xFF000000.toInt(), argb("black"))
17+
}
18+
19+
@Test
20+
fun `transparent aliases resolve to fully transparent`() {
21+
assertEquals(0x00000000, argb("transparent"))
22+
assertEquals(0x00000000, argb("clear"))
23+
}
24+
25+
@Test
26+
fun `hex with and without leading hash resolve the same`() {
27+
assertEquals(0xFFFF0000.toInt(), argb("#FF0000"))
28+
assertEquals(0xFFFF0000.toInt(), argb("FF0000"))
29+
assertEquals(0xFF0000FF.toInt(), argb("#0000FF"))
30+
}
31+
32+
@Test
33+
fun `short hex forms are expanded`() {
34+
// #RGB -> #RRGGBB (opaque)
35+
assertEquals(0xFFFF0000.toInt(), argb("#F00"))
36+
// #RGBA -> #RRGGBBAA
37+
assertEquals(0x88FF0000.toInt(), argb("#F008"))
38+
}
39+
40+
@Test
41+
fun `eight-digit hex carries its alpha channel`() {
42+
assertEquals(0xFFFF0000.toInt(), argb("#FF0000FF"))
43+
assertEquals(0x80FF0000.toInt(), argb("#FF000080"))
44+
}
45+
46+
@Test
47+
fun `null blank and unrecognized input fall back to transparent`() {
48+
assertEquals(0x00000000, argb(null))
49+
assertEquals(0x00000000, argb(""))
50+
assertEquals(0x00000000, argb(" "))
51+
assertEquals(0x00000000, argb("#GGG"))
52+
assertEquals(0x00000000, argb("notacolor"))
53+
}
54+
55+
@Test
56+
fun `contrasting text color follows luminance`() {
57+
assertEquals(Color.Black, Color.White.contrastingTextColor())
58+
assertEquals(Color.White, Color.Black.contrastingTextColor())
59+
}
60+
}

posthog-android/src/main/java/com/posthog/android/surveys/PostHogSurveysIntegration.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ public class PostHogSurveysIntegration(
113113
isStarted = false
114114
}
115115

116+
// Tear down any survey UI still on screen so its dialog window doesn't outlive the
117+
// integration; clearActiveSurvey() only resets our bookkeeping, not the delegate's UI.
118+
cleanupSurveys()
119+
116120
clearActiveSurvey()
117121

118122
this.postHog = null

0 commit comments

Comments
 (0)