Skip to content

fix: clear evicted prepend anchors #479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
agent-p1p wants to merge 1 commit into from
Closed

fix: clear evicted prepend anchors #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 53 additions & 18 deletions whitenoise-mac/Views/MessengerShellView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,22 @@ nonisolated enum TimelineNewestMessageScrollAction: Equatable {
case scrollToBottom
}

nonisolated enum TimelineOldestMessageScrollAction: Equatable {
case none
case clearPendingPrependAnchor
case restorePendingPrependAnchor(String)
}

func timelineOldestMessageScrollAction(
messageIDs: [String],
pendingPrependAnchorId: String?
) -> TimelineOldestMessageScrollAction {
guard let pendingPrependAnchorId else { return .none }
return messageIDs.contains(pendingPrependAnchorId)
? .restorePendingPrependAnchor(pendingPrependAnchorId)
: .clearPendingPrependAnchor
}

func timelineNewestMessageScrollAction(
messageIDs: [String],
newMessageIsOutgoing: Bool,
Expand Down Expand Up @@ -427,24 +443,36 @@ private struct ConversationView: View {
}
}
.onChange(of: messageIDs.first) { _, _ in
guard let anchorId = pendingPrependAnchorId,
workspace.selectedTimelineContainsMessage(anchorId)
else { return }
DispatchQueue.main.async {
// Re-validate against live state: the user may have switched
// chats (which clears pendingPrependAnchorId) or another prepend
// may have landed between scheduling and execution of this block.
// Without re-checking, proxy.scrollTo would run against the new
// conversation using a stale anchor, and the unconditional clear
// would drop restoration for a subsequent legitimate prepend.
guard workspace.selectedChat?.id == chat.id,
pendingPrependAnchorId == anchorId,
workspace.selectedTimelineContainsMessage(anchorId)
else { return }
TimelineSignpost.scroll.interval("restorePrependAnchor") {
proxy.scrollTo(anchorId, anchor: .top)
switch timelineOldestMessageScrollAction(
messageIDs: messageIDs,
pendingPrependAnchorId: pendingPrependAnchorId
) {
case .restorePendingPrependAnchor(let anchorId):
DispatchQueue.main.async {
guard pendingPrependAnchorId == anchorId else { return }
guard workspace.selectedChat?.id == chat.id else {
pendingPrependAnchorId = nil
return
}
switch timelineOldestMessageScrollAction(
messageIDs: workspace.selectedMessageIDs,
pendingPrependAnchorId: pendingPrependAnchorId
) {
case .restorePendingPrependAnchor(let anchorId):
TimelineSignpost.scroll.interval("restorePrependAnchor") {
proxy.scrollTo(anchorId, anchor: .top)
}
pendingPrependAnchorId = nil
case .clearPendingPrependAnchor:
pendingPrependAnchorId = nil
case .none:
break
}
}
case .clearPendingPrependAnchor:
pendingPrependAnchorId = nil
case .none:
return
}
}
}
Expand Down Expand Up @@ -659,8 +687,15 @@ private struct ConversationView: View {
Task {
await workspace.loadOlderMessages(groupIdHex: chat.id)
// Fallback clear when no restoration occurs (e.g. already at the oldest message,
// so `messageIDs.first` never changes and the `.first` onChange won't fire).
if pendingPrependAnchorId == anchorId, workspace.selectedMessageIDs.first == anchorId {
// so `messageIDs.first` never changes and the `.first` onChange won't fire), or
// when a concurrent live append evicted the anchor from the capped window before
// restore could run.
guard pendingPrependAnchorId == anchorId else { return }
let messageIDs = workspace.selectedMessageIDs
if timelineOldestMessageScrollAction(
messageIDs: messageIDs,
pendingPrependAnchorId: anchorId
) == .clearPendingPrependAnchor || messageIDs.first == anchorId {
pendingPrependAnchorId = nil
}
}
Expand Down
18 changes: 18 additions & 0 deletions whitenoise-macTests/whitenoise_macTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8167,6 +8167,24 @@ struct whitenoise_macTests {
) == .scrollToBottom)
}

@Test func olderTimelinePagingClearsEvictedPrependAnchor() {
#expect(
timelineOldestMessageScrollAction(
messageIDs: ["message-150", "message-249"],
pendingPrependAnchorId: "message-100"
) == .clearPendingPrependAnchor)
#expect(
timelineOldestMessageScrollAction(
messageIDs: ["message-050", "message-100", "message-150"],
pendingPrependAnchorId: "message-100"
) == .restorePendingPrependAnchor("message-100"))
#expect(
timelineOldestMessageScrollAction(
messageIDs: ["message-150", "message-249"],
pendingPrependAnchorId: nil
) == .none)
}

@Test func newestMessageAutoScrollUsesBottomProximityNotOlderHistoryAvailability() {
let longLiveEdgePaging = TimelinePagingState(
hasMoreBefore: true,
Expand Down
Loading