issue 12: applied PR feedback

pseudoankit · Nov 30, 2023 · 3b8c79d · 3b8c79d
1 parent 5eeb04b
commit 3b8c79d
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 54 deletions.
diff --git a/coachmark/src/main/java/com/pseudoankit/coachmark/overlay/DimOverlayEffect.kt b/coachmark/src/main/java/com/pseudoankit/coachmark/overlay/DimOverlayEffect.kt
@@ -7,14 +7,19 @@ import androidx.compose.ui.draw.drawBehind
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.graphicsLayer
 import androidx.compose.ui.platform.LocalDensity
+import androidx.compose.ui.unit.Dp
 import com.pseudoankit.coachmark.model.TooltipHolder
 import com.pseudoankit.coachmark.scope.CoachMarkScope
 import com.pseudoankit.coachmark.util.CoachMarkDefaults
 import com.pseudoankit.coachmark.util.highlightActualView
 
+/**
+ * @param paddingForTooltip min distance between tooltip and left/right side of screen/overlay
+ */
 public class DimOverlayEffect(
     private val color: Color = Color.Black.copy(alpha = .75f),
-    override val overlayAnimationSpec: AnimationSpec<Float> = CoachMarkDefaults.Overlay.animationSpec
+    override val overlayAnimationSpec: AnimationSpec<Float> = CoachMarkDefaults.Overlay.animationSpec,
+    private val paddingForTooltip: Dp = CoachMarkDefaults.ToolTip.paddingForTooltip,
 ) : UnifyOverlayEffect {
 
     @Composable
@@ -27,10 +32,9 @@ public class DimOverlayEffect(
         val density = LocalDensity.current
 
         OverlayLayout(
-            content,
-            currentTooltip?.item,
-            previousTooltip?.item,
-            modifier
+            configCurrent = currentTooltip?.item,
+            configPrevious = previousTooltip?.item,
+            modifier = modifier
                 .graphicsLayer(alpha = .99f)
                 .drawBehind {
                     drawRect(color)
@@ -41,6 +45,8 @@ public class DimOverlayEffect(
                         highlightActualView(tooltip, density, previousTooltip.alpha)
                     }
                 },
+            content = content,
+            paddingForTooltip = paddingForTooltip,
         )
     }
 

diff --git a/coachmark/src/main/java/com/pseudoankit/coachmark/overlay/OverlayLayout.kt b/coachmark/src/main/java/com/pseudoankit/coachmark/overlay/OverlayLayout.kt
@@ -9,12 +9,15 @@ import androidx.compose.ui.layout.Placeable
 import androidx.compose.ui.layout.layoutId
 import androidx.compose.ui.unit.Constraints
 import androidx.compose.ui.unit.Dp
-import androidx.compose.ui.unit.dp
 import com.pseudoankit.coachmark.model.ToolTipPlacement
 import com.pseudoankit.coachmark.model.TooltipConfig
+import com.pseudoankit.coachmark.util.CoachMarkDefaults
 
 /** Containing composable must use these values for layoutId on current and previous tooltip. */
-public enum class OverlayChildLayoutId { CURRENT, PREVIOUS }
+public object TooltipId {
+    public const val current: Int = 1
+    public const val previous: Int = 2
+}
 
 /** Proposed minimum useful tooltip width; this can be adjusted if needed */
 private const val TOOLTIP_MAX_WIDTH_OVERRIDE_PX = 30
@@ -24,26 +27,38 @@ private const val TOOLTIP_MAX_WIDTH_OVERRIDE_PX = 30
  *
  * @param configCurrent of coach mark highlight; null suits case where there's no "current" tooltip in content
  * @param configPrevious of coach mark highlight; null suits case where there's no "previous" tooltip in content
- * @param gapTooltipScreen min distance between tooltip and edge of screen
+ * @param paddingForTooltip min distance between tooltip and left/right side of screen/overlay
  */
 @Composable
 public fun OverlayLayout(
-    content: @Composable @UiComposable () -> Unit,
     configCurrent: TooltipConfig?,
     configPrevious: TooltipConfig?,
     modifier: Modifier = Modifier,
-    gapTooltipScreen: Dp = 8.dp,
+    paddingForTooltip: Dp = CoachMarkDefaults.ToolTip.paddingForTooltip,
+    content: @Composable @UiComposable () -> Unit,
 ) {
     Layout(content, modifier) { measurables, constraints ->
 
         // child count < 2 occurs on first and last coach mark
         require(measurables.size <= 2) { "OverlayLayout cannot have more than two children" }
 
-        val gapTooltipScreenPx = gapTooltipScreen.roundToPx()
+        val gapTooltipScreenPx = paddingForTooltip.roundToPx()
 
         // measure children
-        val placeableCurrent = measure(configCurrent, OverlayChildLayoutId.CURRENT, measurables, constraints, gapTooltipScreenPx)
-        val placeablePrevious = measure(configPrevious, OverlayChildLayoutId.PREVIOUS, measurables, constraints, gapTooltipScreenPx)
+        val placeableCurrent = measure(
+            tooltipConfig = configCurrent,
+            layoutId = TooltipId.current,
+            measurables = measurables,
+            constraintsParent = constraints,
+            gapTooltipScreenPx = gapTooltipScreenPx
+        )
+        val placeablePrevious = measure(
+            tooltipConfig = configPrevious,
+            layoutId = TooltipId.previous,
+            measurables = measurables,
+            constraintsParent = constraints,
+            gapTooltipScreenPx = gapTooltipScreenPx
+        )
 
         // place children
         layout(constraints.maxWidth, constraints.maxHeight) {
@@ -55,11 +70,12 @@ public fun OverlayLayout(
 
 
 /**
+ * @param layoutId use consts from TooltipId
  * @return null if tooltipConfig is null
  */
 private fun measure(
     tooltipConfig: TooltipConfig?,
-    layoutId: OverlayChildLayoutId,
+    layoutId: Int,
     measurables: List<Measurable>,
     constraintsParent: Constraints,
     gapTooltipScreenPx: Int,
@@ -69,7 +85,7 @@ private fun measure(
     // constrain max width to prevent tooltip running off screen
     var maxWidth = when (tooltipConfig.toolTipPlacement) {
         ToolTipPlacement.Start -> {
-            constraintsParent.maxWidth - gapTooltipScreenPx - (constraintsParent.maxWidth - tooltipConfig.layout.startX.toInt())
+            tooltipConfig.layout.startX.toInt() - gapTooltipScreenPx // left edge of highlight, minus overlay padding
         }
         ToolTipPlacement.End -> {
             constraintsParent.maxWidth - gapTooltipScreenPx - tooltipConfig.layout.endX.toInt()
@@ -106,42 +122,40 @@ private fun measure(
 
 /**
  * Centralizes null checks and switching on toolTipPlacement value.
- * @param placeableOrNull no-op if null
- * @param configOrNull no-op if null
+ * @param placeable no-op if null
+ * @param config no-op if null
  */
-private fun Placeable.PlacementScope.place(placeableOrNull: Placeable?, configOrNull: TooltipConfig?) {
-    placeableOrNull?.let { placeable ->
-        configOrNull?.let { config ->
-            val layout = config.layout
-            var x = 0
-            var y = 0
-
-            // result positive when highlight is larger, negative when tooltip is larger
-            fun calculateCenteringOffset(independentHeight: Int, dependentHeight: Int): Int = (independentHeight - dependentHeight) shr 1
-
-            fun centerVertically() = (layout.startY + calculateCenteringOffset(layout.height, placeable.height)).toInt()
-            fun centerHorizontally() = (layout.startX + calculateCenteringOffset(layout.width, placeable.width)).toInt()
-
-            when (config.toolTipPlacement) {
-                ToolTipPlacement.Start -> {
-                    x = layout.startX.toInt() - placeable.width
-                    y = centerVertically()
-                }
-                ToolTipPlacement.End -> {
-                    x = layout.endX.toInt()
-                    y = centerVertically()
-                }
-                ToolTipPlacement.Top -> {
-                    x = centerHorizontally()
-                    y = layout.startY.toInt() - placeable.height
-                }
-                ToolTipPlacement.Bottom -> {
-                    x = centerHorizontally()
-                    y = layout.endY.toInt()
-                }
+private fun Placeable.PlacementScope.place(placeable: Placeable?, config: TooltipConfig?) {
+    if (placeable != null && config != null) {
+        val layout = config.layout
+        var x = 0
+        var y = 0
+
+        // result positive when highlight is larger, negative when tooltip is larger
+        fun calculateCenteringOffset(independentHeight: Int, dependentHeight: Int): Int = (independentHeight - dependentHeight) shr 1
+
+        fun centerVertically() = (layout.startY + calculateCenteringOffset(layout.height, placeable.height)).toInt()
+        fun centerHorizontally() = (layout.startX + calculateCenteringOffset(layout.width, placeable.width)).toInt()
+
+        when (config.toolTipPlacement) {
+            ToolTipPlacement.Start -> {
+                x = layout.startX.toInt() - placeable.width
+                y = centerVertically()
+            }
+            ToolTipPlacement.End -> {
+                x = layout.endX.toInt()
+                y = centerVertically()
+            }
+            ToolTipPlacement.Top -> {
+                x = centerHorizontally()
+                y = layout.startY.toInt() - placeable.height
+            }
+            ToolTipPlacement.Bottom -> {
+                x = centerHorizontally()
+                y = layout.endY.toInt()
             }
-
-            placeable.placeRelative(x, y)
         }
+
+        placeable.placeRelative(x, y)
     }
 }
diff --git a/coachmark/src/main/java/com/pseudoankit/coachmark/ui/CoachMarkImpl.kt b/coachmark/src/main/java/com/pseudoankit/coachmark/ui/CoachMarkImpl.kt
@@ -7,7 +7,7 @@ import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.alpha
 import androidx.compose.ui.layout.layoutId
-import com.pseudoankit.coachmark.overlay.OverlayChildLayoutId
+import com.pseudoankit.coachmark.overlay.TooltipId
 import com.pseudoankit.coachmark.overlay.UnifyOverlayEffect
 import com.pseudoankit.coachmark.scope.CoachMarkScope
 import com.pseudoankit.coachmark.scope.CoachMarkScopeImpl
@@ -58,14 +58,14 @@ internal fun CoachMarkImpl(
             previousTooltip = previousTooltip
         ) {
             Tooltip(
-                currentTooltip,
-                modifier = Modifier.layoutId(OverlayChildLayoutId.CURRENT),
+                tooltipHolder = currentTooltip,
+                modifier = Modifier.layoutId(TooltipId.current),
             ) {
                 coachMarkScope.tooltip(it)
             }
             Tooltip(
-                previousTooltip,
-                modifier = Modifier.layoutId(OverlayChildLayoutId.PREVIOUS),
+                tooltipHolder = previousTooltip,
+                modifier = Modifier.layoutId(TooltipId.previous),
             ) {
                 coachMarkScope.tooltip(it)
             }

diff --git a/coachmark/src/main/java/com/pseudoankit/coachmark/util/CoachMarkDefaults.kt b/coachmark/src/main/java/com/pseudoankit/coachmark/util/CoachMarkDefaults.kt
@@ -31,6 +31,7 @@ public object CoachMarkDefaults {
 
     public object ToolTip {
         public val animationSpec: AnimationSpec<Float> = tween(ANIMATION_DURATION)
+        public val paddingForTooltip: Dp = 8.dp
     }
 
     public object Overlay {