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

Scroll top improvement based on context #169038

Closed
2 of 16 tasks
rebornix opened this issue Dec 13, 2022 · 3 comments · Fixed by #193833
Closed
2 of 16 tasks

Scroll top improvement based on context #169038

rebornix opened this issue Dec 13, 2022 · 3 comments · Fixed by #193833
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders notebook-layout plan-item VS Code - planned item for upcoming under-discussion Issue is under discussion for relevance, priority, approach

Comments

@rebornix
Copy link
Member

rebornix commented Dec 13, 2022

Currently the scroll top position update is not context-aware, meaning when we have cell height changes, we don't know if it's because output height change, or users re-run a cell and outputs are updated/replaced, or users shift+enter to run cell and navigate to next cell. Users might have different expectations for each scenario but currently our heuristics for updating the scroll top didn't take the context into account.

Let's use this issue to finalize the desired behavior of scroll top update. The polished/new design should cover

  • Update element height API respects context (execution, execute+create, output update)
  • Reveal API should handle out of sync view/content height

Execute focused cell

  • Click the run button or press Ctrl+Enter to run focused cell
  • If there is no enough room/space after the cell (e.g., the focused cell is the last cell)
    • Users have editor.scrollBeyondLastLine disabled
    • Users have editor.scrollBeyondLastLine enabled
      • Reveal cell output
  • If there is enough room after the cell
    • Reveal cell output
  • If the cell input is larger than the viewport, execute cell should reveal the output properly Disable auto scrolling when executing cell. #129440

Operations on focused cell

Operations on non-focused cell

Split view

@rebornix rebornix added under-discussion Issue is under discussion for relevance, priority, approach notebook-layout labels Dec 13, 2022
@rebornix rebornix added this to the January 2023 milestone Dec 13, 2022
@roblourens
Copy link
Member

roblourens commented Dec 13, 2022

I noticed an issue recently, is this related?

  • Paste a large amount of text into a code cell
  • The notebook scrolls down so far that the focused code cell is out of the viewport

I can open a separate issue if you want

@rebornix
Copy link
Member Author

@roblourens I guess what you were seeing is #165415 , I'll update the description to include this.

@rebornix rebornix added the plan-item VS Code - planned item for upcoming label Aug 31, 2023
@Yoyokrazy
Copy link
Contributor

Couple cases discovered in the latest notebook sync. both of these are likely tied to outputLineLimit being large @ 100

  • when the shift-enter behavior is set to "firstline" the events are as follows:
    • first line of next cell is focused and shown at the bottom of the viewport
    • output is cleared and the focused cell is pulled upwards against the currently executing cell
    • output streams and pushes the focused cell out of the viewport.
    • my ideal thoughts:
      • anchor the focused cell (with setting), maintaining the first line visible. output will stream in, pushing the executing cell upwards out of the viewport
firstLine-lots-of-movement.mp4
  • shift-enter set to "fullCell" and next cell has a large output
    • focus shifts to next cell, and the viewport exposes the bottom of the output for that cell. there is no cell editor in the viewport
    • my thoughts:
      • "fullCell" behavior should show the top of the next cell editor at the top of the viewport
fullCell-focus-bottom-output.mp4

@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 Sep 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders notebook-layout plan-item VS Code - planned item for upcoming under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants