Skip to content

fix(messages): keep a send reveal pending until its scroll runs - #454

Merged
wesbillman merged 2 commits into
mainfrom
send-reveal
Sep 30, 2026
Merged

wesbillman merged 2 commits into
mainfrom
send-reveal

Conversation

@matt2e

@matt2e matt2e commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Summary

The timeline recorded a sent message as revealed as soon as the layout effect scheduled the reveal frame. If a row update (an echo, an edit, or an older-history prepend) landed before that frame ran, it cancelled the frame. The effect then ran again but skipped the reveal because the ID was already recorded, so the sent row stayed off screen.

This came up in a review thread on #401. The defect is in main (introduced in #393), so the fix gets its own PR.

Changes

  • ChannelTimeline.tsx: the reveal now stays pending under the intent that scheduled it, and is only recorded as revealed once the scroll actually runs.
    • If the effect reruns under the same intent, the reveal is rescheduled at the row's current index.
    • Any newer intent (reader input, jump to latest, a message target) cancels it.
  • ChannelTimeline.restore.test.tsx:
    • New case: an older-history prepend arriving before the reveal frame still reveals the sent row at its shifted index. This is tested both with and without the reader scrolling up first.
    • The existing reader-input case now also checks that a prepend doesn't override the reader's intent.

🤖 Generated with Claude Code

The timeline marked a sent message as revealed as soon as the layout
effect scheduled the reveal frame. A row update landing before that
frame (an echo, an edit, or an older-history prepend) cancelled the
frame, and the rerun then skipped the reveal because the ID was already
recorded, leaving the sent row off screen.

Track the reveal as pending under the intent that scheduled it. It is
only recorded as revealed once the scroll runs; a rerun under the same
intent reschedules it at the row's current index, while any newer intent
(reader input, jump to latest, a message target) retires it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested review from a team, comp615 and wesbillman as code owners September 30, 2026 06:33

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actionable findings in this change: pending send reveals survive row-refresh/prepend cancellation, while newer reader and prepared message-target intents retire them.

Star Lord automated source-only review via Wes’s account (wesbillman), head 6e55d070a3fdb0d9bf0ee32be00893339de80a1e, base 5d2b08e2ff3bb40dc62f04015298ff319f4a4f8a. No tests or app workflows were executed; JavaScript and several browser checks were still running in the one-time CI snapshot, so runtime scroll/focus behavior remains unverified.

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 No blocking implementation defects found. One nonblocking regression-test gap below.

Comment thread src/features/messages/ChannelTimeline.tsx
…repend

Review on #454 noted that moving the reveal's completion into the
scroll callback left it unguarded: deleting that block kept every
timeline test green, even though a same-intent row update would then
reveal the sent row again and a later history page would scroll it
back into view.

Add a case that completes a reveal, then prepends older history and
asserts no further scroll runs, for both the restore-first and
bottom-follow mounts. Both cases fail with the completion block removed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actionable source findings in this revision: the added regression checks that a completed send reveal does not restart after a history prepend, covering both saved-reading and bottom-follow starts.

Star Lord automated source-only review via Wes’s account; head 5410d297f8750b4515192da9d11b8b022df093e1, base 5d2b08e2ff3bb40dc62f04015298ff319f4a4f8a.

No tests or app execution performed. The read-only CI snapshot is not green: WebKit shard 1/6 failed the panel-resize bottom-follow check after closing the channel panel (560px gap; expected <4px), so runtime validation remains unresolved.

@wesbillman
wesbillman merged commit 838959f into main Sep 30, 2026
50 of 54 checks passed
@wesbillman
wesbillman deleted the send-reveal branch September 30, 2026 14:41
TheSentinel454 pushed a commit that referenced this pull request Sep 30, 2026
* origin/main: (27 commits)
  Let plugin pages publish NIP-AR artifacts and embed the host thread view (#434)
  test(app): migrate entity-navigation test off removed buzz://open locator API (#463)
  Show agent activity in navigation (#423)
  test(browser): hold motion when it commits, not on its start event (#459)
  fix(navigation): ignore unknown query parameters on Buzz links and remove the buzz://open locator (#457)
  feat(design-system): distinguish controls on floating surfaces (#429)
  feat(native): add community extras and media preparation (#450)
  Clone inventory identities through reviewed text and fresh identity creation (#289)
  feat(communities): add right-click actions to the community rail (#400)
  fix(messages): keep a send reveal pending until its scroll runs (#454)
  fix(messages): reserve a stable scrollbar gutter on the channel feed (#451)
  fix(sidebar): list plugin pages as sidebar rows via an opt-in primary flag (#401)
  feat(channels): surface canvas content in channel settings (#426)
  fix(profiles): remove redundant presence status row (#394)
  test(browser): count live retries once the page handles startup controls (#443)
  feat(composer): host-owned resource links for the Projects picker (#445)
  feat: support native read state and recent channel activity (#444)
  feat(native): serve relay media and uploads in packaged builds (#433)
  feat(channels): suggest joined channels in the composer (#446)
  feat: support native agent activity, library, memories, and community resolution (#441)
  ...

Signed-off-by: Codex <noreply@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants