From f2474a64bd631a6272a1414f093adf81b5326151 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 30 Oct 2024 12:57:08 -0500 Subject: [PATCH 01/18] scroll on keyboard focus --- .../contrib/chat/browser/chatWidget.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 03abf59e04401..20222b0564687 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -218,6 +218,8 @@ export class ChatWidget extends Disposable implements IChatWidget { readonly viewContext: IChatWidgetViewContext; + private _activeElement: HTMLElement | undefined; + constructor( location: ChatAgentLocation | IChatWidgetLocationOptions, _viewContext: IChatWidgetViewContext | undefined, @@ -457,6 +459,36 @@ export class ChatWidget extends Disposable implements IChatWidget { this._register(this.instantiationService.createInstance(ChatDragAndDrop, this.container, this.inputPart, this.styles)); this._register((this.chatWidgetService as ChatWidgetService).register(this)); + + this._setupFocusObserver(); + } + + private _setupFocusObserver(): void { + this.listContainer.addEventListener('focus', () => { + const element = dom.getActiveElement() as HTMLElement | null; + if (this._activeElement !== element && element !== null) { + this._activeElement = element; + this._scrollToActiveElement(this._activeElement); + } + }, true); + } + + + private _scrollToActiveElement(element: HTMLElement) { + const containerRect = this.listContainer.getBoundingClientRect(); + const elementRect = element.getBoundingClientRect(); + + const topOffset = elementRect.top - containerRect.top; + // TODO: figure out why 20 is needed here, remove if possible + const bottomOffset = elementRect.bottom - containerRect.bottom + this.listContainer.clientHeight - 20; + + if (topOffset < 0) { + // Scroll up + this.tree.scrollTop += topOffset; + } else if (bottomOffset > 0) { + // Scroll down + this.tree.scrollTop += bottomOffset; + } } getContrib(id: string): T | undefined { From a4ed3c17cbe52ab6db6ae0c5fdf3bcadfb99312a Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 30 Oct 2024 14:53:41 -0500 Subject: [PATCH 02/18] don't scroll on click --- .../contrib/chat/browser/chatWidget.ts | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 20222b0564687..5b8081b6c0882 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -219,6 +219,7 @@ export class ChatWidget extends Disposable implements IChatWidget { readonly viewContext: IChatWidgetViewContext; private _activeElement: HTMLElement | undefined; + private _lastEventWasFromKeyboard: boolean = false; constructor( location: ChatAgentLocation | IChatWidgetLocationOptions, @@ -464,11 +465,25 @@ export class ChatWidget extends Disposable implements IChatWidget { } private _setupFocusObserver(): void { - this.listContainer.addEventListener('focus', () => { + // Listen for mousedown events + this.listContainer.addEventListener('mousedown', () => { + this._lastEventWasFromKeyboard = false; + }); + + // Listen for keydown events + this.listContainer.addEventListener('keydown', () => { + this._lastEventWasFromKeyboard = true; + }); + + // Listen for focus events + this.listContainer.addEventListener('focus', (e) => { const element = dom.getActiveElement() as HTMLElement | null; if (this._activeElement !== element && element !== null) { - this._activeElement = element; - this._scrollToActiveElement(this._activeElement); + // Ensure the event was from the keyboard + if (this._lastEventWasFromKeyboard) { + this._activeElement = element; + this._scrollToActiveElement(this._activeElement); + } } }, true); } From 9199f02099cc4d461fa9462b7269f07ac800c799 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 30 Oct 2024 16:26:40 -0500 Subject: [PATCH 03/18] get rid of hard coded 20 --- src/vs/workbench/contrib/chat/browser/chatWidget.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 5b8081b6c0882..2c261434d1ad0 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -494,8 +494,7 @@ export class ChatWidget extends Disposable implements IChatWidget { const elementRect = element.getBoundingClientRect(); const topOffset = elementRect.top - containerRect.top; - // TODO: figure out why 20 is needed here, remove if possible - const bottomOffset = elementRect.bottom - containerRect.bottom + this.listContainer.clientHeight - 20; + const bottomOffset = elementRect.bottom - containerRect.bottom + containerRect.height - elementRect.height; if (topOffset < 0) { // Scroll up From c775480d9f80acb22f7b7f857695b864a21a7554 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Thu, 31 Oct 2024 23:04:02 -0500 Subject: [PATCH 04/18] fix issue --- src/vs/workbench/contrib/chat/browser/chatWidget.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 2c261434d1ad0..4211cafcd7e21 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -490,18 +490,17 @@ export class ChatWidget extends Disposable implements IChatWidget { private _scrollToActiveElement(element: HTMLElement) { - const containerRect = this.listContainer.getBoundingClientRect(); + const containerRect = this.tree.getHTMLElement().getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); const topOffset = elementRect.top - containerRect.top; - const bottomOffset = elementRect.bottom - containerRect.bottom + containerRect.height - elementRect.height; if (topOffset < 0) { // Scroll up this.tree.scrollTop += topOffset; - } else if (bottomOffset > 0) { + } else if (element.offsetTop > this.listContainer.clientHeight) { // Scroll down - this.tree.scrollTop += bottomOffset; + this.tree.scrollTop += topOffset; } } From c5b852f70f27f0e6360421b1bef4f180b2fd3e81 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Thu, 31 Oct 2024 23:07:31 -0500 Subject: [PATCH 05/18] clean up --- .../contrib/chat/browser/chatWidget.ts | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 4211cafcd7e21..7d800c19b698d 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -465,25 +465,11 @@ export class ChatWidget extends Disposable implements IChatWidget { } private _setupFocusObserver(): void { - // Listen for mousedown events - this.listContainer.addEventListener('mousedown', () => { - this._lastEventWasFromKeyboard = false; - }); - - // Listen for keydown events - this.listContainer.addEventListener('keydown', () => { - this._lastEventWasFromKeyboard = true; - }); - - // Listen for focus events - this.listContainer.addEventListener('focus', (e) => { + this.listContainer.addEventListener('focus', () => { const element = dom.getActiveElement() as HTMLElement | null; if (this._activeElement !== element && element !== null) { - // Ensure the event was from the keyboard - if (this._lastEventWasFromKeyboard) { - this._activeElement = element; - this._scrollToActiveElement(this._activeElement); - } + this._activeElement = element; + this._scrollToActiveElement(this._activeElement); } }, true); } From 43555c007088be4f265dd351dfbeb8beb9fc3cd2 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Thu, 31 Oct 2024 23:08:48 -0500 Subject: [PATCH 06/18] Update src/vs/workbench/contrib/chat/browser/chatWidget.ts --- src/vs/workbench/contrib/chat/browser/chatWidget.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 7d800c19b698d..14d5d2082e6ac 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -219,7 +219,6 @@ export class ChatWidget extends Disposable implements IChatWidget { readonly viewContext: IChatWidgetViewContext; private _activeElement: HTMLElement | undefined; - private _lastEventWasFromKeyboard: boolean = false; constructor( location: ChatAgentLocation | IChatWidgetLocationOptions, From d480c9cfc204aff41517d4b878cd2ee58f2f932d Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Fri, 1 Nov 2024 11:12:05 -0500 Subject: [PATCH 07/18] fix issue --- src/vs/workbench/contrib/chat/browser/chatWidget.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 14d5d2082e6ac..5d2d6d9d4d59a 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -476,16 +476,17 @@ export class ChatWidget extends Disposable implements IChatWidget { private _scrollToActiveElement(element: HTMLElement) { const containerRect = this.tree.getHTMLElement().getBoundingClientRect(); + const fullScrollRect = this.listContainer.querySelector('.monaco-list-rows')!.getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); const topOffset = elementRect.top - containerRect.top; - + const fullBottom = fullScrollRect.bottom - elementRect.bottom; if (topOffset < 0) { // Scroll up this.tree.scrollTop += topOffset; - } else if (element.offsetTop > this.listContainer.clientHeight) { + } else if (fullBottom > 0) { // Scroll down - this.tree.scrollTop += topOffset; + this.tree.scrollTop = this.tree.scrollHeight - this.tree.renderHeight - fullBottom; } } From 0ab64228c7815a3c6b0a56b4e11db0d7ee0e9225 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Fri, 1 Nov 2024 11:12:52 -0500 Subject: [PATCH 08/18] fix issue --- src/vs/workbench/contrib/chat/browser/chatWidget.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 5d2d6d9d4d59a..f67c533333347 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -476,6 +476,8 @@ export class ChatWidget extends Disposable implements IChatWidget { private _scrollToActiveElement(element: HTMLElement) { const containerRect = this.tree.getHTMLElement().getBoundingClientRect(); + // the list container does not overflow below, so we have to use the monaco list rows to get the + // relative position below const fullScrollRect = this.listContainer.querySelector('.monaco-list-rows')!.getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); From d432b45d729c2e6e72367be6b0422cd36125f514 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Fri, 1 Nov 2024 16:17:41 -0500 Subject: [PATCH 09/18] use observer --- .../contrib/chat/browser/chatWidget.ts | 73 +++++++++++++++++-- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index f67c533333347..61abe744994c5 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -173,6 +173,8 @@ export class ChatWidget extends Disposable implements IChatWidget { */ private scrollLock = true; + private _isScrolling = false; + private readonly viewModelDisposables = this._register(new DisposableStore()); private _viewModel: ChatViewModel | undefined; private set viewModel(viewModel: ChatViewModel | undefined) { @@ -459,30 +461,82 @@ export class ChatWidget extends Disposable implements IChatWidget { this._register(this.instantiationService.createInstance(ChatDragAndDrop, this.container, this.inputPart, this.styles)); this._register((this.chatWidgetService as ChatWidgetService).register(this)); - - this._setupFocusObserver(); } private _setupFocusObserver(): void { + let isObserverEnabled = true; + this._isScrolling = false; + + // Define the callback function for the Intersection Observer + const callback = (entries: IntersectionObserverEntry[]) => { + entries.forEach(entry => { + if (!entry.isIntersecting && !this._isScrolling) { + // If the element goes out of view, scroll to it + // This prevents recursion + this._isScrolling = true; + this._scrollToActiveElement(entry.target as HTMLElement); + } + }); + }; + + const observer = new IntersectionObserver(callback, { + root: this.listContainer.querySelector('.monaco-scrollable-element'), + threshold: 0, + }); + + const observeActiveElement = () => { + // Enable observer only for the active element if it is not already observed + if (this._activeElement && isObserverEnabled) { + observer.observe(this._activeElement); + } + }; + + // Disconnect observer on mousedown or scroll to avoid interference + const disableObserver = () => { + if (isObserverEnabled) { + observer.disconnect(); + isObserverEnabled = false; + } + }; + + // Re-enable observer on keyboard interaction only if disabled + const enableObserverOnKeydown = () => { + if (!isObserverEnabled) { + isObserverEnabled = true; + observeActiveElement(); + } + }; + + this.listContainer.addEventListener('scroll', disableObserver); + this.listContainer.addEventListener('mousedown', disableObserver); + dom.getActiveWindow().addEventListener('keydown', enableObserverOnKeydown); + + // Listen for changes in the active (focused) element this.listContainer.addEventListener('focus', () => { const element = dom.getActiveElement() as HTMLElement | null; if (this._activeElement !== element && element !== null) { + // Unobserve the previous element, if any + if (this._activeElement) { + observer.unobserve(this._activeElement); + } + + // Update the active element and observe it if the observer is enabled this._activeElement = element; - this._scrollToActiveElement(this._activeElement); + if (isObserverEnabled) { + observeActiveElement(); + } } }, true); } - private _scrollToActiveElement(element: HTMLElement) { const containerRect = this.tree.getHTMLElement().getBoundingClientRect(); - // the list container does not overflow below, so we have to use the monaco list rows to get the - // relative position below const fullScrollRect = this.listContainer.querySelector('.monaco-list-rows')!.getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); const topOffset = elementRect.top - containerRect.top; const fullBottom = fullScrollRect.bottom - elementRect.bottom; + if (topOffset < 0) { // Scroll up this.tree.scrollTop += topOffset; @@ -490,8 +544,14 @@ export class ChatWidget extends Disposable implements IChatWidget { // Scroll down this.tree.scrollTop = this.tree.scrollHeight - this.tree.renderHeight - fullBottom; } + + // Allow observer to work again after the scroll is complete + setTimeout(() => { + this._isScrolling = false; + }, 100); } + getContrib(id: string): T | undefined { return this.contribs.find(c => c.id === id) as T; } @@ -589,6 +649,7 @@ export class ChatWidget extends Disposable implements IChatWidget { } else { this.renderFollowups(undefined); } + this._setupFocusObserver(); } } From 776ad58cb53862904228d38e00cc68efea8b4d79 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 30 Oct 2024 12:57:08 -0500 Subject: [PATCH 10/18] scroll on keyboard focus --- .../contrib/chat/browser/chatWidget.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index dc83974397044..e8ba5f8d6cd74 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -211,6 +211,8 @@ export class ChatWidget extends Disposable implements IChatWidget { readonly viewContext: IChatWidgetViewContext; + private _activeElement: HTMLElement | undefined; + constructor( location: ChatAgentLocation | IChatWidgetLocationOptions, _viewContext: IChatWidgetViewContext | undefined, @@ -453,6 +455,36 @@ export class ChatWidget extends Disposable implements IChatWidget { }).filter(isDefined); this._register((this.chatWidgetService as ChatWidgetService).register(this)); + + this._setupFocusObserver(); + } + + private _setupFocusObserver(): void { + this.listContainer.addEventListener('focus', () => { + const element = dom.getActiveElement() as HTMLElement | null; + if (this._activeElement !== element && element !== null) { + this._activeElement = element; + this._scrollToActiveElement(this._activeElement); + } + }, true); + } + + + private _scrollToActiveElement(element: HTMLElement) { + const containerRect = this.listContainer.getBoundingClientRect(); + const elementRect = element.getBoundingClientRect(); + + const topOffset = elementRect.top - containerRect.top; + // TODO: figure out why 20 is needed here, remove if possible + const bottomOffset = elementRect.bottom - containerRect.bottom + this.listContainer.clientHeight - 20; + + if (topOffset < 0) { + // Scroll up + this.tree.scrollTop += topOffset; + } else if (bottomOffset > 0) { + // Scroll down + this.tree.scrollTop += bottomOffset; + } } private scrollToEnd() { From d6057671883ead79b2ee807ab5fc396474b1c5aa Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 8 Jan 2025 13:01:23 -0600 Subject: [PATCH 11/18] fix issues --- src/vs/base/browser/ui/list/listView.ts | 7 +- .../contrib/chat/browser/chatWidget.ts | 77 ++----------------- 2 files changed, 11 insertions(+), 73 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index e305b7de4b9a6..a0d6c4c55f30c 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -441,7 +441,12 @@ export class ListView implements IListView { // Prevent the monaco-scrollable-element from scrolling // https://github.com/microsoft/vscode/issues/44181 - this.disposables.add(addDisposableListener(this.scrollableElement.getDomNode(), 'scroll', e => (e.target as HTMLElement).scrollTop = 0)); + this.disposables.add(addDisposableListener(this.scrollableElement.getDomNode(), 'scroll', e => { + const element = (e.target as HTMLElement); + const scrollValue = element.scrollTop; + element.scrollTop = 0; + this.setScrollTop(this.scrollTop + scrollValue); + })); this.disposables.add(addDisposableListener(this.domNode, 'dragover', e => this.onDragOver(this.toDragEvent(e)))); this.disposables.add(addDisposableListener(this.domNode, 'drop', e => this.onDrop(this.toDragEvent(e)))); diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index a9c46059bd7f0..2867764d95277 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -166,8 +166,6 @@ export class ChatWidget extends Disposable implements IChatWidget { */ private scrollLock = true; - private _isScrolling = false; - private readonly viewModelDisposables = this._register(new DisposableStore()); private _viewModel: ChatViewModel | undefined; private set viewModel(viewModel: ChatViewModel | undefined) { @@ -457,94 +455,31 @@ export class ChatWidget extends Disposable implements IChatWidget { }).filter(isDefined); this._register((this.chatWidgetService as ChatWidgetService).register(this)); + + this._setupFocusObserver(); } private _setupFocusObserver(): void { - let isObserverEnabled = true; - this._isScrolling = false; - - // Define the callback function for the Intersection Observer - const callback = (entries: IntersectionObserverEntry[]) => { - entries.forEach(entry => { - if (!entry.isIntersecting && !this._isScrolling) { - // If the element goes out of view, scroll to it - // This prevents recursion - this._isScrolling = true; - this._scrollToActiveElement(entry.target as HTMLElement); - } - }); - }; - - const observer = new IntersectionObserver(callback, { - root: this.listContainer.querySelector('.monaco-scrollable-element'), - threshold: 0, - }); - - const observeActiveElement = () => { - // Enable observer only for the active element if it is not already observed - if (this._activeElement && isObserverEnabled) { - observer.observe(this._activeElement); - } - }; - - // Disconnect observer on mousedown or scroll to avoid interference - const disableObserver = () => { - if (isObserverEnabled) { - observer.disconnect(); - isObserverEnabled = false; - } - }; - - // Re-enable observer on keyboard interaction only if disabled - const enableObserverOnKeydown = () => { - if (!isObserverEnabled) { - isObserverEnabled = true; - observeActiveElement(); - } - }; - - this.listContainer.addEventListener('scroll', disableObserver); - this.listContainer.addEventListener('mousedown', disableObserver); - dom.getActiveWindow().addEventListener('keydown', enableObserverOnKeydown); - - // Listen for changes in the active (focused) element this.listContainer.addEventListener('focus', () => { const element = dom.getActiveElement() as HTMLElement | null; if (this._activeElement !== element && element !== null) { - // Unobserve the previous element, if any - if (this._activeElement) { - observer.unobserve(this._activeElement); - } - - // Update the active element and observe it if the observer is enabled this._activeElement = element; - if (isObserverEnabled) { - observeActiveElement(); - } + this._scrollToActiveElement(this._activeElement); } }, true); } + private _scrollToActiveElement(element: HTMLElement) { - const containerRect = this.tree.getHTMLElement().getBoundingClientRect(); - const fullScrollRect = this.listContainer.querySelector('.monaco-list-rows')!.getBoundingClientRect(); + const containerRect = this.listContainer.getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); const topOffset = elementRect.top - containerRect.top; - const fullBottom = fullScrollRect.bottom - elementRect.bottom; if (topOffset < 0) { // Scroll up this.tree.scrollTop += topOffset; - } else if (fullBottom > 0) { - // Scroll down - this.tree.scrollTop = this.tree.scrollHeight - this.tree.renderHeight - fullBottom; } - - // Allow observer to work again after the scroll is complete - setTimeout(() => { - this._isScrolling = false; - }, 100); } private scrollToEnd() { @@ -554,7 +489,6 @@ export class ChatWidget extends Disposable implements IChatWidget { } } - getContrib(id: string): T | undefined { return this.contribs.find(c => c.id === id) as T; } @@ -650,7 +584,6 @@ export class ChatWidget extends Disposable implements IChatWidget { } else { this.renderFollowups(undefined); } - this._setupFocusObserver(); } } From d90707e9738b5b26efd8886037bf48aef10f445c Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 30 Oct 2024 12:57:08 -0500 Subject: [PATCH 12/18] scroll on keyboard focus --- .../contrib/chat/browser/chatWidget.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index dc83974397044..e8ba5f8d6cd74 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -211,6 +211,8 @@ export class ChatWidget extends Disposable implements IChatWidget { readonly viewContext: IChatWidgetViewContext; + private _activeElement: HTMLElement | undefined; + constructor( location: ChatAgentLocation | IChatWidgetLocationOptions, _viewContext: IChatWidgetViewContext | undefined, @@ -453,6 +455,36 @@ export class ChatWidget extends Disposable implements IChatWidget { }).filter(isDefined); this._register((this.chatWidgetService as ChatWidgetService).register(this)); + + this._setupFocusObserver(); + } + + private _setupFocusObserver(): void { + this.listContainer.addEventListener('focus', () => { + const element = dom.getActiveElement() as HTMLElement | null; + if (this._activeElement !== element && element !== null) { + this._activeElement = element; + this._scrollToActiveElement(this._activeElement); + } + }, true); + } + + + private _scrollToActiveElement(element: HTMLElement) { + const containerRect = this.listContainer.getBoundingClientRect(); + const elementRect = element.getBoundingClientRect(); + + const topOffset = elementRect.top - containerRect.top; + // TODO: figure out why 20 is needed here, remove if possible + const bottomOffset = elementRect.bottom - containerRect.bottom + this.listContainer.clientHeight - 20; + + if (topOffset < 0) { + // Scroll up + this.tree.scrollTop += topOffset; + } else if (bottomOffset > 0) { + // Scroll down + this.tree.scrollTop += bottomOffset; + } } private scrollToEnd() { From 05c8e1d7291ffa05ddd4cddcc9bfb87df903bab4 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 8 Jan 2025 16:15:32 -0600 Subject: [PATCH 13/18] move code into list view --- src/vs/base/browser/ui/list/listView.ts | 27 ++++++++++++++++++- .../contrib/chat/browser/chatWidget.ts | 25 ----------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index a0d6c4c55f30c..4ab5fd53034ce 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { DataTransfers, IDragAndDropData } from '../../dnd.js'; -import { $, addDisposableListener, animate, Dimension, getContentHeight, getContentWidth, getDocument, getTopLeftOffset, getWindow, isAncestor, isHTMLElement, isSVGElement, scheduleAtNextAnimationFrame } from '../../dom.js'; +import { $, addDisposableListener, animate, Dimension, getActiveElement, getContentHeight, getContentWidth, getDocument, getTopLeftOffset, getWindow, isAncestor, isHTMLElement, isSVGElement, scheduleAtNextAnimationFrame } from '../../dom.js'; import { DomEmitter } from '../../event.js'; import { IMouseWheelEvent } from '../../mouseEvent.js'; import { EventType as TouchEventType, Gesture, GestureEvent } from '../../touch.js'; @@ -324,6 +324,7 @@ export class ListView implements IListView { private onDragLeaveTimeout: IDisposable = Disposable.None; private currentSelectionDisposable: IDisposable = Disposable.None; private currentSelectionBounds: IRange | undefined; + private activeElement: HTMLElement | undefined; private readonly disposables: DisposableStore = new DisposableStore(); @@ -465,6 +466,30 @@ export class ListView implements IListView { this.dnd = options.dnd ?? this.disposables.add(DefaultOptions.dnd); this.layout(options.initialSize?.height, options.initialSize?.width); + this._setupFocusObserver(container); + } + + private _setupFocusObserver(container: HTMLElement): void { + container.addEventListener('focus', () => { + const element = getActiveElement() as HTMLElement | null; + if (this.activeElement !== element && element !== null) { + this.activeElement = element; + this._scrollToActiveElement(this.activeElement, container); + } + }, true); + } + + + private _scrollToActiveElement(element: HTMLElement, container: HTMLElement) { + const containerRect = container.getBoundingClientRect(); + const elementRect = element.getBoundingClientRect(); + + const topOffset = elementRect.top - containerRect.top; + + if (topOffset < 0) { + // Scroll up + this.setScrollTop(this.scrollTop + topOffset); + } } updateOptions(options: IListViewOptionsUpdate) { diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index 2867764d95277..e88650da1a4e4 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -211,8 +211,6 @@ export class ChatWidget extends Disposable implements IChatWidget { readonly viewContext: IChatWidgetViewContext; - private _activeElement: HTMLElement | undefined; - constructor( location: ChatAgentLocation | IChatWidgetLocationOptions, _viewContext: IChatWidgetViewContext | undefined, @@ -455,32 +453,9 @@ export class ChatWidget extends Disposable implements IChatWidget { }).filter(isDefined); this._register((this.chatWidgetService as ChatWidgetService).register(this)); - - this._setupFocusObserver(); - } - - private _setupFocusObserver(): void { - this.listContainer.addEventListener('focus', () => { - const element = dom.getActiveElement() as HTMLElement | null; - if (this._activeElement !== element && element !== null) { - this._activeElement = element; - this._scrollToActiveElement(this._activeElement); - } - }, true); } - private _scrollToActiveElement(element: HTMLElement) { - const containerRect = this.listContainer.getBoundingClientRect(); - const elementRect = element.getBoundingClientRect(); - - const topOffset = elementRect.top - containerRect.top; - - if (topOffset < 0) { - // Scroll up - this.tree.scrollTop += topOffset; - } - } private scrollToEnd() { if (this.lastItem) { From a77f16a720abcff3d575003ea82834305191e55a Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 8 Jan 2025 16:18:25 -0600 Subject: [PATCH 14/18] Update src/vs/workbench/contrib/chat/browser/chatWidget.ts --- src/vs/workbench/contrib/chat/browser/chatWidget.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index e88650da1a4e4..dc83974397044 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -455,8 +455,6 @@ export class ChatWidget extends Disposable implements IChatWidget { this._register((this.chatWidgetService as ChatWidgetService).register(this)); } - - private scrollToEnd() { if (this.lastItem) { const offset = Math.max(this.lastItem.currentRenderedHeight ?? 0, 1e6); From 8e926167a5fa573c6fd37b9bdd0b6895f7662661 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Wed, 8 Jan 2025 16:20:08 -0600 Subject: [PATCH 15/18] move a line up --- src/vs/base/browser/ui/list/listView.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index 4ab5fd53034ce..5d166e2b6ff80 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -479,7 +479,6 @@ export class ListView implements IListView { }, true); } - private _scrollToActiveElement(element: HTMLElement, container: HTMLElement) { const containerRect = container.getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); From 35e6fd8fdb048d5a4094f6290c35f03da99bd085 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Thu, 9 Jan 2025 15:39:43 -0600 Subject: [PATCH 16/18] add comments --- src/vs/base/browser/ui/list/listView.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index 5d166e2b6ff80..214f21257986b 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -440,9 +440,8 @@ export class ListView implements IListView { this.scrollableElement.onScroll(this.onScroll, this, this.disposables); this.disposables.add(addDisposableListener(this.rowsContainer, TouchEventType.Change, e => this.onTouchChange(e as GestureEvent))); - // Prevent the monaco-scrollable-element from scrolling - // https://github.com/microsoft/vscode/issues/44181 this.disposables.add(addDisposableListener(this.scrollableElement.getDomNode(), 'scroll', e => { + // Make sure the active element is scrolled into view const element = (e.target as HTMLElement); const scrollValue = element.scrollTop; element.scrollTop = 0; @@ -480,6 +479,8 @@ export class ListView implements IListView { } private _scrollToActiveElement(element: HTMLElement, container: HTMLElement) { + // The scroll event on the list only fires when scrolling down. + // If the active element is above the viewport, we need to scroll up. microsoft/vscode-copilot#9357 const containerRect = container.getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); From babc3795c75ef505c0cc863f0afde45b5e8a0719 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Fri, 10 Jan 2025 16:32:32 -0600 Subject: [PATCH 17/18] rm comment about issue --- src/vs/base/browser/ui/list/listView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index 214f21257986b..211d1a909c445 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -480,7 +480,7 @@ export class ListView implements IListView { private _scrollToActiveElement(element: HTMLElement, container: HTMLElement) { // The scroll event on the list only fires when scrolling down. - // If the active element is above the viewport, we need to scroll up. microsoft/vscode-copilot#9357 + // If the active element is above the viewport, we need to scroll up. const containerRect = container.getBoundingClientRect(); const elementRect = element.getBoundingClientRect(); From b36b37f3c2647fc881087f0d3c756a38b45753d0 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Fri, 10 Jan 2025 16:35:59 -0600 Subject: [PATCH 18/18] add disposable listener --- src/vs/base/browser/ui/list/listView.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index 211d1a909c445..4e065bf3539d9 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -469,18 +469,18 @@ export class ListView implements IListView { } private _setupFocusObserver(container: HTMLElement): void { - container.addEventListener('focus', () => { + this.disposables.add(addDisposableListener(container, 'focus', () => { const element = getActiveElement() as HTMLElement | null; if (this.activeElement !== element && element !== null) { this.activeElement = element; this._scrollToActiveElement(this.activeElement, container); } - }, true); + }, true)); } private _scrollToActiveElement(element: HTMLElement, container: HTMLElement) { // The scroll event on the list only fires when scrolling down. - // If the active element is above the viewport, we need to scroll up. + // If the active element is above the viewport, we need to scroll up. const containerRect = container.getBoundingClientRect(); const elementRect = element.getBoundingClientRect();