Skip to content

inline chat input: clamp widget to viewport when scrolling with content #291390

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.

Sign up for GitHub

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

Merged
jrieken merged 1 commit into from
Jan 28, 2026
Merged

inline chat input: clamp widget to viewport when scrolling with content #291390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions src/vs/workbench/contrib/inlineChat/browser/inlineChatOverlayWidget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ export class InlineChatInputWidget extends Disposable {
private readonly _input: IActiveCodeEditor;
private readonly _position = observableValue<IOverlayWidgetPosition | null>(this, null);
readonly position: IObservable<IOverlayWidgetPosition | null> = this._position;
readonly minContentWidthInPx = constObservable(0);


private readonly _showStore = this._store.add(new DisposableStore());
private readonly _stickyScrollHeight: IObservable<number>;
private _inlineStartAction: IAction | undefined;
private _anchorLineNumber: number = 0;
private _anchorLeft: number = 0;
private _anchorAbove: boolean = false;

readonly allowEditorOverflow = true;

constructor(
private readonly _editorObs: ObservableCodeEditor,
Expand Down Expand Up @@ -108,6 +108,10 @@ export class InlineChatInputWidget extends Disposable {
this._input.setModel(model);
this._input.layout({ width: 200, height: 18 });

// Initialize sticky scroll height observable
const stickyScrollController = StickyScrollController.get(this._editorObs.editor);
this._stickyScrollHeight = stickyScrollController ? observableFromEvent(stickyScrollController.onDidChangeStickyScrollHeight, () => stickyScrollController.stickyScrollWidgetHeight) : constObservable(0);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The observable created with observableFromEvent should include this as the owner parameter for better lifecycle management and debugging. While the observable will still function without it, passing the owner (similar to how observableValue is used on line 52) ensures proper memory management and makes debugging easier. Change this to: observableFromEvent(this, stickyScrollController.onDidChangeStickyScrollHeight, () => stickyScrollController.stickyScrollWidgetHeight)

Suggested change
this._stickyScrollHeight = stickyScrollController ? observableFromEvent(stickyScrollController.onDidChangeStickyScrollHeight, () => stickyScrollController.stickyScrollWidgetHeight) : constObservable(0);
this._stickyScrollHeight = stickyScrollController ? observableFromEvent(this, stickyScrollController.onDidChangeStickyScrollHeight, () => stickyScrollController.stickyScrollWidgetHeight) : constObservable(0);

Copilot uses AI. Check for mistakes.

// Update placeholder based on selection state
this._store.add(autorun(r => {
const selection = this._editorObs.cursorSelection.read(r);
Expand Down Expand Up @@ -216,22 +220,23 @@ export class InlineChatInputWidget extends Disposable {
this._showStore.add(this._editorObs.createOverlayWidget({
domNode: this._domNode,
position: this._position,
minContentWidthInPx: this.minContentWidthInPx,
allowEditorOverflow: this.allowEditorOverflow,
minContentWidthInPx: constObservable(0),
allowEditorOverflow: true,
}));

// If anchoring above, adjust position after render to account for widget height
if (anchorAbove) {
this._updatePosition();
}

// Update position on scroll, hide if anchor line is out of view
// Update position on scroll, hide if anchor line is out of view (only when input is empty)
this._showStore.add(this._editorObs.editor.onDidScrollChange(() => {
const visibleRanges = this._editorObs.editor.getVisibleRanges();
const isLineVisible = visibleRanges.some(range =>
this._anchorLineNumber >= range.startLineNumber && this._anchorLineNumber <= range.endLineNumber
);
if (!isLineVisible) {
const hasContent = !!this._input.getModel().getValue();
if (!isLineVisible && !hasContent) {
this._hide();
} else {
this._updatePosition();
Expand All @@ -244,19 +249,30 @@ export class InlineChatInputWidget extends Disposable {

private _updatePosition(): void {
const editor = this._editorObs.editor;
const lineHeight = editor.getOption(EditorOption.lineHeight);
const top = editor.getTopForLineNumber(this._anchorLineNumber) - editor.getScrollTop();
let adjustedTop = top;

if (this._anchorAbove) {
const widgetHeight = this._domNode.offsetHeight;
adjustedTop = top - widgetHeight;
} else {
const lineHeight = editor.getOption(EditorOption.lineHeight);
adjustedTop = top + lineHeight;
}

// Clamp to viewport bounds when anchor line is out of view
const stickyScrollHeight = this._stickyScrollHeight.get();
const layoutInfo = editor.getLayoutInfo();
const widgetHeight = this._domNode.offsetHeight;
const minTop = stickyScrollHeight;
const maxTop = layoutInfo.height - widgetHeight;

const clampedTop = Math.max(minTop, Math.min(adjustedTop, maxTop));
const isClamped = clampedTop !== adjustedTop;
this._domNode.classList.toggle('clamped', isClamped);
Comment on lines +263 to +272
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The clamping logic is applied unconditionally in _updatePosition, including during initial positioning when the widget has no content. According to the PR description, clamping should only occur "when the input has content" to keep it visible when scrolling. However, this code will clamp the widget position even when initially showing it with no content. Consider adding a check for content (similar to line 238) before applying clamping, or pass a parameter to _updatePosition to control when clamping should be applied. This would ensure the widget is positioned normally at its anchor when first shown, and only clamped to viewport bounds when scrolling with content.

See below for a potential fix:

		// Only clamp to viewport bounds when the input has content
		const inputHasContent = Boolean(this._inputEditor.getModel()?.getValueLength());
		let topForPosition = adjustedTop;

		if (inputHasContent) {
			const stickyScrollHeight = this._stickyScrollHeight.get();
			const layoutInfo = editor.getLayoutInfo();
			const widgetHeight = this._domNode.offsetHeight;
			const minTop = stickyScrollHeight;
			const maxTop = layoutInfo.height - widgetHeight;

			const clampedTop = Math.max(minTop, Math.min(adjustedTop, maxTop));
			const isClamped = clampedTop !== adjustedTop;
			this._domNode.classList.toggle('clamped', isClamped);
			topForPosition = clampedTop;
		} else {
			this._domNode.classList.remove('clamped');
		}

		this._position.set({
			preference: { top: topForPosition, left: this._anchorLeft },

Copilot uses AI. Check for mistakes.

this._position.set({
preference: { top: adjustedTop, left: this._anchorLeft },
preference: { top: clampedTop, left: this._anchorLeft },
stackOrdinal: 10000,
}, undefined);
}
Expand Down
4 changes: 4 additions & 0 deletions src/vs/workbench/contrib/inlineChat/browser/media/inlineChatOverlayWidget.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

.inline-chat-gutter-menu.clamped {
transition: top 100ms;
}

.inline-chat-gutter-menu .input .monaco-editor-background {
background-color: var(--vscode-menu-background);
}
Expand Down
Loading