Repository navigation
fix(mobile): keep a sent thread reply above the composer - #9026
Conversation
A sent reply grows in over its entrance transition, and the one-shot tail correction could finish before that growth did. The list then stopped short of its real end, hiding the reply under the composer and leaving the Latest button visible at the apparent bottom. While the thread follows its tail and the last reply is laid out, keep a position that was at its previous end on the new end as the extent grows. Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@localhost> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
johnmatthewtennant
left a comment
There was a problem hiding this comment.
🤖 Reviewed exact head d7dca52ddfdb26ff49c55ae33fedd2b3a6b74fbd with the complete teammate-review workflow. No correctness blockers found in the thread tail-growth correction.
A thread opened at a specific reply (Inbox, push or link) skipped the new-reply follow logic for its whole lifetime, so after the reader scrolled, their own sent reply never came into view. Absorb only the replies the entry hydrated with, then use the ordinary follow rules. Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
…roll Signed-off-by: Tom Brow <tomb@squareup.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b82927048b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A targeted entry starts at its target, not the tail. Absorb the | ||
| // replies it hydrated with once; later arrivals use the ordinary rules | ||
| // below, so a reply sent from this page still comes into view. | ||
| if (initialMessageId != null && !initialTailSettle.isComplete) { |
There was a problem hiding this comment.
When a targeted thread initially renders a provisional snapshot and the authoritative query later adds older replies, the earlier target-positioning effect has already called initialTailSettle.abandon(), so this condition is false and the hydration delta falls through to the new-reply logic. If any of those historical replies was authored by currentPubkey, hasNewLocalReply becomes true, clears the target's opt-out, and schedules a jump to the tail, overriding the message opened from Inbox, a notification, or a link. Track whether the initial targeted hydration has been absorbed independently of initialTailSettle; the added test currently preloads all replies and uses only another author's replies, so it does not exercise this case.
Useful? React with 👍 / 👎.
A targeted entry hydrates twice: the route's snapshot of the target, then the relay's thread. Absorbing only the first let the second, when it held the reader's own replies, schedule a tail correction that cancelled the one-shot target jump, leaving the list hidden. Absorb hydration until the target jump reveals the list. Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f577ae3cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the target jump reveals the list; a tail correction scheduled earlier | ||
| // would cancel that one-shot jump and leave the list hidden. Later | ||
| // arrivals use the ordinary rules below, so a sent reply comes into view. | ||
| if (initialMessageId != null && !initialViewportReady.value) { |
There was a problem hiding this comment.
When a targeted thread is already visible and its relay session reconnects, threadRepliesProvider automatically refreshes (thread_replies_provider.dart:44-50), so this condition now lets any increase in the refreshed result fall through to the new-reply logic. That logic uses replies.skip(previous) rather than reply IDs: if the refresh inserts one older remote reply while the previously newest reply belongs to the current user, the shifted suffix is misclassified as a new local send, clears the target opt-out, and jumps from the linked message to the tail. The added delayed-hydration test completes before target readiness and therefore misses this later production refresh path; distinguish optimistic local reply IDs from authoritative refresh additions before enabling tail follow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[P2] Preserve the target through provisional-list dragging
This also appears reachable before initial hydration completes: the provisional list is visible while relayRepliesAvailable is false, and dragging it sets initialViewportReady before the target jump runs. If hydration then includes historical self-authored replies, the ordinary follow path can supersede the pending target jump. Could target completion be tracked separately, with only genuinely new local sends overriding that intent? That would also address the reconnect case above.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
…roll Signed-off-by: Tom Brow <tomb@squareup.com> # Conflicts: # mobile/lib/features/channels/thread_detail_page.dart
tlongwell-block
left a comment
There was a problem hiding this comment.
🤖 Meli, Tyler’s AI agent, posting on his explicit behalf.
Approved as directed by Tyler.
Full mobile suite passed. Mutation checks demonstrated that the regression tests detect both targeted fixes. Simulator before/after verification remains incomplete.
Reviewed head: ca7d5f42b234d19d070bed56cc7075458172cf68.
kazemek21
left a comment
There was a problem hiding this comment.
Kyle reviewed this PR and the Nexus review-tracker notes. Approved with the non-blocking P2 comment.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
…ction-p * origin/main: (111 commits) feat(mobile): tag the reacted-to message author on reactions (#9141) fix(mobile): steady New DM picker rows and animate picks (#9249) perf(mobile): keep remote images on disk and show small ones as thumbnails (#9158) fix(relay): deliver slow cross-node events late instead of closing their recipients (#9235) fix(mobile): read only shown events from filtered Activity rows (#9215) fix(mobile): show the channel name as the notification subtitle (#9173) fix(mobile): keep Home's header clean under sheets (#9152) fix(mobile): show already-loaded custom emoji in the iOS reactions sheet (#9150) mobile: add follow and remind actions to inbox row menu (#9142) fix(mobile): start no threads in Solo chats (#9123) mobile: persist user profiles so names and avatars render on launch (#9116) fix(mobile): keep a sent thread reply above the composer (#9026) [buzz] Recover database pool capacity after stalled connections (#9267) feat: send a User-Agent on relay WebSocket upgrades (#9246) Record why relay connections close and quiet close warnings (#9245) fix(relay): say "channel owner or admin" in the call_webhook save refusal (#9169) feat(mobile): consolidate Solo chat settings in one sheet (#9255) fix(cli): preserve attachments across successive edits (#9226) fix(mobile): swipe left to reply so swipe-back keeps rightward drags (#9127) mobile: long-press a message link for open, copy and share (#9135) ... Signed-off-by: Matthew Boston <mboston@squareup.com>
Fixes two thread symptoms that come from the same state: after sending a reply, the composer clears but the new reply sits hidden under the composer (or out of view entirely) until you leave and return, and the Latest button stays visible even though the list looks like it is at the bottom.
Cause
LocalSendRow). The thread's one-shot tail correction can finish while the row is still growing. Once it has finished, nothing moves the list as the reply keeps growing, so the list stops short of the real end (about 40 pt on an iPhone 17 Pro simulator) and the reply ends up behind the composer. Because the list isn't actually at its end, Latest shows. It is visible even with the composer focused when an earlier drag detached tail-follow.Fix
ScrollMetricsNotificationlistener keeps a position that was at its previous end on the new end whenever the extent grows. It only acts when the last reply is laid out, the reader isn't dragging or opted out, and no navigation to the tail is in progress, so programmatic jumps away from the tail are left alone. This also covers other growth at the tail, such as media or typing rows.Before and after
Same flow on an iPhone 17 Pro simulator, in a synthetic QA channel thread: scroll the list away from the bottom, focus the composer, send a long reply that wraps onto many lines. Before (main) the sent reply ends with its bottom edge under the composer; after (this branch) it ends clear above it.
Before (main):
ev9026-before.mp4
After (this branch):
ev9026-after.mp4
Validation
a sent reply stays above the composer as it grows innow runs for a plain entry, a targeted entry (initialMessageId), and a targeted entry whose relay hydration is delayed and includes the reader's own replies. The targeted row opens at an older reply, checks it is visible, drags a little, focuses the composer with the keyboard up, sends a multi-line reply, and checks that the reply ends above the composer and Latest is hidden. Each row fails without its fix (growth: reply bottom at 504, composer top at 402; targeted: the reply is never followed; delayed hydration: the target never becomes hit-testable because the list stays hidden).flutter analyzeand mobile test suite pass.The simulator recording was taken in an internal community, so it is shared internally rather than embedded here.
🤖 Generated with Claude Code