Skip to content

Commit

Permalink
Disable clear all outputs in notebook main toolbar (#13569)
Browse files Browse the repository at this point in the history 
* reenable initial reading of execution summary

Signed-off-by: Jonah Iden <[email protected]>

* disabling clear all outputs command in notebook toolbar
when no outputs are available

Signed-off-by: Jonah Iden <[email protected]>

* fixed build

Signed-off-by: Jonah Iden <[email protected]>

* formatting

Co-authored-by: Mark Sujew <[email protected]>

---------

Signed-off-by: Jonah Iden <[email protected]>
Co-authored-by: Mark Sujew <[email protected]>
  • Loading branch information
@jonah-iden @msujew
jonah-iden and msujew authored Apr 5, 2024
1 parent 52ab029 commit bce99c2
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 28 deletions.
18 changes: 18 additions & 0 deletions packages/core/src/common/event.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,21 @@ export class AsyncEmitter<T extends WaitUntilEvent> extends Emitter<T> {
}

}

export class QueueableEmitter<T> extends Emitter<T[]> {

currentQueue?: T[];

queue(...arg: T[]): void {
if (!this.currentQueue) {
this.currentQueue = [];
}
this.currentQueue.push(...arg);
}

override fire(): void {
super.fire(this.currentQueue || []);
this.currentQueue = undefined;
}

}
10 changes: 7 additions & 3 deletions packages/notebook/src/browser/contributions/notebook-actions-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { NotebookKernelQuickPickService } from '../service/notebook-kernel-quick
import { NotebookExecutionService } from '../service/notebook-execution-service';
import { NotebookEditorWidget } from '../notebook-editor-widget';
import { NotebookEditorWidgetService } from '../service/notebook-editor-widget-service';
import { NOTEBOOK_CELL_FOCUSED, NOTEBOOK_EDITOR_FOCUSED } from './notebook-context-keys';
import { NOTEBOOK_CELL_FOCUSED, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_HAS_OUTPUTS } from './notebook-context-keys';

export namespace NotebookCommands {
export const ADD_NEW_CELL_COMMAND = Command.toDefaultLocalizedCommand({
Expand Down Expand Up @@ -141,7 +141,10 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon
));

commands.registerCommand(NotebookCommands.CLEAR_ALL_OUTPUTS_COMMAND, this.editableCommandHandler(
notebookModel => notebookModel.cells.forEach(cell => cell.spliceNotebookCellOutputs({ start: 0, deleteCount: cell.outputs.length, newOutputs: [] }))
notebookModel => notebookModel.applyEdits(notebookModel.cells.map(cell => ({
editType: CellEditType.Output,
handle: cell.handle, deleteCount: cell.outputs.length, outputs: []
})), false)
));

commands.registerCommand(NotebookCommands.CHANGE_SELECTED_CELL,
Expand Down Expand Up @@ -217,7 +220,8 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon
commandId: NotebookCommands.CLEAR_ALL_OUTPUTS_COMMAND.id,
label: nls.localizeByDefault('Clear All Outputs'),
icon: codicon('clear-all'),
order: '30'
order: '30',
when: NOTEBOOK_HAS_OUTPUTS
});
// other items
}
Expand Down
8 changes: 7 additions & 1 deletion packages/notebook/src/browser/contributions/notebook-cell-actions-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,13 @@ export class NotebookCellActionContribution implements MenuContribution, Command
}
});
commands.registerCommand(NotebookCellCommands.CLEAR_OUTPUTS_COMMAND, this.editableCellCommandHandler(
(_, cell) => cell.spliceNotebookCellOutputs({ start: 0, deleteCount: (cell ?? this.getSelectedCell()).outputs.length, newOutputs: [] })
(notebook, cell) => notebook.applyEdits([{
editType: CellEditType.Output,
handle: cell.handle,
outputs: [],
deleteCount: cell.outputs.length,
append: false
}], true)
));
commands.registerCommand(NotebookCellCommands.CHANGE_OUTPUT_PRESENTATION_COMMAND, this.editableCellCommandHandler(
(_, __, output) => output?.requestOutputPresentationUpdate()
Expand Down
2 changes: 2 additions & 0 deletions packages/notebook/src/browser/notebook-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,15 @@ export interface CellOutputEdit {
editType: CellEditType.Output;
index: number;
outputs: CellOutput[];
deleteCount?: number;
append?: boolean;
}

export interface CellOutputEditByHandle {
editType: CellEditType.Output;
handle: number;
outputs: CellOutput[];
deleteCount?: number;
append?: boolean;
}

Expand Down
13 changes: 11 additions & 2 deletions packages/notebook/src/browser/service/notebook-context-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import {
NOTEBOOK_CELL_EDITABLE,
NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE,
NOTEBOOK_CELL_FOCUSED, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE,
NOTEBOOK_CELL_TYPE, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_SELECTED,
NOTEBOOK_CELL_TYPE, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_SELECTED,
NOTEBOOK_VIEW_TYPE
} from '../contributions/notebook-context-keys';
import { NotebookEditorWidget } from '../notebook-editor-widget';
import { NotebookCellModel } from '../view-model/notebook-cell-model';
import { CellKind } from '../../common';
import { CellKind, NotebookCellsChangeType } from '../../common';
import { NotebookExecutionStateService } from './notebook-execution-state-service';

@injectable()
Expand Down Expand Up @@ -73,6 +73,15 @@ export class NotebookContextManager {
}
}));

widget.model?.onDidChangeContent(events => {
if (events.some(e => e.kind === NotebookCellsChangeType.ModelChange || e.kind === NotebookCellsChangeType.Output)) {
this.scopedStore.setContext(NOTEBOOK_HAS_OUTPUTS, widget.model?.cells.some(cell => cell.outputs.length > 0));
this.onDidChangeContextEmitter.fire(this.createContextKeyChangedEvent([NOTEBOOK_HAS_OUTPUTS]));
}
});

this.scopedStore.setContext(NOTEBOOK_HAS_OUTPUTS, !!widget.model?.cells.find(cell => cell.outputs.length > 0));

// Cell Selection realted keys
this.scopedStore.setContext(NOTEBOOK_CELL_FOCUSED, !!widget.model?.selectedCell);
widget.model?.onDidChangeSelectedCell(e => {
Expand Down
5 changes: 3 additions & 2 deletions packages/notebook/src/browser/style/index.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,9 @@
cursor: pointer;
}

.theia-notebook-main-toolbar-item:hover {
background-color: var(--theia-toolbar-hoverBackground);
.theia-notebook-main-toolbar-item.theia-mod-disabled:hover {
background-color: transparent;
cursor: default;
}

.theia-notebook-main-toolbar-item-text {
Expand Down
30 changes: 18 additions & 12 deletions packages/notebook/src/browser/view-model/notebook-model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { Disposable, Emitter, Event, Resource, URI } from '@theia/core';
import { Disposable, Emitter, Event, QueueableEmitter, Resource, URI } from '@theia/core';
import { Saveable, SaveOptions } from '@theia/core/lib/browser';
import {
CellData, CellEditType, CellUri, NotebookCellInternalMetadata,
Expand Down Expand Up @@ -65,7 +65,7 @@ export class NotebookModel implements Saveable, Disposable {
protected readonly onDidAddOrRemoveCellEmitter = new Emitter<NotebookModelWillAddRemoveEvent>();
readonly onDidAddOrRemoveCell = this.onDidAddOrRemoveCellEmitter.event;

protected readonly onDidChangeContentEmitter = new Emitter<NotebookContentChangedEvent[]>();
protected readonly onDidChangeContentEmitter = new QueueableEmitter<NotebookContentChangedEvent>();
readonly onDidChangeContent = this.onDidChangeContentEmitter.event;

protected readonly onDidChangeSelectedCellEmitter = new Emitter<NotebookCellModel | undefined>();
Expand Down Expand Up @@ -284,13 +284,18 @@ export class NotebookModel implements Saveable, Disposable {
} else {
// could definitely be more efficient. See vscode __spliceNotebookCellOutputs2
// For now, just replace the whole existing output with the new output
cell.spliceNotebookCellOutputs({ start: 0, deleteCount: cell.outputs.length, newOutputs: edit.outputs });
cell.spliceNotebookCellOutputs({ start: 0, deleteCount: edit.deleteCount ?? cell.outputs.length, newOutputs: edit.outputs });
}

this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.Output, index: cellIndex, outputs: cell.outputs, append: edit.append ?? false });
break;
}
case CellEditType.OutputItems:
cell.changeOutputItems(edit.outputId, !!edit.append, edit.items);
this.onDidChangeContentEmitter.queue({
kind: NotebookCellsChangeType.OutputItem, index: cellIndex, outputItems: edit.items,
outputId: edit.outputId, append: edit.append ?? false
});

break;
case CellEditType.Metadata:
this.changeCellMetadata(this.cells[cellIndex], edit.metadata, computeUndoRedo);
Expand All @@ -311,12 +316,15 @@ export class NotebookModel implements Saveable, Disposable {
this.moveCellToIndex(cellIndex, edit.length, edit.newIdx, computeUndoRedo);
break;
}

// if selected cell is affected update it because it can potentially have been replaced
if (cell === this.selectedCell) {
this.setSelectedCell(this.cells[cellIndex]);
}
}

this.onDidChangeContentEmitter.fire();

}

protected async replaceCells(start: number, deleteCount: number, newCells: CellData[], computeUndoRedo: boolean): Promise<void> {
Expand Down Expand Up @@ -355,7 +363,7 @@ export class NotebookModel implements Saveable, Disposable {
await Promise.all(cells.map(cell => cell.resolveTextModel()));

this.onDidAddOrRemoveCellEmitter.fire({ rawEvent: { kind: NotebookCellsChangeType.ModelChange, changes }, newCellIds: cells.map(cell => cell.handle) });
this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ModelChange, changes }]);
this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ModelChange, changes });
if (cells.length > 0) {
this.setSelectedCell(cells[cells.length - 1]);
cells[cells.length - 1].requestEdit();
Expand All @@ -373,9 +381,7 @@ export class NotebookModel implements Saveable, Disposable {
}

cell.internalMetadata = newInternalMetadata;
this.onDidChangeContentEmitter.fire([
{ kind: NotebookCellsChangeType.ChangeCellInternalMetadata, index: this.cells.indexOf(cell), internalMetadata: newInternalMetadata }
]);
this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeCellInternalMetadata, index: this.cells.indexOf(cell), internalMetadata: newInternalMetadata });
}

protected updateNotebookMetadata(metadata: NotebookDocumentMetadata, computeUndoRedo: boolean): void {
Expand All @@ -388,7 +394,7 @@ export class NotebookModel implements Saveable, Disposable {
}

this.metadata = metadata;
this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ChangeDocumentMetadata, metadata: this.metadata }]);
this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeDocumentMetadata, metadata: this.metadata });
}

protected changeCellMetadataPartial(cell: NotebookCellModel, metadata: NullablePartialNotebookCellMetadata, computeUndoRedo: boolean): void {
Expand Down Expand Up @@ -420,7 +426,7 @@ export class NotebookModel implements Saveable, Disposable {
}

cell.metadata = metadata;
this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ChangeCellMetadata, index: this.cells.indexOf(cell), metadata: cell.metadata }]);
this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeCellMetadata, index: this.cells.indexOf(cell), metadata: cell.metadata });
}

protected changeCellLanguage(cell: NotebookCellModel, languageId: string, computeUndoRedo: boolean): void {
Expand All @@ -430,7 +436,7 @@ export class NotebookModel implements Saveable, Disposable {

cell.language = languageId;

this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.ChangeCellLanguage, index: this.cells.indexOf(cell), language: languageId }]);
this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ChangeCellLanguage, index: this.cells.indexOf(cell), language: languageId });
}

protected moveCellToIndex(fromIndex: number, length: number, toIndex: number, computeUndoRedo: boolean): boolean {
Expand All @@ -443,7 +449,7 @@ export class NotebookModel implements Saveable, Disposable {

const cells = this.cells.splice(fromIndex, length);
this.cells.splice(toIndex, 0, ...cells);
this.onDidChangeContentEmitter.fire([{ kind: NotebookCellsChangeType.Move, index: fromIndex, length, newIdx: toIndex, cells }]);
this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.Move, index: fromIndex, length, newIdx: toIndex, cells });

return true;
}
Expand Down
23 changes: 15 additions & 8 deletions packages/notebook/src/browser/view/notebook-main-toolbar.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ export class NotebookMainToolbar extends React.Component<NotebookMainToolbarProp

protected toDispose = new DisposableCollection();

protected nativeSubmenus = [
NotebookMenus.NOTEBOOK_MAIN_TOOLBAR_CELL_ADD_GROUP[NotebookMenus.NOTEBOOK_MAIN_TOOLBAR_CELL_ADD_GROUP.length - 1],
NotebookMenus.NOTEBOOK_MAIN_TOOLBAR_EXECUTION_GROUP[NotebookMenus.NOTEBOOK_MAIN_TOOLBAR_EXECUTION_GROUP.length - 1]];

constructor(props: NotebookMainToolbarProps) {
super(props);

Expand Down Expand Up @@ -107,22 +111,22 @@ export class NotebookMainToolbar extends React.Component<NotebookMainToolbarProp
</div>;
}

protected renderMenuItem(item: MenuNode): React.ReactNode {
protected renderMenuItem(item: MenuNode, submenu?: string): React.ReactNode {
if (item.role === CompoundMenuNodeRole.Group) {
const itemNodes = ArrayUtils.coalesce(item.children?.map(child => this.renderMenuItem(child)) ?? []);
const itemNodes = ArrayUtils.coalesce(item.children?.map(child => this.renderMenuItem(child, item.id)) ?? []);
return <React.Fragment key={item.id}>
{itemNodes}
{itemNodes && itemNodes.length > 0 && <span key={`${item.id}-separator`} className='theia-notebook-toolbar-separator'></span>}
</React.Fragment>;
} else if (!item.when || this.props.contextKeyService.match(item.when, this.props.editorNode)) {
} else if ((this.nativeSubmenus.includes(submenu ?? '')) || !item.when || this.props.contextKeyService.match(item.when, this.props.editorNode)) {
const visibleCommand = Boolean(this.props.commandRegistry.getVisibleHandler(item.command ?? '', this.props.notebookModel));
if (!visibleCommand) {
return undefined;
}
const title = (this.props.commandRegistry.getCommand(item.command ?? '') as NotebookCommand)?.tooltip ?? item.label;
return <div key={item.id} title={title} className='theia-notebook-main-toolbar-item action-label'
return <div key={item.id} title={title} className={`theia-notebook-main-toolbar-item action-label${this.getAdditionalClasses(item)}`}
onClick={() => {
if (item.command) {
if (item.command && (!item.when || this.props.contextKeyService.match(item.when, this.props.editorNode))) {
this.props.commandRegistry.executeCommand(item.command, this.props.notebookModel);
}
}}>
Expand All @@ -133,15 +137,18 @@ export class NotebookMainToolbar extends React.Component<NotebookMainToolbarProp
return undefined;
}

private getMenuItems(): readonly MenuNode[] {
protected getMenuItems(): readonly MenuNode[] {
const menuPath = NotebookMenus.NOTEBOOK_MAIN_TOOLBAR;
const pluginCommands = this.props.menuRegistry.getMenuNode(menuPath).children;
const theiaCommands = this.props.menuRegistry.getMenu([menuPath]).children;
// TODO add specifc arguments to commands
return theiaCommands.concat(pluginCommands);
}

private getAllContextKeys(menus: readonly MenuNode[], keySet: Set<string>): void {
protected getAdditionalClasses(item: MenuNode): string {
return !item.when || this.props.contextKeyService.match(item.when, this.props.editorNode) ? '' : ' theia-mod-disabled';
}

protected getAllContextKeys(menus: readonly MenuNode[], keySet: Set<string>): void {
menus.filter(item => item.when)
.forEach(item => this.props.contextKeyService.parseKeys(item.when!)?.forEach(key => keySet.add(key)));

Expand Down

0 comments on commit bce99c2

Please sign in to comment.