Skip to content

[Fix] Approval controls disappear when user scrolls through chat - #1448

Merged
edelauna merged 2 commits into
mainfrom
fix/keep-approval-controls-visible-2rxure6hwxnz3
Aug 31, 2026
Merged

[Fix] Approval controls disappear when user scrolls through chat#1448
edelauna merged 2 commits into
mainfrom
fix/keep-approval-controls-visible-2rxure6hwxnz3

Conversation

@zoomote

@zoomote zoomote Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

​Created by Roomote. Follow up by mentioning @roomote, in the web UI, or in Discord.

What changed

  • Keep pending approval controls visible beside a compact scroll-to-bottom action while users browse earlier chat output.
  • Prioritize approvals over the optional checkpoint shortcut and truncate long localized labels safely in narrow sidebars.
  • Cover single- and two-button approval states, both approval click handlers, checkpoint suppression, and fallback tooltip labels while browsing history.
  • Reduce the production diff so coverage and review focus on the behavior change instead of indentation churn.

Why this change was made

Scrolling previously replaced the entire action row with navigation controls. A child task waiting for completion approval therefore appeared stuck because the only control that could return to its parent was no longer rendered.

Closes #1447.

Impact

Users can approve subtask completion and other blocking actions without first changing their chat scroll position. The regression was developed test-first and the full repository suite passes. A post-fix extension screenshot is not included because this task's approved proof browser exposes only the documentation site, not the VS Code Extension Host surface.

Related PRs

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Review process

This PR was opened by an automated account. A human maintainer must verify the change intent, provenance, and validation before merging.

  1. Required CI checks pass.
  2. The workflow starts CodeRabbit automatically.
  3. For eligible human-authored PRs, CodeRabbit reviews and approves the latest commit.
  4. A human maintainer reviews and approves after CodeRabbit.

Current step: The required review sequence passed. Remaining merge requirements apply.

@edelauna

Copy link
Copy Markdown
Contributor

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 12e359eb-2cd3-41ab-a184-8f8d8469ce6a

📥 Commits

Reviewing files that changed from the base of the PR and between 97265fd and 346d34f.

📒 Files selected for processing (2)
  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (6)
Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases. Check cleanup and deterministic async behavior and prefer shared typed test helpe...

⚙️ CodeRabbit configuration file

Files:

  • webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths. Verify promises and errors are handled, existing helpers are reused, and new code introduces no `any`, unjustified dou...

⚙️ CodeRabbit configuration file

Files:

  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx
Check React state and effect dependencies, cleanup, accessibility, i18n, and light/dark theme behavior. New markup should use Tailwind; add VS Code CSS variables to `src/index.css` before Tailwind use. Use Vitest for behavior and Playwright...

⚙️ CodeRabbit configuration file

Files:

  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx
Act as an adversarial second-opinion reviewer. Verify PR claims against implementation, contracts, and tests. Trace changed inputs through normal, boundary, error, cancellation, retry, and default paths and their consumers. Seek plausible c...

⚙️ CodeRabbit configuration file

Files:

  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx
Add focused tests for UI binding and save behavior, persistence or normalization, and the value returned by `getStateToPostToWebview()`, including true and false/unset cases when defaults could hide omissions.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx
Fix lint violations in new TypeScript code instead of suppressing them.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • webview-ui/src/components/chat/ChatView.tsx
  • webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx
🔇 Additional comments (2)
webview-ui/src/components/chat/ChatView.tsx (1)

1644-1645: LGTM!

Also applies to: 1739-1755, 1767-1821

webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx (1)

505-538: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Approval controls now remain visible while browsing earlier chat messages.
    • The scroll-to-bottom control is displayed alongside approval actions when applicable.
    • Approval tooltips and button labels now display more reliably, including for longer text.
  • Tests

    • Added regression coverage for chat history navigation with pending approval actions.

Walkthrough

ChatView now displays scroll-to-bottom and approval controls at the same time. It adjusts button layout and label handling. A regression test verifies that finishTask approval remains available while browsing earlier messages.

Changes

Chat action-row behavior

Layer / File(s) Summary
Combined action-row rendering
webview-ui/src/components/chat/ChatView.tsx, webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx
ChatView renders scroll and approval buttons together, hides checkpoint navigation when approval buttons exist, provides tooltip fallbacks, truncates labels, and tests the finishTask approval state while scrolled up.

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

Merge Risk: ⚪ Minimal · up to 346d3

This change keeps existing approval controls visible while users browse earlier chat output and adds regression coverage for the affected interaction. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: edelauna

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Regression Evidence ⚠️ Warning The focused JSDOM regression test covers the main finishTask case: it keeps the chevron, hides the checkpoint shortcut, and preserves the approval label while browsing history (`ChatView.scroll-debu… Add a Playwright component visual test and committed baseline for the ChatView action row. Cover the scrolled state with the compact scroll-to-bottom control beside an approval control, checkpoint suppression, and a long localized label in …
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary fix: approval controls remain available while users scroll through chat.
Description check ✅ Passed The description explains the problem, implementation, impact, linked issue, and regression coverage. It does not reproduce every template section, such as the checklist and a detailed test procedure, …
Linked Issues check ✅ Passed The changes satisfy issue #1447. ChatView now renders approval controls alongside the scroll-to-bottom action, suppresses the checkpoint shortcut when approvals are present, and adds regression covera…
Out of Scope Changes check ✅ Passed The changed files are limited to ChatView action-row behavior and its regression test. The changes directly support issue #1447 and introduce no unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Trust And Persistence Invariants ✅ Passed PASS: No changed path meets the custom failure conditions. The PR changes only ChatView.tsx rendering/layout and a regression test. Approval buttons still use the existing `disabled={!enableButtons}…
Full details: Description check

Explanation

The description explains the problem, implementation, impact, linked issue, and regression coverage. It does not reproduce every template section, such as the checklist and a detailed test procedure, but the core information is complete.

Full details: Linked Issues check

Explanation

The changes satisfy issue #1447. ChatView now renders approval controls alongside the scroll-to-bottom action, suppresses the checkpoint shortcut when approvals are present, and adds regression coverage for a scrolled child task waiting for finishTask approval.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

Full details: Regression Evidence

Explanation

The focused JSDOM regression test covers the main finishTask case: it keeps the chevron, hides the checkpoint shortcut, and preserves the approval label while browsing history (ChatView.scroll-debug-repro.spec.tsx:506-537). However, the PR also changes durable action-row layout and text truncation (ChatView.tsx:1734-1820) and adds no ChatView *.visual.tsx test or snapshot. The repository UI policy requires a Playwright snapshot for user-visible layout changes (webview-ui/AGENTS.md:44-57). The changed default tooltip fallback is also untested: the new fallback at ChatView.tsx:1768-1812 is an unset/unknown-label branch, while the regression test mocks StandardTooltip without inspecting content (ChatView.scroll-debug-repro.spec.tsx:97-103).

Resolution

Add a Playwright component visual test and committed baseline for the ChatView action row. Cover the scrolled state with the compact scroll-to-bottom control beside an approval control, checkpoint suppression, and a long localized label in a narrow layout. Add a focused Vitest test that preserves StandardTooltip content and verifies that unknown primary and secondary button labels use the button text as the tooltip fallback.

Full details: Trust And Persistence Invariants

Explanation

PASS: No changed path meets the custom failure conditions. The PR changes only ChatView.tsx rendering/layout and a regression test. Approval buttons still use the existing disabled={!enableButtons} guard and existing click handlers; the scroll and checkpoint controls do not send approval responses or bypass allowlists. All new tooltip content and labels come from localized button text, with no secret or PII source. The diff adds no persistence writes, awaits, resource allocation, or lifecycle cleanup paths.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/keep-approval-controls-visible-2rxure6hwxnz3

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

webview-ui/src/components/chat/ChatView.tsx

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsx

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 31, 2026
@edelauna
edelauna marked this pull request as ready for review August 31, 2026 02:53
@zoomote

zoomote Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Coverage follow-up shipped in 4db0139. Focused tests now cover single- and two-button approval states, both approval click handlers, checkpoint suppression, and fallback tooltip labels. Local V8 coverage confirms the previously uncovered click lines execute, and the production diff was reduced from 127 changed lines to 31 to remove indentation-only patch noise. Full tests, lint, and type checks pass. Codecov has not refreshed its bot comment yet.

@github-actions github-actions Bot added the awaiting-maintainer CodeRabbit approved; waiting for a human maintainer label Aug 31, 2026
@github-actions github-actions Bot added awaiting-maintainer CodeRabbit approved; waiting for a human maintainer and removed awaiting-maintainer CodeRabbit approved; waiting for a human maintainer labels Aug 31, 2026
@github-actions github-actions Bot removed the awaiting-maintainer CodeRabbit approved; waiting for a human maintainer label Aug 31, 2026
@edelauna
edelauna added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit dfed27d Aug 31, 2026
20 checks passed
@edelauna
edelauna deleted the fix/keep-approval-controls-visible-2rxure6hwxnz3 branch August 31, 2026 14:14
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.

[BUG] Scrolling hides pending approval controls behind the scroll-to-bottom button

3 participants