Interactive window- don't leave space for insert toolbar #181949
Conversation
|
@amunger I have cleaned up my previous PR you reviewed a long while ago. Could you take a look? |
| @@ -145,7 +145,7 @@ export class NotebookOptions extends Disposable { | |||
| const cellToolbarInteraction = overrides?.cellToolbarInteraction ?? this.configurationService.getValue<string>(NotebookSetting.cellToolbarVisibility); | |||
| const compactView = this.configurationService.getValue<boolean | undefined>(NotebookSetting.compactView) ?? true; | |||
| const focusIndicator = this._computeFocusIndicatorOption(); | |||
| const insertToolbarPosition = this._computeInsertToolbarPositionOption(); | |||
| const insertToolbarBetweenCells = overrides?.insertToolbarBetweenCells ?? this._computeInsertToolbarBetweenCellsOption(); | |||
There was a problem hiding this comment.
should we check if the notebook document is readonly or not?
There was a problem hiding this comment.
Yep, it is a bigger change now but it looks great. I used Show Contents on local history timeline/git timeline to see a readonly jupyter notebook.
There was a problem hiding this comment.
insertToolbarBetweenCells should be set to false here if the notebook is readonly. That would eliminate the need to pass around the readonly parameter in all those other methods.
There was a problem hiding this comment.
That information comes from the model, NotebookEditorWidget doesn't have the model in its constructor, only later when NotebookEditorWidget.setModel is called (which can be done multiple times)
Here's the call stack when using Show Contents on a git revision (creationOptions is { menuIds: {... }, cellEditorContributions: [ ...] })
Then setModel-
Then setOptions with isReadOnly set-
| bottomToolbarGap: 0, | ||
| bottomToolbarHeight: 0 | ||
| } : { | ||
| bottomToolbarGap: 12, |
There was a problem hiding this comment.
this is enough gap for the cellToolbar to have a gap between the top of the toolbar and the editor outline of the previous cell (in the case the previous cell has no output)
17434cd
to
761a1d3
Compare
Interactive window is more compact than Notebook now, by not leaving space for the insert cell toolbar(which has no items anyway in the interactive window)
This is cleaned up from #164760