Fix correctness, performance, and polish issues in timeline rendering#66
Merged
Conversation
- Make JetLimeStyle.arrangement an immutable constructor param so a shared
style instance is never mutated across Column/Row usages, and include it
in equals/hashCode.
- Drop keyless remember { ... } wrapping lineVerticalAlignment /
lineHorizontalAlignment so alignment changes on the provided style
propagate instead of being frozen to the first composition.
- Remove duplicate upward-line drawLine for CENTER point placement that was
painting the same segment twice (visible under alpha/dashed brushes).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- calculateRadiusAnimFactor: only create rememberInfiniteTransition when pointAnimation is non-null. Previously every item (even without an animation) allocated an infinite transition and registered it with the recomposer, wasting frame callbacks on large timelines. - JetLimeColumn / JetLimeRow: remember the arrangement-stamped JetLimeStyle so the value provided to LocalJetLimeStyle is stable across recompositions, letting Compose skip unchanged items. - JetLimeExtendedEvent: drop BoxWithConstraints subcomposition and the separate Canvas child. The timeline X offset is now published from measure into a MutableFloatState read by a drawBehind modifier on the Layout itself, so state writes only invalidate the draw phase instead of triggering sibling recomposition. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- EventPosition.isNotStart() now compares against the private START singleton instead of the literal string "Start", matching isNotEnd() and removing a silent-break risk if the name ever changes. - Swap LocalJetLimeStyle to staticCompositionLocalOf. Every timeline item reads the style and it is already stabilized via remember in JetLimeColumn / JetLimeRow, so a static local avoids per-reader invalidation bookkeeping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three-phase cleanup of the
:jetlimelibrary (commonMain). Public API is unchanged.Critical correctness (f2a3324)
JetLimeStyle.arrangementis now aninternal valconstructor param;alignment()returns a new instance instead of mutatingthis. Prevents silent corruption when a single style instance is shared between a Column and a Row.remember { ... }aroundlineVerticalAlignment/lineHorizontalAlignment— the cached values never updated when the caller swapped the style.drawLinecall forCENTERpoint placement (vertical) that painted the same segment twice — visible under alpha / dashed brushes.Performance / recomposition (6227ddf)
calculateRadiusAnimFactoronly callsrememberInfiniteTransitionwhenpointAnimation != null. Previously every item registered an unused infinite transition with the Recomposer.JetLimeColumn/JetLimeRowremember(style) { style.alignment(...) }so the value provided toLocalJetLimeStyleis stable across recompositions and consumers can skip.JetLimeExtendedEventno longer usesBoxWithConstraintsor a siblingCanvas. The timeline X offset is published from measure into aMutableFloatStateread by aModifier.drawBehindon theLayoutitself, so the state write only invalidates the draw phase instead of triggering recomposition of a sibling composable.Polish (e5edc1c)
EventPosition.isNotStart()compares against theSTARTsingleton instead of the literal string"Start", matchingisNotEnd().LocalJetLimeStyleswitched tostaticCompositionLocalOf— the style is now stable across recompositions and read by every item.Test plan
./gradlew :jetlime:compileDebugKotlinAndroid— clean build./gradlew :jetlime:spotlessCheck— passes./gradlew :jetlime:connectedAndroidTest— run existing instrumented suite (Column/Row/RTL tests) on a device / emulator before mergePointPlacementvariants in both LTR and RTLDeferred to a future major version
Tracked separately — all require source or binary breaks:
JetLimeEventStyle.position/pointPlacementvar→valItemsListbacked byImmutableList<T>EventPointType.equalssemantics overPainterEventPosition→enum classremembereventStyle(...)per item🤖 Generated with Claude Code