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

Native Notebooks - Outline selection does not go to top #123828

Closed
Tracked by #7360
claudiaregio opened this issue May 13, 2021 · 19 comments · Fixed by #150336 or #151003
Closed
Tracked by #7360

Native Notebooks - Outline selection does not go to top #123828

claudiaregio opened this issue May 13, 2021 · 19 comments · Fixed by #150336 or #151003
Assignees
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders notebook notebook-toc-outline verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@claudiaregio
Copy link

Steps to Reproduce:

  1. Open notebook
  2. Select a cell in the notebook outline

Actual Behavior:

Cell selected ends up ~middle of the window/notebook + doesn't have any selection on the cell that was picked so it's hard to identify where it is.

Expected Behavior:

Cell to be at the top + selected mode indicator

@claudiaregio
Copy link
Author

claudiaregio commented May 13, 2021

Couldn't find the original issue and thought that I forgot to file it but turns out it was closed. #123175

Received customer feedback (4/4) in interviews that they expected selecting a cell in the table of contents would bring the cell to the top and currently they have a hard time identifying where the cell is.

(I have also struggled with this same issue, when I click on a cell in the outline I find my eyes scattering to figure out where the cell actually is)

This also deviates from Jupyter behavior which is why it could be hard to adapt to

@rebornix rebornix added notebook under-discussion Issue is under discussion for relevance, priority, approach labels May 14, 2021
@rebornix
Copy link
Member

@jrieken not sure if we want a different experience in notebook.

@greazer
Copy link
Member

greazer commented Sep 13, 2021

Some thoughts here:

  • Markdown is typically used in a notebook to separate logical blocks of cells. It would seem to make sense to be able to see as much of that logical set of cells as possible when navigating to it. With a regular text editor seeing code context around the navigation point can make more sense.
  • Jupyter's behavior is to scroll markdown to the top.
  • It's unclear what would be appropriate if Show Code Cells for outline view is on and the user clicks on a piece of code in the outline view. Scrolling to the middle or top of the screen doesn't necessarily seem right. What about scrolling such that the cell that contains the piece of code is set at the top of the workbench window and the selected piece of code is highlighted temporarily? Of course we'd have to deal with large cells where the selected line appears below the bottom of the editor window. In that case, scrolling the code to the middle of the screen makes more sense (now you're more in the editor mode in this situation). Most notebooks do not contain large cells; however.

@greazer
Copy link
Member

greazer commented Sep 13, 2021

I'd also point out that the current behavior is quite buggy, which doesn't help. If you open this notebook and use breadcrumbs or the outline view to navigate the markdown, you'll easily run across them. I.e. sometimes the selected markdown is up off the top of the viewport (possibly because a cell is large below it), you'll also sometimes see headers getting rendered and then erased.

https://github.com/greazer/covid-19-data/blob/master/SampleNbsAndScripts/my-analysis-plotly.ipynb

@jrieken jrieken removed this from the September 2021 milestone Sep 24, 2021
@greazer
Copy link
Member

greazer commented Feb 20, 2022

This is still bothersome. And I also noticed that the scroll position for notebooks when clicking on one of the outline items is further down vertically than even regular code:

image

image

I still think that when any item is clicked in a notebook outline it should be scrolled to the top as it's not likely that prior code/cells will be interesting to see in context. But barring that, the scroll position should at least be identical to that of pure code.

Jupyter, Google Colab and probably others scroll the selected item to the top of the window.

@greazer greazer added notebook-toc-outline bug Issue identified by VS Code Team member as probable bug notebook-polish labels Feb 20, 2022
@claudiaregio
Copy link
Author

I know I filed this issue but just wanted to add this is actually why I don't use the TOC in the notebook. It doesn't actually help me find where I am in the notebook, my eyes end up scattering all over the page to find where I am anyway

@jrieken jrieken removed the bug Issue identified by VS Code Team member as probable bug label Feb 28, 2022
@rebornix
Copy link
Member

From all feedbacks above, there seem to be three issues:

  • Hard to find the focused/selected cell. We have added the focus indicator on the gutter since the original issue was created.
  • Cell top is revealed to the viewport center. Instead we should try to reveal the middle of the cell to the viewport center. I have pushed a fix for it, see below screen recording
  • Lastly, if we should always reveal focused/selected cell into the top of the viewport. The reasoning behind this can be that notebook cells are richer and the focused cell is less noticeable compared to active line in a text editor (especially when there are outputs in the view). It's not just TOC as you get the same behavior when clicking on cells on problems panel, debug view, etc. It makes sense to me but I'd love to wait a bit for more feedback before making this significant change.
Screen.Recording.2022-03-28.at.11.15.15.AM.mov

@claudiaregio
Copy link
Author

@rebornix Thanks for the update! I can't tell in your recording if the gutter indicator is...passive or active? (I'm probably labeling that wrong)

What I call passive (gray):
image

vs.

What I call active (blue):
image

I was really hoping to see "active" since there is so much more gray on the canvas and it is so delicate I don't end up seeing the gutter indicator, the blue has enough contrast to where I can see it clearly and quickly

@rebornix
Copy link
Member

@claudiaregio passive colors are meant to the "passive" but you can always update the color to something stands out by tweaking notebook.inactiveFocusedCellBorder

@claudiaregio
Copy link
Author

Oh perfect, thanks! I'll go ahead and update that

@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 25, 2022
@roblourens roblourens added feature-request Request for new features or functionality verification-needed Verification of issue is requested and removed under-discussion Issue is under discussion for relevance, priority, approach labels May 30, 2022
@roblourens
Copy link
Member

Verification steps - clicking a cell in the outline that is out of the view should reveal that cell at the top 5th of the viewport. If already in the viewport, it just focuses the cell. Same behavior as the text editor outline.

@lramos15 lramos15 added the verified Verification succeeded label Jun 1, 2022
@lramos15
Copy link
Member

lramos15 commented Jun 1, 2022

Recording 2022-06-01 at 12 23 03
See gif. When I select feature request missing labels I would expect it to scroll so the entire cell is in view. Only a very small sliver of the header is showing I wouldn't call that in the viewport. You can see when I scroll away it does what is expected.

@lramos15 lramos15 added verification-found Issue verification failed and removed verified Verification succeeded labels Jun 1, 2022
@lramos15 lramos15 reopened this Jun 1, 2022
@vscodenpa vscodenpa removed the insiders-released Patch has been released in VS Code Insiders label Jun 1, 2022
@roblourens
Copy link
Member

Thanks. I think I just want to compare elementBottom to wrapperBottom here:

but I will fix in June since that will affect many reveal scenarios.

@roblourens roblourens modified the milestones: May 2022, June 2022 Jun 1, 2022
@roblourens
Copy link
Member

Maybe it's not a big deal

@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 2, 2022
@lramos15 lramos15 added verified Verification succeeded and removed verification-found Issue verification failed labels Jun 2, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2022
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 insiders-released Patch has been released in VS Code Insiders notebook notebook-toc-outline verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
9 participants