-
Notifications
You must be signed in to change notification settings - Fork 6
In Agent mode: Auto scroll to prompts like 'Continue'. #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -204,9 +204,17 @@ public void processTurnEvent(ChatProgressValue value) { | |||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| refreshScrollerLayout(); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Auto-scroll to bottom if enabled | ||||||||||||||||||||||||||||||||
| if (shouldAutoScrollToBottom()) { | ||||||||||||||||||||||||||||||||
| scrollToBottom(); | ||||||||||||||||||||||||||||||||
| // For agent-mode responses (agent rounds/tool calls) we always force the view | ||||||||||||||||||||||||||||||||
| // to scroll to the bottom so prompts that require user action (e.g. Continue, | ||||||||||||||||||||||||||||||||
| // permission dialogs) are visible even if the user previously scrolled away. | ||||||||||||||||||||||||||||||||
| if (value.getAgentRounds() != null && !value.getAgentRounds().isEmpty()) { | ||||||||||||||||||||||||||||||||
| // Use a forced scroll to ensure visibility regardless of manual scroll state. | ||||||||||||||||||||||||||||||||
| forceScrollToBottom(); | ||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||
|
Comment on lines
+207
to
+213
|
||||||||||||||||||||||||||||||||
| // Auto-scroll to bottom if enabled for regular chat-mode responses | ||||||||||||||||||||||||||||||||
| if (shouldAutoScrollToBottom()) { | ||||||||||||||||||||||||||||||||
| scrollToBottom(); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| String errMsg = value.getErrorMessage(); | ||||||||||||||||||||||||||||||||
|
|
@@ -375,12 +383,75 @@ private boolean shouldAutoScrollToBottom() { | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * Scroll to the bottom. | ||||||||||||||||||||||||||||||||
| * Made public so child widgets can request scrolling when they show dialogs or | ||||||||||||||||||||||||||||||||
| * other interactive controls that should be visible to the user. | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
| private void scrollToBottom() { | ||||||||||||||||||||||||||||||||
| ScrollBar verticalBar = this.getVerticalBar(); | ||||||||||||||||||||||||||||||||
| if (verticalBar != null) { | ||||||||||||||||||||||||||||||||
| this.setOrigin(0, verticalBar.getMaximum()); | ||||||||||||||||||||||||||||||||
| public void scrollToBottom() { | ||||||||||||||||||||||||||||||||
| // Ensure layout is settled and compute an explicit origin based on content size | ||||||||||||||||||||||||||||||||
| // rather than relying on scroll bar metrics which can be inconsistent during | ||||||||||||||||||||||||||||||||
| // rapid layout changes (dialog creation/disposal). Run on UI thread. | ||||||||||||||||||||||||||||||||
| SwtUtils.invokeOnDisplayThreadAsync(() -> { | ||||||||||||||||||||||||||||||||
| if (this.isDisposed()) { | ||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Rectangle clientArea = this.getClientArea(); | ||||||||||||||||||||||||||||||||
| // compute content height with current client width | ||||||||||||||||||||||||||||||||
| Point containerSize = cmpContent.computeSize(clientArea.width, SWT.DEFAULT); | ||||||||||||||||||||||||||||||||
| int contentHeight = containerSize.y; | ||||||||||||||||||||||||||||||||
| int originY = Math.max(0, contentHeight - clientArea.height); | ||||||||||||||||||||||||||||||||
| this.setOrigin(0, originY); | ||||||||||||||||||||||||||||||||
| }, this); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * Force the view to scroll to the bottom regardless of the user's manual scroll state. This is used for important UI | ||||||||||||||||||||||||||||||||
| * prompts (like tool confirmations) to ensure they are visible even if the user had scrolled away. The implementation | ||||||||||||||||||||||||||||||||
| * performs a two-phase scroll to be robust against layout timing issues. | ||||||||||||||||||||||||||||||||
|
Comment on lines
+408
to
+410
|
||||||||||||||||||||||||||||||||
| * Force the view to scroll to the bottom regardless of the user's manual scroll state. This is used for important UI | |
| * prompts (like tool confirmations) to ensure they are visible even if the user had scrolled away. The implementation | |
| * performs a two-phase scroll to be robust against layout timing issues. | |
| * Force the view to scroll to the bottom regardless of the user's manual | |
| * scroll state. This is used for important UI prompts (like tool | |
| * confirmations) to ensure they are visible even if the user had scrolled | |
| * away. The implementation performs a two-phase scroll to be robust against | |
| * layout timing issues. |
Copilot
AI
Apr 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forceScrollToBottom() temporarily overwrites autoScrollEnabled and restores it later via a delayed timer. This can clobber the user's scroll state if they manually scroll during the delay, and it can also restore the wrong value if forceScrollToBottom() is invoked again before the 350ms timer runs. Consider avoiding mutation of autoScrollEnabled here, or restoring conditionally (e.g., only if it wasn't changed by user input) / using a request counter so only the latest invocation restores state.
Copilot
AI
Apr 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forceScrollToBottom() schedules delayed timerExec callbacks but the 350ms callback catches a generic Exception and ignores it. This makes real SWT issues harder to diagnose and can mask failures. Since doScrollToBottom() already guards disposal, consider removing the catch entirely or at least logging unexpected exceptions (and also ensure restoration of autoScrollEnabled happens even if the timer callback is never reached).
| try { | |
| if (!this.isDisposed()) { | |
| doScrollToBottom(); | |
| } | |
| } catch (Exception ignore) { | |
| // ignore | |
| } finally { | |
| // Restore previous state after the delayed scrolls | |
| if (!this.isDisposed()) { | |
| doScrollToBottom(); | |
| } | |
| }); | |
| SwtUtils.getDisplay().timerExec(350, () -> { | |
| if (!this.isDisposed()) { | |
| // Restore previous state after the delayed scrolls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are now multiple copies of the same "walk up the parent chain to find ChatContentViewer" logic (here and in
InvokeToolConfirmationDialog.scrollToCancel). To reduce duplication and make future scrolling changes safer, consider extracting a shared helper (e.g.,SwtUtils.findParentOfType(Control, Class)or aChatContentViewer.findAncestor(Control)utility).