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.

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

Allow inspecting scroll position of the new tree #64749

Closed
roblourens opened this issue Dec 10, 2018 · 7 comments
Closed

Allow inspecting scroll position of the new tree #64749

roblourens opened this issue Dec 10, 2018 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality tree-widget Tree widget issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@roblourens
Copy link
Member

The settings tree uses these API from the old tree to implement syncing the visible elements with the TOC:

  • getFirstVisibleElement, onDidScroll

This is also used to handle the case when we tab out of the virtualized tree and have to manually scroll down to get to the next element:

  • getLastVisibleElement
@joaomoreno joaomoreno added this to the December 2018 milestone Dec 12, 2018
@joaomoreno joaomoreno added feature-request Request for new features or functionality tree-widget Tree widget issues labels Dec 12, 2018
@roblourens
Copy link
Member Author

I am now using the new tree with the settings editor (much more straightforward than the search view, and the dynamic sizing works great!), but a couple things won't work quite right without this.

@roblourens
Copy link
Member Author

@joaomoreno do you think you'll be able to get to this this month?

@joaomoreno joaomoreno added the verification-needed Verification of issue is requested label Feb 22, 2019
@joaomoreno
Copy link
Member

Verification: please code review.

@roblourens
Copy link
Member Author

roblourens commented Feb 22, 2019

Awesome, thanks. Is it possible to get the same behavior as the old version where it looks at how far the top element is scrolled off the page:

https://github.com/Microsoft/vscode/blob/59ba3f853d19d572d6cb3b8919d99990de338e51/src/vs/base/parts/tree/browser/treeView.ts#L668-L672

I can try it if you can tell me what's the best way to get the scroll position of an item? style.top?

@roblourens roblourens modified the milestones: February 2019, March 2019 Feb 22, 2019
@roblourens roblourens assigned roblourens and unassigned joaomoreno Feb 22, 2019
@roblourens roblourens reopened this Feb 22, 2019
@roblourens
Copy link
Member Author

this.rangeMap.positionAt(range.start) will get you the top position of that element (for firstVisibleIndex)

@joaomoreno joaomoreno removed the verification-needed Verification of issue is requested label Feb 25, 2019
@weinand weinand added the verification-needed Verification of issue is requested label Mar 26, 2019
@bpasero bpasero added the verification-steps-needed Steps to verify are needed for verification label Mar 27, 2019
@bpasero
Copy link
Member

bpasero commented Mar 27, 2019

We should not do verification through code review. Steps?

@roblourens
Copy link
Member Author

roblourens commented Mar 27, 2019

  • Scroll the settings list
  • See the highlighted TOC item as you scroll - it should be accurate and feel natural. When the top item is >= 50% scrolled out of view, then the next item is used as the current item for the TOC.

I think this is the only place these tree APIs are used.

@roblourens roblourens removed the verification-steps-needed Steps to verify are needed for verification label Mar 27, 2019
@RMacfarlane RMacfarlane added the verified Verification succeeded label Mar 27, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality tree-widget Tree widget issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants