mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-25 02:25:20 +00:00
Snapshot delivery status in message rows so read receipts render immediately
The publish chain was healthy: the store mutates the shared BitchatMessage and republishes, the .statusChanged fan-out reaches both mirrored conversations, and PrivateInboxModel fires objectWillChange for the selected DM under either key. The break was at the row view: TextMessageView/MediaMessageView stored the reference-typed message and read deliveryStatus in body, so SwiftUI's structural diff compared the field by identity - same instance, mutated in place, row body skipped. The blue tick waited for an unrelated invalidation (proven empirically with a hosting-view probe). Rows now snapshot deliveryStatus as a value at init; every republish rebuilds row values with a fresh enum, the diff sees the change, and the row re-renders immediately. Also fixes in-place send-progress updates in media rows. Regression tests cover both mirrored selection keyings at the feature-model level and the snapshot mechanic itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,8 +14,21 @@ struct TextMessageView: View {
|
|||||||
@EnvironmentObject private var conversationUIModel: ConversationUIModel
|
@EnvironmentObject private var conversationUIModel: ConversationUIModel
|
||||||
|
|
||||||
let message: BitchatMessage
|
let message: BitchatMessage
|
||||||
|
/// Value snapshot of the message's mutable delivery status, captured at
|
||||||
|
/// construction. `BitchatMessage` is a reference type mutated in place by
|
||||||
|
/// `ConversationStore`, and SwiftUI compares reference-typed view fields
|
||||||
|
/// by identity — so a status-only change (e.g. delivered → read) on the
|
||||||
|
/// SAME instance would otherwise compare "unchanged" and this row's body
|
||||||
|
/// would be skipped even though the parent list re-rendered. Snapshotting
|
||||||
|
/// the enum makes the change visible to SwiftUI's structural diff.
|
||||||
|
private let deliveryStatus: DeliveryStatus?
|
||||||
@State private var expandedMessageIDs: Set<String> = []
|
@State private var expandedMessageIDs: Set<String> = []
|
||||||
|
|
||||||
|
init(message: BitchatMessage) {
|
||||||
|
self.message = message
|
||||||
|
self.deliveryStatus = message.deliveryStatus
|
||||||
|
}
|
||||||
|
|
||||||
var body: some View {
|
var body: some View {
|
||||||
VStack(alignment: .leading, spacing: 0) {
|
VStack(alignment: .leading, spacing: 0) {
|
||||||
// Precompute heavy token scans once per row
|
// Precompute heavy token scans once per row
|
||||||
@@ -31,7 +44,7 @@ struct TextMessageView: View {
|
|||||||
|
|
||||||
// Delivery status indicator for private messages
|
// Delivery status indicator for private messages
|
||||||
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
||||||
let status = message.deliveryStatus {
|
let status = deliveryStatus {
|
||||||
DeliveryStatusView(status: status)
|
DeliveryStatusView(status: status)
|
||||||
.padding(.leading, 4)
|
.padding(.leading, 4)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -13,11 +13,24 @@ struct MediaMessageView: View {
|
|||||||
@EnvironmentObject private var conversationUIModel: ConversationUIModel
|
@EnvironmentObject private var conversationUIModel: ConversationUIModel
|
||||||
let message: BitchatMessage
|
let message: BitchatMessage
|
||||||
let media: BitchatMessage.Media
|
let media: BitchatMessage.Media
|
||||||
|
/// Value snapshot of the message's mutable delivery status, captured at
|
||||||
|
/// construction (see `TextMessageView.deliveryStatus`): `BitchatMessage`
|
||||||
|
/// is a reference type mutated in place, and SwiftUI compares reference
|
||||||
|
/// fields by identity, so without the snapshot a status-only change
|
||||||
|
/// (send progress, delivered → read) would not re-render this row.
|
||||||
|
private let deliveryStatus: DeliveryStatus?
|
||||||
|
|
||||||
@Binding var imagePreviewURL: URL?
|
@Binding var imagePreviewURL: URL?
|
||||||
|
|
||||||
|
init(message: BitchatMessage, media: BitchatMessage.Media, imagePreviewURL: Binding<URL?>) {
|
||||||
|
self.message = message
|
||||||
|
self.media = media
|
||||||
|
self.deliveryStatus = message.deliveryStatus
|
||||||
|
self._imagePreviewURL = imagePreviewURL
|
||||||
|
}
|
||||||
|
|
||||||
var body: some View {
|
var body: some View {
|
||||||
let state = mediaSendState(for: message)
|
let state = mediaSendState(for: deliveryStatus)
|
||||||
let isFromMe = conversationUIModel.isMediaMessageFromCurrentUser(message)
|
let isFromMe = conversationUIModel.isMediaMessageFromCurrentUser(message)
|
||||||
let cancelAction: (() -> Void)? = state.canCancel ? { conversationUIModel.cancelMediaSend(messageID: message.id) } : nil
|
let cancelAction: (() -> Void)? = state.canCancel ? { conversationUIModel.cancelMediaSend(messageID: message.id) } : nil
|
||||||
|
|
||||||
@@ -27,7 +40,7 @@ struct MediaMessageView: View {
|
|||||||
.fixedSize(horizontal: false, vertical: true)
|
.fixedSize(horizontal: false, vertical: true)
|
||||||
.frame(maxWidth: .infinity, alignment: .leading)
|
.frame(maxWidth: .infinity, alignment: .leading)
|
||||||
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
||||||
let status = message.deliveryStatus {
|
let status = deliveryStatus {
|
||||||
DeliveryStatusView(status: status)
|
DeliveryStatusView(status: status)
|
||||||
.padding(.leading, 4)
|
.padding(.leading, 4)
|
||||||
}
|
}
|
||||||
@@ -63,10 +76,10 @@ struct MediaMessageView: View {
|
|||||||
.padding(.vertical, 4)
|
.padding(.vertical, 4)
|
||||||
}
|
}
|
||||||
|
|
||||||
private func mediaSendState(for message: BitchatMessage) -> (isSending: Bool, progress: Double?, canCancel: Bool) {
|
private func mediaSendState(for deliveryStatus: DeliveryStatus?) -> (isSending: Bool, progress: Double?, canCancel: Bool) {
|
||||||
var isSending = false
|
var isSending = false
|
||||||
var progress: Double?
|
var progress: Double?
|
||||||
if let status = message.deliveryStatus {
|
if let status = deliveryStatus {
|
||||||
switch status {
|
switch status {
|
||||||
case .sending:
|
case .sending:
|
||||||
isSending = true
|
isSending = true
|
||||||
|
|||||||
@@ -298,6 +298,47 @@ struct AppArchitectureTests {
|
|||||||
#expect(inboxModel.messages(for: selectedPeerID).map(\.id) == ["dm-sel-1"])
|
#expect(inboxModel.messages(for: selectedPeerID).map(\.id) == ["dm-sel-1"])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test("PrivateInboxModel republishes read receipts for the selected DM (ephemeral- and stable-keyed)")
|
||||||
|
@MainActor
|
||||||
|
func privateInboxModelRepublishesReadReceiptsForSelectedConversation() {
|
||||||
|
// A DM's messages can live under BOTH .directPeer(ephemeral) and
|
||||||
|
// .directPeer(stableKey) (mirroring shares one BitchatMessage
|
||||||
|
// instance); the view's read-receipt update must fire no matter
|
||||||
|
// which of the two keys the selection holds.
|
||||||
|
let ephemeralPeerID = PeerID(str: "abcdef1234567890")
|
||||||
|
let stablePeerID = PeerID(str: String(repeating: "ab", count: 32))
|
||||||
|
|
||||||
|
for selectedPeerID in [ephemeralPeerID, stablePeerID] {
|
||||||
|
let store = ConversationStore()
|
||||||
|
let inboxModel = PrivateInboxModel(conversations: store)
|
||||||
|
store.setSelectedPrivatePeer(selectedPeerID)
|
||||||
|
|
||||||
|
// One shared instance mirrored into both direct conversations,
|
||||||
|
// exactly like `mirrorToEphemeralIfNeeded`.
|
||||||
|
let message = makeArchitectureMessage(
|
||||||
|
id: "dm-read-1",
|
||||||
|
isPrivate: true,
|
||||||
|
senderPeerID: ephemeralPeerID
|
||||||
|
)
|
||||||
|
store.append(message, to: .directPeer(ephemeralPeerID))
|
||||||
|
store.upsertByID(message, in: .directPeer(stablePeerID))
|
||||||
|
|
||||||
|
var emissions = 0
|
||||||
|
let cancellable = inboxModel.objectWillChange.sink { _ in emissions += 1 }
|
||||||
|
defer { cancellable.cancel() }
|
||||||
|
|
||||||
|
// ID-only intent — the exact call `ChatDeliveryCoordinator`
|
||||||
|
// makes when a READ ack arrives.
|
||||||
|
let read = DeliveryStatus.read(by: "builder", at: Date(timeIntervalSince1970: 100))
|
||||||
|
#expect(store.setDeliveryStatus(read, forMessageID: "dm-read-1"))
|
||||||
|
|
||||||
|
// The fan-out emits .statusChanged for both containing
|
||||||
|
// conversations; exactly the selected one republishes the model.
|
||||||
|
#expect(emissions == 1)
|
||||||
|
#expect(inboxModel.messages(for: selectedPeerID).first?.deliveryStatus == read)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test("PublicChatModel ignores appends to background conversations")
|
@Test("PublicChatModel ignores appends to background conversations")
|
||||||
@MainActor
|
@MainActor
|
||||||
func publicChatModelIsolatesBackgroundConversations() {
|
func publicChatModelIsolatesBackgroundConversations() {
|
||||||
|
|||||||
@@ -629,6 +629,52 @@ struct ViewSmokeTests {
|
|||||||
#expect(playback.progress == 0)
|
#expect(playback.progress == 0)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
func messageRows_snapshotDeliveryStatusForSwiftUIDiffing() {
|
||||||
|
// Regression: `BitchatMessage` is a reference type mutated in place
|
||||||
|
// by `ConversationStore.applyDeliveryStatus`, and SwiftUI compares
|
||||||
|
// reference-typed view fields by identity — so a status-only change
|
||||||
|
// (delivered → read) on the SAME instance is invisible to the row's
|
||||||
|
// structural diff and its body gets skipped even when the list
|
||||||
|
// re-renders. The row views must therefore snapshot the status as a
|
||||||
|
// value-typed stored property at init, so a rebuilt row value
|
||||||
|
// compares different and re-renders.
|
||||||
|
func deliveryStatusSnapshot(of row: Any) -> DeliveryStatus? {
|
||||||
|
Mirror(reflecting: row).children
|
||||||
|
.first { $0.label == "deliveryStatus" }?
|
||||||
|
.value as? DeliveryStatus
|
||||||
|
}
|
||||||
|
|
||||||
|
let delivered = DeliveryStatus.delivered(to: "builder", at: Date(timeIntervalSince1970: 50))
|
||||||
|
let message = BitchatMessage(
|
||||||
|
id: "dm-status-1",
|
||||||
|
sender: "anon",
|
||||||
|
content: "hello",
|
||||||
|
timestamp: Date(),
|
||||||
|
isRelay: false,
|
||||||
|
isPrivate: true,
|
||||||
|
recipientNickname: "builder",
|
||||||
|
senderPeerID: PeerID(str: "abcdef1234567890"),
|
||||||
|
deliveryStatus: delivered
|
||||||
|
)
|
||||||
|
|
||||||
|
#expect(deliveryStatusSnapshot(of: TextMessageView(message: message)) == delivered)
|
||||||
|
|
||||||
|
// In-place mutation of the shared instance (what the store does on a
|
||||||
|
// READ ack); a freshly built row must carry the new status value.
|
||||||
|
let read = DeliveryStatus.read(by: "builder", at: Date(timeIntervalSince1970: 100))
|
||||||
|
message.deliveryStatus = read
|
||||||
|
|
||||||
|
#expect(deliveryStatusSnapshot(of: TextMessageView(message: message)) == read)
|
||||||
|
|
||||||
|
let mediaRow = MediaMessageView(
|
||||||
|
message: message,
|
||||||
|
media: .image(URL(fileURLWithPath: "/tmp/never-loaded.jpg")),
|
||||||
|
imagePreviewURL: .constant(nil)
|
||||||
|
)
|
||||||
|
#expect(deliveryStatusSnapshot(of: mediaRow) == read)
|
||||||
|
}
|
||||||
|
|
||||||
#if os(iOS)
|
#if os(iOS)
|
||||||
@Test
|
@Test
|
||||||
func cameraScannerView_previewAndCoordinatorSmoke() {
|
func cameraScannerView_previewAndCoordinatorSmoke() {
|
||||||
|
|||||||
Reference in New Issue
Block a user