Merge pull request #66 from pushpalroy/proy/fix/performance

pushpalroy · web-flow · commit 07f67cc9ea09 · 2026-04-23T01:03:42.000+05:30
Fix correctness, performance, and polish issues in timeline rendering
diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml
@@ -44,6 +44,9 @@ jobs:
           # Update README installation snippet
           sed -i '' "s/implementation(\"io.github.pushpalroy:jetlime:.*\")/implementation(\"io.github.pushpalroy:jetlime:$VERSION\")/g" README.md
 
+          # Update commented-out maven testing snippet in sample app
+          sed -i '' "s|// implementation(\"io.github.pushpalroy:jetlime:.*\")|// implementation(\"io.github.pushpalroy:jetlime:$VERSION\")|g" sample/composeApp/build.gradle.kts
+
       - name: Publish to Maven Central
         shell: bash
         env:
@@ -55,6 +58,11 @@ jobs:
         run: |
           ./gradlew publishAndReleaseToMavenCentral --no-configuration-cache
 
+      - name: Generate Dokka API docs
+        shell: bash
+        run: |
+          ./scripts/run_dokka.sh
+
       - name: Commit version updates
         shell: bash
         run: |
@@ -70,3 +78,14 @@ jobs:
           VERSION="${{ github.event.inputs.version }}"
           git tag -a "$VERSION" -m "Release $VERSION"
           git push origin "$VERSION"
+
+      - name: Create GitHub release
+        shell: bash
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          VERSION="${{ github.event.inputs.version }}"
+          gh release create "$VERSION" \
+            --title "v$VERSION" \
+            --target main \
+            --generate-notes
diff --git a/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/EventPosition.kt b/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/EventPosition.kt
@@ -38,7 +38,7 @@ class EventPosition internal constructor(val name: String) {
   companion object {
 
     /** Represents the start position in a sequence. */
-    private val START = EventPosition("Start")
+    internal val START = EventPosition("Start")
 
     /** Represents the middle position in a sequence. */
     private val MIDDLE = EventPosition("Middle")
@@ -78,7 +78,7 @@ class EventPosition internal constructor(val name: String) {
 
   /** Helper to check if current position is not the start. */
   @Stable
-  fun isNotStart(): Boolean = name != "Start"
+  fun isNotStart(): Boolean = this != START
 
   /**
    * Checks if this instance is equal to another object. Two instances of [EventPosition] are
diff --git a/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeEvent.kt b/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeEvent.kt
@@ -117,7 +117,7 @@ internal fun VerticalEvent(
   modifier: Modifier = Modifier,
   content: @Composable () -> Unit,
 ) {
-  val verticalAlignment = remember { jetLimeStyle.lineVerticalAlignment }
+  val verticalAlignment = jetLimeStyle.lineVerticalAlignment
   val radiusAnimFactor by calculateRadiusAnimFactor(style)
   Box(
     modifier = modifier
@@ -148,17 +148,6 @@ internal fun VerticalEvent(
         val radius = style.pointRadius.toPx() * radiusAnimFactor
         val strokeWidth = style.pointStrokeWidth.toPx()
 
-        // Line
-        // Upward connector only for CENTER placement (connects from top of item to point center)
-        if (style.pointPlacement == PointPlacement.CENTER && style.position.isNotStart()) {
-          drawLine(
-            brush = jetLimeStyle.lineBrush,
-            start = Offset(x = xOffset, y = 0f),
-            end = Offset(x = xOffset, y = yOffset),
-            strokeWidth = jetLimeStyle.lineThickness.toPx(),
-            pathEffect = jetLimeStyle.pathEffect,
-          )
-        }
         // Line logic for CENTER placement: keep continuity through centers
         if (style.pointPlacement == PointPlacement.CENTER) {
           // Upward segment (skip for first item)
@@ -330,7 +319,7 @@ internal fun HorizontalEvent(
   modifier: Modifier = Modifier,
   content: @Composable () -> Unit,
 ) {
-  val horizontalAlignment = remember { jetLimeStyle.lineHorizontalAlignment }
+  val horizontalAlignment = jetLimeStyle.lineHorizontalAlignment
   val radiusAnimFactor by calculateRadiusAnimFactor(style)
   val layoutDirection = LocalLayoutDirection.current
   val isRtl = layoutDirection == LayoutDirection.Rtl
@@ -535,15 +524,12 @@ private fun PlaceHorizontalEventContent(
  */
 @Composable
 internal fun calculateRadiusAnimFactor(style: JetLimeEventStyle): State<Float> {
+  val animation = style.pointAnimation ?: return remember { mutableFloatStateOf(1.0f) }
   val infiniteTransition = rememberInfiniteTransition(label = "RadiusInfiniteTransition")
-  return if (style.pointAnimation != null) {
-    infiniteTransition.animateFloat(
-      initialValue = style.pointAnimation.initialValue,
-      targetValue = style.pointAnimation.targetValue,
-      animationSpec = style.pointAnimation.animationSpec,
-      label = "RadiusFloatAnimation",
-    )
-  } else {
-    remember { mutableFloatStateOf(1.0f) }
-  }
+  return infiniteTransition.animateFloat(
+    initialValue = animation.initialValue,
+    targetValue = animation.targetValue,
+    animationSpec = animation.animationSpec,
+    label = "RadiusFloatAnimation",
+  )
 }
diff --git a/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeExtendedEvent.kt b/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeExtendedEvent.kt
@@ -24,18 +24,16 @@
 */
 package com.pushpal.jetlime
 
-import androidx.compose.foundation.Canvas
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.BoxScope
-import androidx.compose.foundation.layout.BoxWithConstraints
 import androidx.compose.foundation.layout.padding
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.ExperimentalComposeApi
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableFloatStateOf
 import androidx.compose.runtime.remember
-import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
+import androidx.compose.ui.draw.drawBehind
 import androidx.compose.ui.geometry.Offset
 import androidx.compose.ui.geometry.Size
 import androidx.compose.ui.graphics.ColorFilter
@@ -99,113 +97,14 @@ fun JetLimeExtendedEvent(
   val layoutDirection = LocalLayoutDirection.current
   val isRtl = layoutDirection == LayoutDirection.Rtl
 
-  BoxWithConstraints(modifier = modifier) {
-    var logicalTimelineXOffset by remember { mutableFloatStateOf(0f) }
-    val maxAdditionalContentWidth = with(density) { additionalContentMaxWidth.toPx() }
+  val timelineXState = remember { mutableFloatStateOf(0f) }
+  val maxAdditionalContentWidth = with(density) { additionalContentMaxWidth.toPx() }
 
-    Layout(
-      content = {
-        // Box for main content with optional padding at the bottom
-        Box(
-          modifier = Modifier.padding(
-            bottom = if (style.position.isNotEnd()) {
-              jetLimeStyle.itemSpacing
-            } else {
-              0.dp
-            },
-          ),
-        ) {
-          content()
-        }
-        // Additional content passed as a composable lambda
-        additionalContent()
-      },
-    ) { measurables, constraints ->
-      // Ensuring that there is at least one child in the layout
-      require(measurables.isNotEmpty()) {
-        "JetLimeExtendedEvent should have at-least one child for content"
-      }
-      // Thickness of the line drawn for the timeline
-      val timelineThickness = jetLimeStyle.lineThickness.toPx()
-      // Distance between the content/additional content and the timeline
-      val contentDistance = jetLimeStyle.contentDistance.toPx()
-
-      // Extracting the first and potentially second child for layout
-      val contentMeasurable = measurables.first()
-      val additionalContentMeasurable = measurables.getOrNull(1)
-
-      // Measuring the additional content if it exists
-      val additionalContentPlaceable = additionalContentMeasurable?.let { measurable ->
-        // Calculating intrinsic width and adjusting it according to the maximum allowed width
-        val intrinsicWidth = measurable.minIntrinsicWidth(constraints.maxHeight)
-        val adjustedMinWidth = intrinsicWidth.coerceAtMost(maxAdditionalContentWidth.toInt())
-        // Ensure we do not exceed available width
-        val maxWidthForAdditional =
-          maxAdditionalContentWidth.coerceAtMost(constraints.maxWidth.toFloat()).toInt()
-        val newConstraints = constraints.copy(
-          minWidth = adjustedMinWidth.coerceAtMost(maxWidthForAdditional),
-          maxWidth = maxWidthForAdditional,
-        )
-        // Measuring the additional content with the new constraints
-        measurable.measure(newConstraints)
-      }
-
-      // Calculating the logical X offset for the timeline based on the width of the additional content
-      logicalTimelineXOffset =
-        (additionalContentPlaceable?.width?.toFloat() ?: 0f) + contentDistance
-
-      // Calculating the X offset and width available for the main content in logical LTR space
-      val logicalContentXOffset = logicalTimelineXOffset + timelineThickness + contentDistance
-      val contentWidth = constraints.maxWidth - logicalContentXOffset
-
-      // Measuring the main content with the calculated width
-      val contentPlaceable = contentMeasurable.measure(
-        constraints.copy(minWidth = 0, maxWidth = contentWidth.toInt()),
-      )
-
-      // Determining the height of the layout based on the measured content
-      val contentHeight = contentPlaceable.height
-      val layoutHeight = additionalContentPlaceable?.let { additional ->
-        maxOf(contentHeight, additional.height)
-      } ?: contentHeight
-
-      val totalWidth = constraints.maxWidth
-
-      // Placing the measured composables in the layout, mirroring in RTL so that
-      // additional content is always on the logical "left" of the timeline and main
-      // content on the "right", but visually flipped when isRtl is true.
-      layout(totalWidth, layoutHeight) {
-        if (isRtl) {
-          // In RTL, place additional content flush to the right so it stays visible
-          additionalContentPlaceable?.placeRelative(
-            x = totalWidth - additionalContentPlaceable.width,
-            y = 0,
-          )
-        } else {
-          // LTR: original behavior, additional content starts from left
-          additionalContentPlaceable?.placeRelative(x = 0, y = 0)
-        }
-
-        // Place main content on the opposite side of the timeline depending on direction
-        val contentX = if (isRtl) {
-          // In RTL, main content should be left of the timeline, but still within bounds
-          (logicalTimelineXOffset - contentPlaceable.width - jetLimeStyle.contentDistance.toPx())
-            .coerceAtLeast(0f)
-            .toInt()
-        } else {
-          logicalContentXOffset.toInt()
-        }
-        contentPlaceable.placeRelative(x = contentX, y = 0)
-      }
-    }
-
-    // Drawing on canvas for additional graphical elements
-    Canvas(modifier = Modifier.matchParentSize()) {
-      // Use the logical timeline offset directly in both LTR and RTL so that
-      // the line stays aligned with the layout’s coordinate system. RTL
-      // placement is handled by how content is positioned relative to this
-      // logical offset, avoiding overlap with the main content.
-      val timelineXOffset = logicalTimelineXOffset
+  Layout(
+    modifier = modifier.drawBehind {
+      // Line and point are drawn here so a state write from the measure phase only
+      // invalidates the draw phase (not composition) and stays in the same frame.
+      val timelineXOffset = timelineXState.floatValue
 
       val yOffset = when (style.pointPlacement) {
         PointPlacement.START -> style.pointRadius.toPx() * jetLimeStyle.pointStartFactor
@@ -304,6 +203,104 @@ fun JetLimeExtendedEvent(
           style = Stroke(width = strokeWidth),
         )
       }
+    },
+    content = {
+      // Box for main content with optional padding at the bottom
+      Box(
+        modifier = Modifier.padding(
+          bottom = if (style.position.isNotEnd()) {
+            jetLimeStyle.itemSpacing
+          } else {
+            0.dp
+          },
+        ),
+      ) {
+        content()
+      }
+      // Additional content wrapped in a Box to provide the expected BoxScope receiver.
+      Box {
+        additionalContent()
+      }
+    },
+  ) { measurables, constraints ->
+    // Ensuring that there is at least one child in the layout
+    require(measurables.isNotEmpty()) {
+      "JetLimeExtendedEvent should have at-least one child for content"
+    }
+    // Thickness of the line drawn for the timeline
+    val timelineThickness = jetLimeStyle.lineThickness.toPx()
+    // Distance between the content/additional content and the timeline
+    val contentDistance = jetLimeStyle.contentDistance.toPx()
+
+    // Extracting the first and potentially second child for layout
+    val contentMeasurable = measurables.first()
+    val additionalContentMeasurable = measurables.getOrNull(1)
+
+    // Measuring the additional content if it exists
+    val additionalContentPlaceable = additionalContentMeasurable?.let { measurable ->
+      // Calculating intrinsic width and adjusting it according to the maximum allowed width
+      val intrinsicWidth = measurable.minIntrinsicWidth(constraints.maxHeight)
+      val adjustedMinWidth = intrinsicWidth.coerceAtMost(maxAdditionalContentWidth.toInt())
+      // Ensure we do not exceed available width
+      val maxWidthForAdditional =
+        maxAdditionalContentWidth.coerceAtMost(constraints.maxWidth.toFloat()).toInt()
+      val newConstraints = constraints.copy(
+        minWidth = adjustedMinWidth.coerceAtMost(maxWidthForAdditional),
+        maxWidth = maxWidthForAdditional,
+      )
+      // Measuring the additional content with the new constraints
+      measurable.measure(newConstraints)
+    }
+
+    // Calculating the logical X offset for the timeline based on the width of the additional content
+    val logicalTimelineXOffset =
+      (additionalContentPlaceable?.width?.toFloat() ?: 0f) + contentDistance
+    // Publish for the drawBehind modifier. Only the draw phase observes this state,
+    // so the write does not trigger recomposition.
+    timelineXState.floatValue = logicalTimelineXOffset
+
+    // Calculating the X offset and width available for the main content in logical LTR space
+    val logicalContentXOffset = logicalTimelineXOffset + timelineThickness + contentDistance
+    val contentWidth = constraints.maxWidth - logicalContentXOffset
+
+    // Measuring the main content with the calculated width
+    val contentPlaceable = contentMeasurable.measure(
+      constraints.copy(minWidth = 0, maxWidth = contentWidth.toInt()),
+    )
+
+    // Determining the height of the layout based on the measured content
+    val contentHeight = contentPlaceable.height
+    val layoutHeight = additionalContentPlaceable?.let { additional ->
+      maxOf(contentHeight, additional.height)
+    } ?: contentHeight
+
+    val totalWidth = constraints.maxWidth
+
+    // Placing the measured composables in the layout, mirroring in RTL so that
+    // additional content is always on the logical "left" of the timeline and main
+    // content on the "right", but visually flipped when isRtl is true.
+    layout(totalWidth, layoutHeight) {
+      if (isRtl) {
+        // In RTL, place additional content flush to the right so it stays visible
+        additionalContentPlaceable?.placeRelative(
+          x = totalWidth - additionalContentPlaceable.width,
+          y = 0,
+        )
+      } else {
+        // LTR: original behavior, additional content starts from left
+        additionalContentPlaceable?.placeRelative(x = 0, y = 0)
+      }
+
+      // Place main content on the opposite side of the timeline depending on direction
+      val contentX = if (isRtl) {
+        // In RTL, main content should be left of the timeline, but still within bounds
+        (logicalTimelineXOffset - contentPlaceable.width - jetLimeStyle.contentDistance.toPx())
+          .coerceAtLeast(0f)
+          .toInt()
+      } else {
+        logicalContentXOffset.toInt()
+      }
+      contentPlaceable.placeRelative(x = contentX, y = 0)
     }
   }
 }
diff --git a/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeList.kt b/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeList.kt
@@ -34,7 +34,8 @@ import androidx.compose.foundation.lazy.itemsIndexed
 import androidx.compose.foundation.lazy.rememberLazyListState
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.CompositionLocalProvider
-import androidx.compose.runtime.compositionLocalOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.staticCompositionLocalOf
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.unit.dp
@@ -84,7 +85,8 @@ fun <T> JetLimeColumn(
   key: ((index: Int, item: T) -> Any)? = null,
   itemContent: @Composable (index: Int, T, EventPosition) -> Unit,
 ) {
-  CompositionLocalProvider(LocalJetLimeStyle provides style.alignment(VERTICAL)) {
+  val providedStyle = remember(style) { style.alignment(VERTICAL) }
+  CompositionLocalProvider(LocalJetLimeStyle provides providedStyle) {
     LazyColumn(
       modifier = modifier,
       state = listState,
@@ -149,7 +151,8 @@ fun <T> JetLimeRow(
   key: ((index: Int, item: T) -> Any)? = null,
   itemContent: @Composable (index: Int, T, EventPosition) -> Unit,
 ) {
-  CompositionLocalProvider(LocalJetLimeStyle provides style.alignment(HORIZONTAL)) {
+  val providedStyle = remember(style) { style.alignment(HORIZONTAL) }
+  CompositionLocalProvider(LocalJetLimeStyle provides providedStyle) {
     LazyRow(
       modifier = modifier,
       state = listState,
@@ -177,4 +180,4 @@ fun <T> JetLimeRow(
  * This is used to provide a default or overridden style configuration down the composition tree. Accessing this without a provider
  * will result in an error, ensuring that the style is always defined when used within a composable context.
  */
-val LocalJetLimeStyle = compositionLocalOf<JetLimeStyle> { error("No JetLimeStyle provided") }
+val LocalJetLimeStyle = staticCompositionLocalOf<JetLimeStyle> { error("No JetLimeStyle provided") }
diff --git a/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeStyle.kt b/jetlime/src/commonMain/kotlin/com/pushpal/jetlime/JetLimeStyle.kt
