Skip to content

fix: preserve reading position when unpinning messages - #1165

Merged
xHeaven merged 2 commits into
dev-null-sh:productionfrom
xHeaven:fix/pinned-message-scroll
Oct 26, 2026
Merged

xHeaven merged 2 commits into
dev-null-sh:productionfrom
xHeaven:fix/pinned-message-scroll

Conversation

@xHeaven

@xHeaven xHeaven commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Summary

Removing the final pinned message while reading the oldest messages moved the thread upward by 36 px. Keep the reading position fixed when that pin disappears, including when the same poll also delivers a new message.

Changes

Keep the banner's existing inset while removing it would clamp the reverse scroller. Check after restoring the reading anchor, then release it after scrolling away from the boundary or immediately at the latest messages. Unpinned chats keep their normal spacing on fresh loads.

Validation

  • composer test:lint — passed; existing warnings remain.
  • composer test:type-coverage — 100%.
  • bun run test:unit — 190 Node tests and 535 component tests passed.
  • bun run test:types and bun run build — passed.
  • Full PHP CI coverage command — 4,441 tests, 24,839 assertions, 100.0% coverage. Used an isolated database with test-default Remotes flags. An earlier run inherited the demo flags and failed 10 unrelated preference/restriction cases; the corrected full run passes.
  • Actual endpoint/polling scenarios: 1440×900, 1366×650, 390×844, light/dark. Before: 36 px drift in all 6 cases. After: 0 px drift and 0 layout shifts in all 6 cases.
  • Combined new-message/final-unpin polls in all 6 cases: zero layout shifts, at most 0.25 px of existing anchor rounding, versus about 36 px before correcting the effect order.
  • Browser regression scenarios are temporary validation, not persistent CI tests; committed browser tests are prohibited by repository rules.
  • Three repeated pin/unpin cycles, user scroll release, bottom/fresh loads, compact dock, short history, and a simulated unpin 500 rollback checked. No browser tests or capture scripts committed.

Visual evidence

Local seeded fictional conversations in Chrome. Each recording shows the last pin disappearing through a real server update and automatic polling.

Desktop before Desktop after
Before: final pin removal shifts the messages After: messages stay fixed

Before desktop MP4 · After desktop MP4

Mobile before (390×844, dark, viewport emulation) Mobile after
Before mobile polling jump After mobile stable polling

Before mobile MP4 · After mobile MP4

Screenshot comparisons after final-pin removal
Viewport/theme Before After
1440×900 light Before wide light After wide light
1440×900 dark Before wide dark After wide dark
1366×650 light Before short light After short light
1366×650 dark Before short dark After short dark
390×844 light Before mobile light After mobile light
390×844 dark Before mobile dark After mobile dark

Combined message + final-unpin polling: desktop GIF · desktop MP4 · mobile GIF · mobile MP4

Compact dock · Simulated unpin 500 rollback · Short history

Device verification

Safari on an iPhone 18 Pro Max simulator running iOS 27.0: touch scrolling to the oldest messages, a new message and final unpin arriving together by polling, and stable reading position verified. The focused composer and enabled Push action remain usable above the navigation/browser controls. Checked fresh navigation and live updates.

iOS Safari simulator: final pin disappears without moving the messages

iOS recording MP4 · Before · After

Independent review

Independent Sol 6.1 reviewer, high reasoning / fast service: CODE: PASS; VISUAL: PASS, no unresolved findings. Reviewed base 1481df136343248a704b8f06d97ce6a3a8450d63 → head 7f60cd82a496b6621b965c057d7c07f4ca15eaae and all 43 media assets in manifest SHA256 cae933c7403a3a2a026c65a1ac3abe6dd31cd799e21a06387fadc16bd6a067b4. Independently replayed the combined poll twice and verified safe inset release afterward. Recordings inspected through sampled frames; native evidence and passing test logs inspected, not independently rerun.

Limitations

  • Existing relative-time labels can produce tiny unrelated layout-shift entries during longer observations; bounded message-update captures reported zero.
  • No physical-device or Android verification. Android SDK unavailable.
  • The iOS simulator has a hardware keyboard connected. Composer focus/typing and the primary action were checked; the full on-screen keyboard could not be shown. No inputs or actions change in this patch.
  • Short bottom-aligned history can report a layout-shift entry for the empty list box when its padding shrinks, although its messages stay fixed. The exact base produces the same 0.003265 entry; this patch targets the 36 px movement of visible messages at the oldest scroll boundary.

Summary by CodeRabbit

  • Bug Fixes
    • Improved conversation scrolling when pinned messages are added or removed. The message list now preserves the reading position when clearing space left by the pinned-message banner, while still returning to the latest message when appropriate.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3d39ef8b-564b-4a8c-9e0f-6c6d5c2043ce
Commits

Reviewing files that changed from the base of the PR and between 1481df1 and 7f60cd8.

Files selected for processing: 1
  • resources/js/components/chat/conversation-thread.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

ConversationThread now tracks pin clearance independently of pinned messages. It retains top padding after pins disappear until the reverse scroller can release it without moving the reading position.

Changes: Pinned-message clearance

Layer / File(s) Summary
Track and release pin clearance
resources/js/components/chat/conversation-thread.tsx
ConversationThread enables clearance when pins appear. It releases clearance after pins are gone when the scroller is at the latest message or has enough room above. The list’s top padding follows the clearance state.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

Suggested reviewers: simonhamp

Merge Risk: Minimal · up to 7f60c

No identified issue blocks merging the reading-position fix after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving the reading position when messages are unpinned.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@xHeaven
xHeaven merged commit 9285f65 into dev-null-sh:production Oct 26, 2026
7 checks passed
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.

1 participant