Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds thread display functionality to the status detail view by implementing date-time comparison capabilities and enhancing thread structure in status contexts. The changes enable proper ordering and filtering of threaded conversations based on creation timestamps.
- Adds
compareTooperator forUiDateTimeacross platforms to enable chronological comparison - Enhances
StatusContextPresenterto filter thread parents based on the current status timestamp - Modifies Bluesky data source to better structure threaded replies with proper parent-child relationships
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| UiDateTime.kt | Adds expected compareTo operator declaration for cross-platform date comparison |
| StatusContextPresenter.kt | Implements thread filtering logic based on creation timestamps and adds async transform support |
| TimelinePresenter.kt | Changes transform method signature to support async operations |
| StatusDetailRemoteMediator.kt | Restructures Bluesky thread data to use FeedViewPost wrapper with proper reply references |
| UiDateTime.apple.kt | Implements compareTo operator for iOS/macOS using Kotlin instant conversion |
| UiDateTime.androidJvm.kt | Implements compareTo operator for Android/JVM using direct instant comparison |
| service.context(statusKey) | ||
| }.combine(currentStatusCreatedAtFlow) { loader, _ -> | ||
| loader | ||
| } |
There was a problem hiding this comment.
The underscore parameter _ suggests the currentStatusCreatedAtFlow value is being ignored in the combine operation. Consider removing this combine if the flow value isn't needed, or use the value if it's intended for triggering loader updates.
| } | |
| }.sample(currentStatusCreatedAtFlow) |
| if (last != null) { | ||
| val parents = | ||
| listOfNotNull(it.value.post) + | ||
| it.value.replies.toList().dropLast(1).mapNotNull { |
There was a problem hiding this comment.
Converting immutable collection to list with toList() and then calling dropLast(1) creates unnecessary intermediate collections. Consider using take(it.value.replies.size - 1) or direct indexing for better performance.
| it.value.replies.toList().dropLast(1).mapNotNull { | |
| it.value.replies.take(it.value.replies.size - 1).mapNotNull { |
| override fun transform(data: UiTimeline): UiTimeline = | ||
| data.copy( | ||
| override suspend fun transform(data: UiTimeline): UiTimeline { | ||
| val currentCreatedAt = currentStatusCreatedAtFlow.firstOrNull() |
There was a problem hiding this comment.
Calling firstOrNull() on the flow in every transform operation could be inefficient. Consider passing the current timestamp as a parameter to the transform method or caching it at a higher level.
No description provided.