Skip to content
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

Jump to bottom

Making the sticky scroll keep focus when the sticky lines disappear on scrolling #177813

Merged
merged 1 commit into from
Mar 21, 2023
Merged

Making the sticky scroll keep focus when the sticky lines disappear on scrolling #177813

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 25 additions & 9 deletions src/vs/editor/contrib/stickyScroll/browser/stickyScrollController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,14 @@ export class StickyScrollController extends Disposable implements IEditorContrib
this._stickyScrollVisibleContextKey = EditorContextKeys.stickyScrollVisible.bindTo(this._contextKeyService);
const focusTracker = this._register(dom.trackFocus(this._stickyScrollWidget.getDomNode()));
this._register(focusTracker.onDidBlur(_ => {
this._disposeFocusStickyScrollStore();
const height = this._stickyScrollWidget.getDomNode().clientHeight;
if (height !== 0) {
this._disposeFocusStickyScrollStore();
} else {
// If the height is 0, then the blur has been caused by scrolling. In that case keep the focus on the sticky scroll.
this._focusedStickyElementIndex = -1;
this.focus();
}
}));
this._register(focusTracker.onDidFocus(_ => {
this.focus();
Expand Down Expand Up @@ -327,20 +334,29 @@ export class StickyScrollController extends Disposable implements IEditorContrib
this._stickyScrollWidget.setState(this._widgetState);
} else {
this._stickyElements = this._stickyScrollWidget.getDomNode().children;
if (this._stickyElements.length === 0) {
this._disposeFocusStickyScrollStore();
// Suppose that previously the sticky scroll widget had height 0, then if there are visible lines, set the last line as focused
if (this._focusedStickyElementIndex === -1) {
this._stickyScrollWidget.setState(this._widgetState);
this._focusedStickyElementIndex = this._stickyElements.length - 1;
if (this._focusedStickyElementIndex !== -1) {
(this._stickyElements.item(this._focusedStickyElementIndex) as HTMLDivElement).focus();
Copy link
Member

Choose a reason for hiding this comment

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

You could store _stickyElements as a HTMLElement[] to avoid these casts spread throughout the file:

this._stickyElements = Array.from(this._stickyScrollWidget.getDomNode().childNodes) as HTMLElement[]

You should also be able to use HTMLElement here instead of the more specific HTMLDivElement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip! The issue is that I need to access the original children HTML elements. The above code creates a copy.

}
} else {
const focusedStickyElementLineNumber = this._stickyScrollWidget.lineNumbers[this._focusedStickyElementIndex];
this._stickyScrollWidget.setState(this._widgetState);
const previousFocusedLineNumberExists = this._stickyScrollWidget.lineNumbers.includes(focusedStickyElementLineNumber);
// Suppose that after setting the state, there are no sticky lines, set the focused index to -1
if (this._stickyElements.length === 0) {
this._focusedStickyElementIndex = -1;
} else {
const previousFocusedLineNumberExists = this._stickyScrollWidget.lineNumbers.includes(focusedStickyElementLineNumber);

// If the line number is still there, do not change anything
// If the line number is not there, set the new focused line to be the last line
if (!previousFocusedLineNumberExists) {
this._focusedStickyElementIndex = this._stickyElements.length - 1;
// If the line number is still there, do not change anything
// If the line number is not there, set the new focused line to be the last line
if (!previousFocusedLineNumberExists) {
this._focusedStickyElementIndex = this._stickyElements.length - 1;
}
(this._stickyElements.item(this._focusedStickyElementIndex) as HTMLDivElement).focus();
}
(this._stickyElements.item(this._focusedStickyElementIndex) as HTMLDivElement).focus();
}
}
}
Expand Down
12 changes: 10 additions & 2 deletions src/vs/editor/contrib/stickyScroll/browser/stickyScrollWidget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,16 @@ export class StickyScrollWidget extends Disposable implements IOverlayWidget {
dom.clearNode(this._rootDomNode);
this._disposableStore.clear();
this._lineNumbers.length = 0;
this._lastLineRelativePosition = state.lastLineRelativePosition;
this._lineNumbers = state.lineNumbers;
const editorLineHeight = this._editor.getOption(EditorOption.lineHeight);
const futureWidgetHeight = state.lineNumbers.length * editorLineHeight + state.lastLineRelativePosition;

if (futureWidgetHeight > 0) {
this._lastLineRelativePosition = state.lastLineRelativePosition;
this._lineNumbers = state.lineNumbers;
} else {
this._lastLineRelativePosition = 0;
this._lineNumbers = [];
}
this._renderRootNode();
}

Expand Down