Skip to content

refactor(web): composer clears scroll-collapse in render, not after paint - #25909

Merged
juliusmarminge merged 1 commit into
mainfrom
effect/split-a2-composer-scroll-collapse
Oct 27, 2026
Merged

juliusmarminge merged 1 commit into
mainfrom
effect/split-a2-composer-scroll-collapse

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Problem

ChatComposer cleared its scroll-collapsed flag in a passive useEffect whenever the composer stopped being eligible for scroll-collapse (multiline prompt, expanded chrome, mobile viewport, non-server route, or the "collapse on scroll" setting turned off). The value it reacted to is already computed in render, so the effect only mirrored derived state into local state, and each eligibility drop while collapsed committed the component twice: once with the stale flag, then again after paint.

Change

The flag is plain useState inside useComposerFocusState, called by ChatComposer itself, so it is reset during render with a guard (!canScrollCollapseComposer && isComposerScrollCollapsed), the same pattern the file already uses for the model picker. The guard stops once the flag is false, and canScrollCollapseComposer does not read the flag, so it cannot loop. The effect is deleted.

Behavior: none intended. The flag is still cleared rather than masked, so the composer still does not re-collapse by itself when it becomes eligible again. The only difference is timing: the stale collapsed render is discarded before commit instead of being painted for one frame.

What this improves

  • Sync removed: one effect that mirrored a render-derived value into state.
  • Renders avoided: one extra post-paint commit of ChatComposer (and its layout effects) on each eligibility flip while collapsed.
  • No bug-fix claim, so no regression test.

Effects

  • apps/web/src/components/chat/ChatComposer.tsx: 17 -> 16 useEffect

Verification

  • vp run check:changed: format, lint, knip 1/2, knip 2/2, typecheck (3 packages), test all pass.

Model: Claude Opus 5.5, Claude Code harness.

🤖 Generated with Claude Code

…aint

ChatComposer cleared its scroll-collapsed flag in a passive effect whenever
the composer stopped being eligible for scroll-collapse (multiline prompt,
expanded chrome, mobile viewport, non-server route, or the setting turned
off). That committed the 3.8k-line component once with the stale flag and
then a second time after paint.

The flag lives in useState inside this component (useComposerFocusState),
so reset it during render with a guard, the same pattern this file already
uses for the model picker. The flag is still cleared rather than masked, so
the composer does not re-collapse on its own when it becomes eligible again.

Effects: ChatComposer.tsx 17 -> 16 useEffect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 27, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 325d841

Macroscope's review found this PR approvable — This is a narrowly scoped refactor in ChatComposer that moves an existing state reset before paint, removing a transient stale collapsed frame and an extra commit. It does not change product defaults or add any static-analysis overrides.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 79f6cd3 and 325d841.

Files selected for processing: 1
  • apps/web/src/components/chat/ChatComposer.tsx

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

Walkthrough

ChatComposer now clears its scroll-collapsed state during rendering when scroll-collapse eligibility becomes false. The effect that previously cleared the state after rendering was removed.

Changes: Composer scroll-collapse reset

Layer / File(s) Summary
Reset collapsed state when eligibility is lost
apps/web/src/components/chat/ChatComposer.tsx
When scroll-collapse eligibility becomes false, ChatComposer clears the collapsed state during rendering instead of in an effect.

Priority: Low

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

Change: Refactor

Suggested labels: size:M

Suggested reviewers: macroscopeapp

Merge Risk: Minimal · up to 325d8

The change preserves scroll-collapse reset behavior without introducing an identified merge-blocking risk.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description clearly explains the problem, implementation, unchanged behavior, and verification results. It does not include the required Scope and approval section or explain why this focused refa… Add a Scope and approval section with the linked issue or maintainer approval, or explain why this focused refactor is an obvious bug-free change that qualifies without prior approval. Add before/after UI screenshots, or state why screensho…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the main change: clearing the composer scroll-collapse state during render instead of after paint.
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.
Full details: Description check

Explanation

The description clearly explains the problem, implementation, unchanged behavior, and verification results. It does not include the required Scope and approval section or explain why this focused refactor qualifies without prior approval. It also does not provide the required UI screenshots or state why they could not be provided.

Resolution

Add a Scope and approval section with the linked issue or maintainer approval, or explain why this focused refactor is an obvious bug-free change that qualifies without prior approval. Add before/after UI screenshots, or state why screenshots are not meaningful for this timing-only change.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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.

@juliusmarminge
juliusmarminge merged commit c39a115 into main Oct 27, 2026
30 checks passed
@juliusmarminge
juliusmarminge deleted the effect/split-a2-composer-scroll-collapse branch October 27, 2026 12:17
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Oct 27, 2026
## What's Changed
* refactor(mobile): subagent timers share the visible second clock by @juliusmarminge in pingdotgg/t3code#25854
* refactor(mobile): app update checks start with the app by @juliusmarminge in pingdotgg/t3code#25856
* refactor(mobile): workspace tabs, home filters and connection status derive instead of syncing by @juliusmarminge in pingdotgg/t3code#25813
* refactor(mobile): settings permission and update checks without sync effects by @juliusmarminge in pingdotgg/t3code#25895
* fix(desktop): run the packaged bundle self-check on macOS artifacts by @stbenjam in pingdotgg/t3code#25900
* fix(web): the Add ChatGPT account dialog animates closed once the account connects by @juliusmarminge in pingdotgg/t3code#25868
* refactor(web): composer clears scroll-collapse in render, not after paint by @juliusmarminge in pingdotgg/t3code#25909
* refactor(web): composer overlay re-measures from one layout effect by @juliusmarminge in pingdotgg/t3code#25915
* refactor(web): file browser records the right-click position on its own panel by @juliusmarminge in pingdotgg/t3code#25908
* refactor(web): preview element picker drops its unmount guard by @juliusmarminge in pingdotgg/t3code#25920
* fix(web): PR Code tab no longer requests the diff of a commit the PR dropped by @juliusmarminge in pingdotgg/t3code#25916
* perf(mobile): worktree setup updates commit the thread screen once by @juliusmarminge in pingdotgg/t3code#25910
* fix(desktop): a broken release opens an error window instead of hanging by @t3dotgg in pingdotgg/t3code#25903
* ci(web): releases fail when the built web app cannot boot by @t3dotgg in pingdotgg/t3code#25929
* ci(release): desktop builds that cannot open no longer publish by @t3dotgg in pingdotgg/t3code#25928
* refactor(web): file tree drag mentions use React drag handlers by @juliusmarminge in pingdotgg/t3code#25913
* refactor(mobile): git action results buzz from where they are reported by @juliusmarminge in pingdotgg/t3code#25912
* fix(mobile): the Add environment form shows only its own pairing errors by @juliusmarminge in pingdotgg/t3code#25859
* refactor(mobile): showcase capture runs as an app service, not a chain of effects by @juliusmarminge in pingdotgg/t3code#25857
* fix(server): bound hung delegated stop effects so later runs can start by @juliusmarminge in pingdotgg/t3code#25675
* feat(mobile): add One UI navigation preference on Android by @PixPMusic in pingdotgg/t3code#25758


**Full Changelog**: pingdotgg/t3code@v0.1.4-nightly.20261027.3770...v0.1.4-nightly.20261027.3783

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.1.4-nightly.20261027.3783
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant