Notebook canvas overflows out of the file tab and into the sidebar. #129120

Anshul7sp1 · 2021-07-21T16:31:55Z

I changed my sidebar location while having a jupyter notebook open.
It resulted in the notebook canvas overflowing onto the sidebar.
Gets fixed once I switch to some other file then come back.
This glitch appears only when there is a switch of side bar position from left to right.

Environment data

VS Code version: 1.58
Jupyter Extension version (available under the Extensions sidebar): v2021.8.1046824664
Python Extension version (available under the Extensions sidebar): v2021.7.1050252941
OS (Windows | Mac | Linux distro) and version: Windows 11
Python and/or Anaconda version: Python 3.8.2 - 64bit
Type of virtual environment used (N/A | venv | virtualenv | conda | ...): NA
Jupyter server running: Local

Actual behaviour

Initial state:

Just after I changed the layout (Move Side Bar Right)

Expected behaviour

After switching to a different file and coming back.

Steps to reproduce:

[NOTE: Self-contained, minimal reproducing code samples are extremely helpful and will expedite addressing your issue]

Create a new empty jupyter notebook
Try changing sidebar position. (only left to right switch gives this glitch, the other way is fine)

Logs

Not sure if this is relevant for this issue

vscodebot · 2021-07-21T16:33:18Z

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

abdlrhmanshehatamoussa · 2021-07-21T17:57:08Z

Hi Guys,

I'm ready to help,

If any one could give a little push on where exactly can find the relevant code and any other recommendations, I'm ready to gooo.

Best of luck !

rebornix · 2021-07-28T23:32:00Z

This is a really good catch.

rebornix · 2021-07-28T23:42:46Z

I can reproduce this issue with both notebook and custom editor. After toggling the position of the sidebar, a layout request is triggered after the sidebar moves to the right side, but no event when the activity bar moves to the right side. Thus there is a gap on the left side (whose width is the same as the activity bar)

rebornix · 2021-07-28T23:46:02Z

Apparently editor.layout is invoked before setting left/top on the editor container DOM node thus the position tracking in notebook and custom editor is measuring position incorrectly. Adding a setTimeout(() => editor.layout()) can fix the problem but can also be too costly.

rebornix · 2021-07-28T23:48:54Z

A proper fix would be we pass the left/top layout info from the grid view to the editor group view all the way down.

bpasero · 2021-08-02T07:52:15Z

This seems like a general grid layout issue to me unrelated to editors specifically. Thus assigning @joaomoreno and @sbatten (who are on vacation though).

mjbvz · 2021-08-02T19:31:14Z

Both notebooks and normal webviews suffer from this problem. I believe the root cause is the same although the two have different implementations

After debugging, I believe that the root cause is that we get a call to layout before the activity bar has moved to the other side (or before the editor has shifted fully to replace the space the activity bar occupied). Here's a screenshot of the dom state when this happens. I was shifting the activity bar from left to right:

The important bit is that left has been calculated to be 47px when it should be zero. Here's where the value is calculated:

vscode/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts

Line 1588 in 5c1b0b7

if (this._webviewTransparentCover) {

Here you can also see how we are calling layout before main the editor dom node has shifted to the left to replace the activity bar:

I believe this could be fixed by calling layout again once the activity bar (or editor locations) have been fully updated. Ideally we would also avoid calling layout until we have the final locations as well. While debugging I noticed that toggling the sidebar already seems to trigger 3 calls to layout today

bpasero · 2021-08-03T05:56:50Z

I think the sidebar location change code is here:

vscode/src/vs/workbench/browser/layout.ts

Lines 417 to 451 in 9aab65c

    
           private setSideBarPosition(position: Position): void { 
        
           	const activityBar = this.getPart(Parts.ACTIVITYBAR_PART); 
        
           	const sideBar = this.getPart(Parts.SIDEBAR_PART); 
        
           	const wasHidden = this.state.sideBar.hidden; 
        
           	const newPositionValue = (position === Position.LEFT) ? 'left' : 'right'; 
        
           	const oldPositionValue = (this.state.sideBar.position === Position.LEFT) ? 'left' : 'right'; 
        
           	this.state.sideBar.position = position; 
        
           	// Adjust CSS 
        
           	const activityBarContainer = assertIsDefined(activityBar.getContainer()); 
        
           	const sideBarContainer = assertIsDefined(sideBar.getContainer()); 
        
           	activityBarContainer.classList.remove(oldPositionValue); 
        
           	sideBarContainer.classList.remove(oldPositionValue); 
        
           	activityBarContainer.classList.add(newPositionValue); 
        
           	sideBarContainer.classList.add(newPositionValue); 
        
           	// Update Styles 
        
           	activityBar.updateStyles(); 
        
           	sideBar.updateStyles(); 
        
           	// Layout 
        
           	if (!wasHidden) { 
        
           		this.state.sideBar.width = this.workbenchGrid.getViewSize(this.sideBarPartView).width; 
        
           	} 
        
           	if (position === Position.LEFT) { 
        
           		this.workbenchGrid.moveViewTo(this.activityBarPartView, [2, 0]); 
        
           		this.workbenchGrid.moveViewTo(this.sideBarPartView, [2, 1]); 
        
           	} else { 
        
           		this.workbenchGrid.moveViewTo(this.sideBarPartView, [2, 4]); 
        
           		this.workbenchGrid.moveViewTo(this.activityBarPartView, [2, 4]); 
        
           	} 
        
           	this.layout(); 
        
           }

I resort to Steven for a fix who authored this.

sbatten · 2021-08-05T17:47:13Z

After digging into this, I believe 0449a18 is the reason we hit this issue. @joaomoreno

Essentially, if a grid gets the same dimensions and position info it will not call layout again on the contained view (in this case the webview or notebook). At the workbench level, after moving the activity bar (the 2nd action in moving the sidebar, we can see that the editor part layout is correctly called.) however, the editor part has several nested grids, one of which does not use top/left info. Since the size and position are the same on the 2nd call, the cache check is hit and we bail before the webview or notebook can get the correct offset info.

It seems this shouldn't work on the first call either but moving the sidebar triggers a size and position change on the top level grid and the activity bar does not.

joaomoreno · 2021-08-31T13:46:30Z

After digging into this, I believe 0449a18 is the reason we hit this issue. @joaomoreno

Does reverting this commit fix this issue?

sbatten · 2021-08-31T20:58:38Z

@joaomoreno yes reverting fixes the issue. I did not, however, because I was not sure of the repercussions.

sbatten · 2021-10-15T23:40:27Z

@joaomoreno there was a lot of discussion in the issue that led to the caching you introduced with 0449a18 . Unless you say so I will not revert that change for this fix since a simple reload resolves this issue.

The crux of the issue with caching however is that the notebook layout code does not treat 2 layout calls with the same dimensions as the same since they can be in a different position overall and that impacts the layout. You could say this is a bug with notebook layout, but I am not sure where we draw that line.

joaomoreno · 2021-10-25T13:05:47Z

0449a18 was pushed as a way to force all views to think about layout, since that's such a core part of performance. I don't think we should revert it and should rather fix the notebook layout.

The crux of the issue with caching however is that the notebook layout code does not treat 2 layout calls with the same dimensions as the same since they can be in a different position overall and that impacts the layout

Grid view layout calls do not only take dimension into account but positioning as well. Even 0449a18 shows it clearly. So the reason above can't be the problem, right?

joaomoreno · 2021-10-25T13:08:02Z

however, the editor part has several nested grids, one of which does not use top/left info

Hmmm just read this again. I see. Let me see what I can do about that.

joaomoreno · 2021-10-25T14:17:31Z

I've connected both grids such that views on the inner grid get absolute offsets, in respect to the outter grid.

joaomoreno · 2021-10-25T14:25:14Z

@mjbvz @rebornix This also means that this getBoundingClientRect call can just be avoided altogether... there's no reason for the positioning information not to be reaching all the way down there, now that it's propagated from both grids throughout layout calls. This would be great, since getBoundingClientRect is expensive.

The hard part of doing the above is making sure positional data gets passed around the composite world, ideally Composite.layout would look like this instead:

	abstract layout(width: number, height: number, top: number, left: number): void;

So maybe @sbatten would give you this. 😉

Handing it over to you all. ❤️

mjbvz · 2021-10-25T21:15:23Z

Thanks @joaomoreno! I confirmed the issue is fixed now for notebooks and webviews

github-actions bot assigned rchiodo Jul 21, 2021

rchiodo transferred this issue from microsoft/vscode-jupyter Jul 21, 2021

rchiodo removed their assignment Jul 21, 2021

rchiodo added notebook notebook-layout labels Jul 21, 2021

vscode-triage-bot assigned rebornix Jul 21, 2021

rebornix added the bug Issue identified by VS Code Team member as probable bug label Jul 28, 2021

rebornix added this to the July 2021 milestone Jul 28, 2021

rebornix added notebook notebook-layout and removed notebook notebook-layout labels Jul 28, 2021

rebornix assigned mjbvz Jul 28, 2021

rebornix assigned bpasero Jul 28, 2021

rebornix modified the milestones: July 2021, August 2021 Jul 28, 2021

jeanp413 mentioned this issue Jul 29, 2021

Moving sidebar to the right overlays scroll bar #129609

Closed

rebornix closed this as completed Jul 29, 2021

rebornix reopened this Jul 29, 2021

bpasero added the layout General VS Code workbench layout issues label Aug 2, 2021

bpasero assigned joaomoreno and sbatten Aug 2, 2021

bpasero removed their assignment Aug 3, 2021

sbatten modified the milestones: August 2021, September 2021 Aug 25, 2021

mjbvz modified the milestones: September 2021, October 2021 Sep 27, 2021

sbatten removed their assignment Oct 15, 2021

tanhakabir added notebook-discuss-at-standup and removed notebook-discuss-at-standup labels Oct 19, 2021

rebornix removed the notebook label Oct 21, 2021

joaomoreno closed this as completed in 910511b Oct 25, 2021

mjbvz added the verified Verification succeeded label Oct 27, 2021

github-actions bot locked and limited conversation to collaborators Dec 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notebook canvas overflows out of the file tab and into the sidebar. #129120

Notebook canvas overflows out of the file tab and into the sidebar. #129120

Anshul7sp1 commented Jul 21, 2021

vscodebot bot commented Jul 21, 2021

abdlrhmanshehatamoussa commented Jul 21, 2021

rebornix commented Jul 28, 2021

rebornix commented Jul 28, 2021

rebornix commented Jul 28, 2021

rebornix commented Jul 28, 2021 •

edited

bpasero commented Aug 2, 2021

mjbvz commented Aug 2, 2021 •

edited

bpasero commented Aug 3, 2021

sbatten commented Aug 5, 2021

joaomoreno commented Aug 31, 2021

sbatten commented Aug 31, 2021

sbatten commented Oct 15, 2021

joaomoreno commented Oct 25, 2021

joaomoreno commented Oct 25, 2021

joaomoreno commented Oct 25, 2021

joaomoreno commented Oct 25, 2021

mjbvz commented Oct 25, 2021

Notebook canvas overflows out of the file tab and into the sidebar. #129120

Notebook canvas overflows out of the file tab and into the sidebar. #129120

Comments

Anshul7sp1 commented Jul 21, 2021

Environment data

Actual behaviour

Expected behaviour

Steps to reproduce:

Logs

vscodebot bot commented Jul 21, 2021

abdlrhmanshehatamoussa commented Jul 21, 2021

rebornix commented Jul 28, 2021

rebornix commented Jul 28, 2021

rebornix commented Jul 28, 2021

rebornix commented Jul 28, 2021 • edited

bpasero commented Aug 2, 2021

mjbvz commented Aug 2, 2021 • edited

bpasero commented Aug 3, 2021

sbatten commented Aug 5, 2021

joaomoreno commented Aug 31, 2021

sbatten commented Aug 31, 2021

sbatten commented Oct 15, 2021

joaomoreno commented Oct 25, 2021

joaomoreno commented Oct 25, 2021

joaomoreno commented Oct 25, 2021

joaomoreno commented Oct 25, 2021

mjbvz commented Oct 25, 2021

rebornix commented Jul 28, 2021 •

edited

mjbvz commented Aug 2, 2021 •

edited