Skip to content

Commit 492e399

Browse files
Positron notebook contributable cell actions (#9172)
Addresses #8791. ### Summary This PR implements a contributable cell action bar system for Positron notebooks, establishing the infrastructure for extensions and other parts of the codebase to dynamically register actions that appear in notebook cell action bars. The implementation follows VS Code patterns by separating command execution from UI metadata and using a registry system. **Context menus are now functional** - several actions have been implemented using the `"menu"` position, including cell movement actions (Move Cell Up/Down) and execution actions (Run Above/Below), organized by categories. **Run button has been migrated** - The primary run/stop button functionality has been successfully converted to use the contributable system with proper cell conditions for showing run vs stop states. The implementation was inspired by the positron action bar along with the vscode notebooks and their notebook contribution helper functions. <img width="952" height="553" alt="image" src="https://github.com/user-attachments/assets/a491fcb2-b055-489e-b385-a6f146deb020" /> _More menu with groupings for related actions_ <img width="972" height="482" alt="image" src="https://github.com/user-attachments/assets/f16e4bda-c25c-486d-9b2e-564d466a0b07" /> _Conditional system lets actions only show when applicable. Here the "run cells above" action is absent because that doesn't make sense for the first cell._ ### Architecture **Command and UI Separation:** - Commands are registered through VS Code's `CommandsRegistry` for execution logic - UI metadata is registered separately through `NotebookCellActionBarRegistry` for display configuration - This enables commands to work everywhere (command palette, keyboard shortcuts, API calls) **Registry System:** - `NotebookCellActionBarRegistry` - Singleton registry managing cell action bar contributions - Observable arrays for reactive UI updates when actions are added/removed so dynamically added actions are automatically reflected. - Support for main action bar (primary actions) and dropdown menu (secondary actions) - Configurable sorting, visibility conditions, context requirements, and cell-specific conditions **Cell Condition System:** - `CellConditionPredicate` functions determine if commands apply to specific cells based on type, position, and state - `ICellInfo` provides cell metadata (type, index, position flags, execution status) - Pre-built conditions in `CellConditions` helper (isCode, isMarkdown, notFirst, notLast, isRunning, etc.) - Composable logic operators (and, or, not) for complex conditions **Helper Function:** - `registerCellCommand()` - Convenient API to register both command and UI metadata in one call - Automatic cell selection handling with multi-select support - Optional UI registration with position, icon, visibility configuration, and cell conditions - Integrated keybinding support ### Key Features ```typescript // Run button with conditional visibility based on execution state registerCellCommand({ commandId: 'positronNotebook.cell.executeAndFocusContainer', handler: (cell) => cell.run(), cellCondition: CellConditions.and( CellConditions.isCode, CellConditions.not(CellConditions.isRunning) // Only show when not running ), actionBar: { icon: 'codicon-play', position: 'main', order: 10 }, keybinding: { primary: KeyMod.CtrlCmd | KeyCode.Enter } }); // Stop button replaces run button when cell is executing registerCellCommand({ commandId: 'positronNotebook.cell.stopExecution', handler: (cell) => cell.run(), // Run called when executing acts as stop cellCondition: CellConditions.and( CellConditions.isCode, CellConditions.isRunning // Only show when running ), actionBar: { icon: 'codicon-stop', position: 'main', order: 10 // Same position as run button for seamless replacement } }); ``` **UI Metadata Interface:** ```typescript interface INotebookCellActionBarItem { /** VS Code command ID to execute */ commandId: string; /** Command label, for display purposes, if not defined, use the commandId */ label?: ILocalizedString | string; /** Codicon class for the button icon (optional) */ icon?: string; /** Location in UI - either main action bar or dropdown menu */ position: 'main' | 'menu'; /** Sort order within position (lower numbers appear first) */ order?: number; /** Visibility condition using VS Code context keys (optional) */ when?: ContextKeyExpression; /** If true, the cell will be selected before executing the command */ needsCellContext?: boolean; /** Cell-specific condition that determines if this command applies to a given cell */ cellCondition?: CellConditionPredicate; /** Category of the action bar item. Items that share the same category will be grouped together. */ category?: string; } ``` ### Implementation Details **Refactored Components:** - `NotebookCellActionBar.tsx` now renders contributed actions dynamically using observable arrays - Registry integration with automatic cell context handling - All existing cell actions (run, stop, delete) now use the contributable system - Cell-type-specific actions maintain their behavior while being fully integrated **Context Menu System:** - `NotebookCellMoreActionsMenu.tsx` renders menu actions grouped by category with separators - Automatic cell selection when executing commands from the dropdown menu - Dynamic menu population based on registry contents **Migration Complete:** - **Run/Stop button**: Converted with conditional visibility based on execution state - **Delete functionality**: Converted with multi-select support and keyboard shortcuts preserved - **Cell movement**: Move Up/Down actions available in context menu - **Execution actions**: Run Above/Below commands in context menu - **Markdown editing**: Toggle editor for markdown cells **Currently Implemented Actions:** - **Main Action Bar**: Run/Stop (conditional), Delete (main), Toggle Markdown Editor - **Context Menu**: - **Cell Category**: Move Cell Up, Move Cell Down - **Execution Category**: Run Above, Run Below ### File Structure ``` notebookCells/ ├── actionBar/ │ ├── actionBarRegistry.ts # Registry for UI metadata │ ├── cellConditions.ts # Cell condition system with helpers │ ├── registerCellCommand.ts # Helper for command/keybinding/UI registration │ ├── actionBarMenuItems.ts # Context menu builder with category grouping │ ├── CellActionButton.tsx # Reusable button component │ ├── NotebookCellMoreActionsMenu.tsx # Context menu component │ └── useActionBarVisibility.ts # Visibility hook └── NotebookCellActionBar.tsx # Main action bar using registry system ``` ### Release Notes #### New Features - Cell action bar context menu with categorized actions - Cell movement commands available via dropdown menu - Run Above/Below commands for targeted execution - Unified contributable action system for all cell actions #### Bug Fixes - N/A ### QA Notes @:notebooks @:win There is a new e2e test for the delete action being run from the action bar. This verifies that the contributions are being properly registered and are functional. Manual testing: - Open a notebook with multiple cells - Verify the run button appears on code cells and changes to stop when executing - Click the "..." menu button to verify the context menu appears with categorized actions - Test Move Cell Up/Down actions to verify cell reordering works - Test Run Above/Below actions to verify partial execution works - Test delete functionality from both main action bar and multi-select - Verify actions respect cell conditions (e.g., Move Up disabled for first cell, Run only on code cells) - Test markdown cell edit toggle functionality - Verify keyboard shortcuts still work (Ctrl+Enter for run, Backspace/dd for delete, etc.) - Ensure functionality works across multiple open notebooks --------- Signed-off-by: Nick Strayer <[email protected]> Co-authored-by: Chris Mead <[email protected]>
1 parent cd23ba4 commit 492e399

24 files changed

+1414
-204
lines changed

src/vs/workbench/browser/positronComponents/customContextMenu/customContextMenu.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
}
8585

8686
.custom-context-menu-item .shortcut {
87-
padding: 10px;
87+
padding-inline: 10px; /* Vertical padding can cause overflow and supurious scrollbars. */
8888
white-space: nowrap;
8989
grid-column: shortcut / end;
9090
}

src/vs/workbench/browser/positronComponents/customContextMenu/customContextMenu.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export interface CustomContextMenuProps {
3737
readonly width?: number | 'auto';
3838
readonly minWidth?: number | 'auto';
3939
readonly entries: CustomContextMenuEntry[];
40+
readonly onClose?: () => void;
4041
}
4142

4243
/**
@@ -48,20 +49,23 @@ export interface CustomContextMenuProps {
4849
* @param width The width.
4950
* @param minWidth The minimum width.
5051
* @param entries The context menu entries.
52+
* @param onClose The callback to call when the context menu is closed/disposed.
5153
*/
52-
export const showCustomContextMenu = async ({
54+
export const showCustomContextMenu = ({
5355
anchorElement,
5456
anchorPoint,
5557
popupPosition,
5658
popupAlignment,
5759
width,
5860
minWidth,
59-
entries
61+
entries,
62+
onClose,
6063
}: CustomContextMenuProps) => {
6164
// Create the renderer.
6265
const renderer = new PositronModalReactRenderer({
6366
container: PositronReactServices.services.workbenchLayoutService.getContainer(DOM.getWindow(anchorElement)),
64-
parent: anchorElement
67+
parent: anchorElement,
68+
onDisposed: onClose
6569
});
6670

6771
// Supply the default width.

src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookCells/PositronNotebookCell.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Disposable } from '../../../../../base/common/lifecycle.js';
7-
import { ISettableObservable } from '../../../../../base/common/observableInternal/base.js';
87
import { URI } from '../../../../../base/common/uri.js';
98
import { ITextModel } from '../../../../../editor/common/model.js';
109
import { ITextModelService } from '../../../../../editor/common/services/resolverService.js';
@@ -14,7 +13,7 @@ import { ExecutionStatus, IPositronNotebookCodeCell, IPositronNotebookCell, IPos
1413
import { CodeEditorWidget } from '../../../../../editor/browser/widget/codeEditor/codeEditorWidget.js';
1514
import { CellSelectionType } from '../../../../services/positronNotebook/browser/selectionMachine.js';
1615
import { PositronNotebookInstance } from '../PositronNotebookInstance.js';
17-
import { observableValue } from '../../../../../base/common/observable.js';
16+
import { ISettableObservable, observableValue } from '../../../../../base/common/observable.js';
1817

1918
export abstract class PositronNotebookCellGeneral extends Disposable implements IPositronNotebookCell {
2019
kind!: CellKind;
@@ -106,6 +105,15 @@ export abstract class PositronNotebookCellGeneral extends Disposable implements
106105
deselect(): void {
107106
this._instance.selectionStateMachine.deselectCell(this);
108107
}
108+
109+
insertCodeCellAbove(): void {
110+
this._instance.insertCodeCellAndFocusContainer('above', this);
111+
}
112+
113+
insertCodeCellBelow(): void {
114+
this._instance.insertCodeCellAndFocusContainer('below', this);
115+
}
116+
109117
}
110118

111119

src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts

Lines changed: 77 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -512,41 +512,90 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
512512
this._onDidChangeContent.fire();
513513
}
514514

515-
insertCodeCellAndFocusContainer(aboveOrBelow: 'above' | 'below'): void {
516-
const indexOfSelectedCell = this.selectionStateMachine.getIndexOfSelectedCell();
517-
if (indexOfSelectedCell === null) {
515+
insertCodeCellAndFocusContainer(aboveOrBelow: 'above' | 'below', referenceCell?: IPositronNotebookCell): void {
516+
let index: number | null;
517+
518+
this._assertTextModel();
519+
520+
if (referenceCell) {
521+
const cellIndex = this.textModel.cells.indexOf(referenceCell.cellModel as NotebookCellTextModel);
522+
index = cellIndex >= 0 ? cellIndex : null;
523+
} else {
524+
index = this.selectionStateMachine.getIndexOfSelectedCell();
525+
}
526+
527+
if (index === null) {
518528
return;
519529
}
520530

521-
this.addCell(CellKind.Code, indexOfSelectedCell + (aboveOrBelow === 'above' ? 0 : 1));
531+
this.addCell(CellKind.Code, index + (aboveOrBelow === 'above' ? 0 : 1));
522532
}
523533

524534
deleteCell(cellToDelete?: IPositronNotebookCell): void {
525-
this._assertTextModel();
526-
527535
const cell = cellToDelete ?? this.selectionStateMachine.getSelectedCell();
528536

529537
if (!cell) {
530538
return;
531539
}
540+
this.deleteCells([cell]);
541+
}
542+
543+
deleteCells(cellsToDelete: IPositronNotebookCell[]): void {
544+
this._assertTextModel();
545+
546+
if (cellsToDelete.length === 0) {
547+
return;
548+
}
532549

533550
const textModel = this.textModel;
534551
// TODO: Hook up readOnly to the notebook actual value
535552
const readOnly = false;
536553
const computeUndoRedo = !readOnly || textModel.viewType === 'interactive';
537-
const cellIndex = textModel.cells.indexOf(cell.cellModel as NotebookCellTextModel);
538554

539-
const edits: ICellReplaceEdit = {
540-
editType: CellEditType.Replace, index: cellIndex, count: 1, cells: []
541-
};
555+
// Get indices and sort in descending order to avoid index shifting
556+
const cellIndices = cellsToDelete
557+
.map(cell => textModel.cells.indexOf(cell.cellModel as NotebookCellTextModel))
558+
.filter(index => index >= 0)
559+
.sort((a, b) => b - a);
542560

543-
const nextCellAfterContainingSelection = textModel.cells[cellIndex + 1] ?? undefined;
561+
if (cellIndices.length === 0) {
562+
return;
563+
}
564+
565+
// Calculate where focus should go after deletion
566+
const lowestDeletedIndex = Math.min(...cellIndices);
567+
const totalCellsToDelete = cellIndices.length;
568+
const originalCellCount = textModel.cells.length;
569+
const newCellCount = originalCellCount - totalCellsToDelete;
570+
571+
// Determine the index of the cell that should receive focus after deletion
572+
let targetFocusIndex: number | null = null;
573+
if (newCellCount > 0) {
574+
if (lowestDeletedIndex < newCellCount) {
575+
// Focus on the cell that takes the place of the first deleted cell
576+
targetFocusIndex = lowestDeletedIndex;
577+
} else {
578+
// We deleted from the end, focus on the last remaining cell
579+
targetFocusIndex = newCellCount - 1;
580+
}
581+
}
582+
583+
// Create delete edits for each cell
584+
const edits: ICellReplaceEdit[] = cellIndices.map(index => ({
585+
editType: CellEditType.Replace,
586+
index,
587+
count: 1,
588+
cells: []
589+
}));
590+
591+
// Find the cell that will be at the position of the first (lowest index) deleted cell
592+
const nextCellAfterContainingSelection = textModel.cells[lowestDeletedIndex + cellIndices.length] ?? undefined;
544593
const focusRange = {
545-
start: cellIndex,
546-
end: cellIndex + 1
594+
start: lowestDeletedIndex,
595+
end: lowestDeletedIndex + 1
547596
};
548597

549-
textModel.applyEdits([edits], true, { kind: SelectionStateType.Index, focus: focusRange, selections: [focusRange] }, () => {
598+
textModel.applyEdits(edits, true, { kind: SelectionStateType.Index, focus: focusRange, selections: [focusRange] }, () => {
550599
if (nextCellAfterContainingSelection) {
551600
const cellIndex = textModel.cells.findIndex(cell => cell.handle === nextCellAfterContainingSelection.handle);
552601
return { kind: SelectionStateType.Index, focus: { start: cellIndex, end: cellIndex + 1 }, selections: [{ start: cellIndex, end: cellIndex + 1 }] };
@@ -562,6 +611,20 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
562611
}, undefined, computeUndoRedo);
563612

564613
this._onDidChangeContent.fire();
614+
615+
// After the content change fires and cells are synced, explicitly set the selection
616+
// to maintain focus on the appropriate cell after deletion
617+
if (targetFocusIndex !== null) {
618+
this._register(disposableTimeout(() => {
619+
if (targetFocusIndex !== null && targetFocusIndex < this._cells.length) {
620+
const cellToFocus = this._cells[targetFocusIndex];
621+
if (cellToFocus) {
622+
this.selectionStateMachine.selectCell(cellToFocus, CellSelectionType.Normal);
623+
cellToFocus.focus();
624+
}
625+
}
626+
}, 0));
627+
}
565628
}
566629

567630

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellActionBar.css

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@
1515
display: flex;
1616
gap: 0.25rem;
1717

18+
&.visible {
19+
visibility: visible;
20+
}
21+
22+
&.hidden {
23+
visibility: hidden;
24+
}
25+
1826
.action-button {
1927
aspect-ratio: 1;
2028
display: grid;

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellActionBar.tsx

Lines changed: 93 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,107 @@
77
import './NotebookCellActionBar.css';
88

99
// React.
10-
import React from 'react';
10+
import React, { useState } from 'react';
1111

1212
// Other dependencies.
1313
import { localize } from '../../../../../nls.js';
14+
import { usePositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js';
1415
import { IPositronNotebookCell } from '../../../../services/positronNotebook/browser/IPositronNotebookCell.js';
1516
import { ActionButton } from '../utilityComponents/ActionButton.js';
17+
import { useNotebookInstance } from '../NotebookInstanceProvider.js';
18+
import { useSelectionStatus } from './useSelectionStatus.js';
19+
import { NotebookCellMoreActionsMenu } from './actionBar/NotebookCellMoreActionsMenu.js';
20+
import { useActionBarVisibility } from './actionBar/useActionBarVisibility.js';
21+
import { NotebookCellActionBarRegistry, INotebookCellActionBarItem } from './actionBar/actionBarRegistry.js';
22+
import { useObservedValue } from '../useObservedValue.js';
23+
import { CellSelectionType } from '../../../../services/positronNotebook/browser/selectionMachine.js';
24+
import { createCellInfo } from './actionBar/cellConditions.js';
1625

1726

18-
export function NotebookCellActionBar({ cell, children }: { cell: IPositronNotebookCell; children: React.ReactNode }) {
27+
interface NotebookCellActionBarProps {
28+
cell: IPositronNotebookCell;
29+
children: React.ReactNode;
30+
isHovered: boolean;
31+
}
32+
33+
export function NotebookCellActionBar({ cell, children, isHovered }: NotebookCellActionBarProps) {
34+
const services = usePositronReactServicesContext();
35+
const commandService = services.commandService;
36+
const instance = useNotebookInstance();
37+
const registry = NotebookCellActionBarRegistry.getInstance();
38+
const [isMenuOpen, setIsMenuOpen] = useState(false);
39+
const selectionStatus = useSelectionStatus(cell);
40+
41+
// Use observable values for reactive updates
42+
const allMainActions = useObservedValue(registry.mainActions) ?? [];
43+
const allMenuActions = useObservedValue(registry.menuActions) ?? [];
44+
45+
// Filter actions based on cell conditions
46+
const cells = instance.cells.get();
47+
const cellIndex = cells.indexOf(cell);
48+
const cellInfo = createCellInfo(cell, cellIndex, cells.length);
49+
50+
const mainActions = allMainActions.filter(action => {
51+
return !action.cellCondition || action.cellCondition(cellInfo);
52+
});
53+
54+
const menuActions = allMenuActions.filter(action => {
55+
return !action.cellCondition || action.cellCondition(cellInfo);
56+
});
57+
58+
const hasMenuActions = menuActions.length > 0;
1959

20-
return <div className='positron-notebooks-cell-action-bar'>
60+
// Determine visibility using the extracted hook
61+
const shouldShowActionBar = useActionBarVisibility(isHovered, isMenuOpen, selectionStatus);
62+
63+
const handleActionClick = (action: INotebookCellActionBarItem) => {
64+
// If action needs cell context, ensure cell is selected first
65+
if (action.needsCellContext) {
66+
// Select the cell if not already selected
67+
const currentState = instance.selectionStateMachine.state.get();
68+
const isSelected = (currentState.type !== 'NoSelection' &&
69+
currentState.type !== 'EditingSelection' &&
70+
currentState.selected.includes(cell));
71+
72+
if (!isSelected) {
73+
instance.selectionStateMachine.selectCell(cell, CellSelectionType.Normal);
74+
}
75+
}
76+
77+
// Execute the command (without passing cell as argument)
78+
commandService.executeCommand(action.commandId);
79+
};
80+
81+
return <div
82+
aria-hidden={!shouldShowActionBar}
83+
aria-label={localize('cellActions', 'Cell actions')}
84+
className={`positron-notebooks-cell-action-bar ${shouldShowActionBar ? 'visible' : 'hidden'}`}
85+
role='toolbar'
86+
>
87+
{/* Render cell-specific actions (e.g., run button for code cells) */}
2188
{children}
22-
<ActionButton
23-
ariaLabel={(() => localize('deleteCell', 'Delete cell'))()}
24-
onPressed={() => cell.delete()}
25-
>
26-
<div className='button-icon codicon codicon-trash' />
27-
</ActionButton>
89+
90+
{/* Render contributed main actions - will auto-update when registry changes */}
91+
{mainActions.map(action => (
92+
<ActionButton
93+
key={action.commandId}
94+
ariaLabel={String(action.label ?? action.commandId)}
95+
onPressed={() => handleActionClick(action)}
96+
>
97+
<div className={`button-icon codicon ${action.icon}`} />
98+
</ActionButton>
99+
))}
100+
101+
{/* Dropdown menu for additional actions - only render if there are menu actions */}
102+
{hasMenuActions ? (
103+
<NotebookCellMoreActionsMenu
104+
cell={cell}
105+
commandService={commandService}
106+
instance={instance}
107+
menuActions={menuActions}
108+
onMenuStateChange={setIsMenuOpen}
109+
/>
110+
) : null}
28111
</div>;
29112
}
113+

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellWrapper.css

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@
99

1010
border-radius: var(--vscode-positronNotebook-cell-radius);
1111

12-
.positron-notebooks-cell-action-bar {
13-
visibility: hidden;
14-
}
15-
1612
&[data-is-running='true'] {
1713
outline: 1px solid var(--vscode-focusBorder);
1814
animation: running-cell-pulse 2s ease-in-out infinite;
@@ -21,26 +17,6 @@
2117
&.selected {
2218
outline: 1px solid var(--vscode-focusBorder);
2319
}
24-
25-
&.editing,
26-
&.selected,
27-
&:hover {
28-
.positron-notebooks-cell-action-bar {
29-
visibility: visible;
30-
}
31-
}
32-
33-
.positron-notebooks-cell-action-bar {
34-
visibility: hidden;
35-
}
36-
37-
&.editing,
38-
&.selected,
39-
&:hover {
40-
.positron-notebooks-cell-action-bar {
41-
visibility: visible;
42-
}
43-
}
4420
}
4521

4622
@keyframes running-cell-pulse {

0 commit comments

Comments
 (0)