KEMBAR78
reset scroll position core by TylerLeonhardt · Pull Request #132013 · microsoft/vscode · GitHub
Skip to content

Conversation

@TylerLeonhardt
Copy link
Member

Adds a new property to quick picks called:

resetScrollPosition: boolean

which defaults to true

that controls whether or not the cursor position will be reset to the top when either items or activeItems are changed.

This PR fixes #109969

@TylerLeonhardt TylerLeonhardt marked this pull request as ready for review August 31, 2021 23:57
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

I only looked at pickerQuickAccess.ts

const removed = items.splice(index, 1);
const activeItems = input.activeItems.filter((ai) => ai !== removed[0]);
const resetScrollPositionBefore = input.resetScrollPosition;
input.resetScrollPosition = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It still resets when you remove the item?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes because all it's doing is setting items which triggers an update which triggers the reset of the scroll position.

}

get scrollTop() {
return this.list.scrollTop;
Copy link
Member

Choose a reason for hiding this comment

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

Unsure about the list-widget but for the tree there is getViewState which captured scroll position and selection. I happily use that for the outline when switching between editors - might be an option for the list too...

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't see a getViewState equivalent in my look around the list-widget but good to know!


autoFocusOnList: boolean;

resetScrollPosition: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

For the API, I would turn this around because default values are never true

Copy link
Member Author

Choose a reason for hiding this comment

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

flipped it around and called it keepScrollPosition: boolean

@TylerLeonhardt TylerLeonhardt merged commit ad31298 into main Sep 1, 2021
@TylerLeonhardt TylerLeonhardt deleted the TylerLeonhardt/reset-scroll-position branch September 1, 2021 17:16
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quick input reset scroll position

5 participants