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

Notebook scrolling out of sync #153708

Closed
jrieken opened this issue Jun 29, 2022 · 9 comments · Fixed by #158232
Closed

Notebook scrolling out of sync #153708

jrieken opened this issue Jun 29, 2022 · 9 comments · Fixed by #158232
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Jun 29, 2022

related #148164

  • have my-endgame-GHINB
  • run all cell, scroll to the bottom, and run touch <path>
  • scroll upwards
  • 🧀 notice an occasional layout shift, the alignment of code and MD cells is getting out of sync
Recording of the recording
Screen.Recording.2022-06-29.at.17.40.30.mov
Recording of the scroll
Screen.Recording.2022-06-29.at.17.37.18.mov
@jrieken
Copy link
Member Author

jrieken commented Jun 29, 2022

There will still be some layout shifts. I spent awhile trying to eliminate the scroll shift that you will see when you run touch. That turns out to be essentially impossible.

From #148164 (comment) - Unsure if this case is meant by that but if it is too hard did you consider some creative workaround, like animating the scrolling (positioning of markdown) might make it look less chunky and more "fluent"

@jrieken jrieken closed this as completed Jun 29, 2022
@jrieken jrieken reopened this Jun 29, 2022
@jrieken
Copy link
Member Author

jrieken commented Jun 29, 2022

(what the heck... it seems GH confuses the default commenting command and I keep closing/re-opening issues)

@roblourens
Copy link
Member

The first one shows an editor adding and removing the horizontal scroll bar, I have no idea why that would happen and can't repro it, any idea @rebornix?

The second is editors getting their scrollbars, we don't know whether they will overflow until they are rendered. I'm not sure whether that would be fixed by keeping view state across the model reload, like the other issue.

I'm not that motivated to work more on these specific cases because a model reload is probably not actually that common. The most common case would probably be doing a git pull with changes in that file.

@jrieken
Copy link
Member Author

jrieken commented Jun 30, 2022

I'm not that motivated to work more on these specific cases because a model reload is probably not actually that common. The most common case would probably be doing a git pull with changes in that file.

Can you explain to me why the handling of reload is different from clear all output (or an API notebook edit)?

@roblourens
Copy link
Member

You are replacing all cells with a totally different set of cells, vs editing an existing cell. Or inserting some cells with a notebook edit that are probably actually new.

@roblourens
Copy link
Member

Looking at that first gif again, clearly there is something messed up with the editor.

Regarding the editors resizing when scrolled into view, this can show up in other cases and I'd like to do something about it. We could do a better at guessing whether the editor will get a scrollbar (early on tried to do an accurate estimate of text width in the editor, turns out to be complex) or we can render editors ahead of the viewport, like we already do for outputs. I guess we only aren't doing that currently for the expense. I guess I should say that a model reset is an easy way to repro the issue, but this will show up in other more common scenarios too and we should definitely keep trying to improve.

@rebornix was also continuing a leftover investigation item this week from #148164, which would improve the case of a model reset while scrolled near the bottom of the list.

@roblourens roblourens added bug Issue identified by VS Code Team member as probable bug notebook-rendering labels Jul 8, 2022
@roblourens roblourens added this to the Backlog milestone Jul 8, 2022
@roblourens roblourens modified the milestones: Backlog, August 2022 Jul 8, 2022
@rebornix
Copy link
Member

rebornix commented Jul 8, 2022

In the first gif, the editor of the first visible code cell is always in the viewport, meaning there is no re-render while slightly scrolling up and down. However the horizontal bar hides sometimes, it feels wrong to me.

roblourens added a commit that referenced this issue Aug 23, 2022
* Fix shifting in cell editors that have horizontal scrollbars, for #153708

* use BaseCellEditorOptions to check wordwrap
@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 Aug 23, 2022
@connor4312 connor4312 added the verified Verification succeeded label Aug 25, 2022
@connor4312
Copy link
Member

When I follow the directions and run touch .vscode/notebooks/my-endgame.github-issues, I see the content jump and all outputs get cleared, which prevents verifying with the original steps. Is this a different bug or are there different steps now?

@connor4312 connor4312 added the verification-steps-needed Steps to verify are needed for verification label Aug 25, 2022
@roblourens
Copy link
Member

roblourens commented Sep 28, 2022

The outputs are going to be cleared because they aren't saved. But when you scroll up after this model reload, there should be less jumping with the markdown cells

Edit - oh, this was an old one...

@roblourens roblourens removed the verification-steps-needed Steps to verify are needed for verification label Sep 28, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants