Positron Notebooks UI sprint

src/vs/base/browser/ui/positronComponents/button/button.tsx

@@ -34,7 +34,7 @@ export interface KeyboardModifiers {
 /**
  * ButtonProps interface.
  */
-interface ButtonProps {
+export interface ButtonProps {
 	readonly ariaLabel?: string;
 	readonly className?: string;
 	readonly disabled?: boolean;

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

@@ -3,20 +3,28 @@
  *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
  *--------------------------------------------------------------------------------------------*/
 .positron-cell-editor-monaco-widget {
-	/* Override the default editor background */
-	--vscode-editor-background: var(--vscode-checkbox-background, #f8f8f8);
-	--vscode-editorGutter-background: var(--vscode-editor-background);
 	border: var(--vscode-positronNotebook-border);
+	border-left: none; /* The selection bar on the left handles the left-side border */
 	border-radius: var(--vscode-positronNotebook-cell-radius);
+	/* The selection bar handles the left-side border radius */
+	border-bottom-left-radius: 0;
+	border-top-left-radius: 0;
 	padding: 0;
-	overflow: hidden;
+	overflow: hidden; /* Clip content to respect rounded corners */
+}
+
+/* Show focus outline with default VS Code focus color */
+.positron-cell-editor-monaco-widget:focus-within {
+	outline: 1px solid var(--vscode-focusBorder);
+	outline-offset: -1px;
+}
 
-	&:focus-within {
-		outline: 1px solid var(--vscode-focusBorder);
-		outline-offset: -1px;
-	}
+/* When the parent cell has an error, highlight the editor with error color */
+.positron-notebook-cell[data-has-error="true"] .positron-cell-editor-monaco-widget:focus-within {
+	outline-color: var(--vscode-positronNotebook-error-color);
+}
 
-	.positron-monaco-editor-container {
-		min-height: 1rem;
-	}
+/* Ensure the editor container has minimum height for empty cells */
+.positron-monaco-editor-container {
+	min-height: 1rem;
 }

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

@@ -122,6 +122,11 @@
 	font-family: var(--monaco-monospace-font);
 }
 
+/* Error state styling - use error color for execution badge */
+.execution-order-badge-container[data-has-error="true"] {
+	color: var(--vscode-errorForeground);
+}
+
 /* Animation for running state */
 @keyframes spin {
 	0% {

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

@@ -22,6 +22,7 @@ import { CellActionButton } from './actionBar/CellActionButton.js';
 
 interface CellLeftActionMenuProps {
 	cell: PositronNotebookCodeCell;
+	hasError: boolean;
 }
 
 const POPUP_DELAY = 100;
@@ -32,7 +33,7 @@ const POPUP_DELAY = 100;
  * - Action buttons (play, stop, etc.) when selected or hovered
  * - Execution info popup on hover with timing details
  */
-export function CellLeftActionMenu({ cell }: CellLeftActionMenuProps) {
+export function CellLeftActionMenu({ cell, hasError }: CellLeftActionMenuProps) {
 	// Reference hooks.
 	const containerRef = useRef<HTMLDivElement>(null);
 	const hoverTimeoutIdRef = useRef<number | null>(null);
@@ -110,6 +111,7 @@ export function CellLeftActionMenu({ cell }: CellLeftActionMenuProps) {
 					cellSelected={isSelected}
 					executionOrder={executionOrder}
 					executionStatus={dataExecutionStatus}
+					hasError={hasError}
 					isHovered={isHovered}
 					showPending={showPending}
 				/>

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

@@ -11,14 +11,15 @@ interface ExecutionStatusBadgeProps {
 	executionOrder?: number;
 	showPending: boolean;
 	executionStatus: string;
+	hasError: boolean;
 }
 
 /**
  * Component that displays execution order badges like [1], [2], or [-] for pending cells.
  * Used when the cell is not selected/hovered and showing static execution status.
  * Only renders when the cell is in the 'idle' execution state.
  */
-export function ExecutionStatusBadge({ cellSelected, isHovered, executionOrder, showPending, executionStatus }: ExecutionStatusBadgeProps) {
+export function ExecutionStatusBadge({ cellSelected, isHovered, executionOrder, showPending, executionStatus, hasError }: ExecutionStatusBadgeProps) {
 
 	if (cellSelected || isHovered) {
 		// We show action buttons in this case
@@ -36,7 +37,7 @@ export function ExecutionStatusBadge({ cellSelected, isHovered, executionOrder,
 
 	if (executionOrder !== undefined) {
 		return (
-			<div className='execution-order-badge-container'>
+			<div className='execution-order-badge-container' data-has-error={hasError}>
 				<span className='execution-order-badge-bracket'>[</span>
 				<span className='execution-order-badge'> {String(executionOrder)} </span>
 				<span className='execution-order-badge-bracket'>]</span>

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

@@ -9,6 +9,7 @@
 	--_positron-notebook-selection-bar-width: 7px;
 	--_positron-notebook-selection-bar-top-offset: 5px;
 	--_positron-notebook-selection-bar-inset: 0;
+	--_positron-notebook-selection-bar-gap: 2px;
 }
 
 /* Base styles for selected and editing states */
@@ -27,46 +28,79 @@
 	position: relative;
 }
 
-/* Common base styles for all selection bars */
-.positron-notebook-cell.selected .positron-notebook-editor-container::before,
-.positron-notebook-cell.editing .positron-notebook-editor-container::before,
-.positron-notebook-cell.selected .positron-notebook-cell-outputs::before,
-.positron-notebook-cell.editing .positron-notebook-cell-outputs::before {
+/* Add permanent left padding to editor container to make room for selection bar */
+.positron-notebook-editor-container {
+	position: relative;
+	padding-left: calc(var(--_positron-notebook-selection-bar-width) + var(--_positron-notebook-selection-bar-gap));
+}
+
+/* Permanent spacer/selection bar for editor - always visible, changes color when selected */
+.positron-notebook-editor-container::after {
 	content: "";
 	position: absolute;
 	width: var(--_positron-notebook-selection-bar-width);
-	background-color: var(--vscode-focusBorder);
-	z-index: 1;
-	left: var(--_positron-notebook-selection-bar-inset);
+	background-color: var(--vscode-editor-background);
+	left: var(--_positron-notebook-selection-bar-gap);
 	top: var(--_positron-notebook-selection-bar-inset);
 	bottom: var(--_positron-notebook-selection-bar-inset);
+	border: var(--vscode-positronNotebook-border);
+	border-right: none;
+	border-top-left-radius: var(--_positron-notebook-selection-bar-radius);
+	border-bottom-left-radius: var(--_positron-notebook-selection-bar-radius);
+	transition: border-bottom-left-radius 0.15s ease;
 }
 
-/* Don't round bottom corner for editor bar to make it look like a continuous bar */
-.positron-notebook-cell.selected .positron-notebook-editor-container::before,
-.positron-notebook-cell.editing .positron-notebook-editor-container::before {
-	border-top-left-radius: var(--_positron-notebook-selection-bar-radius);
+/* When selected/editing: change to focus color and remove bottom rounding by default */
+.positron-notebook-cell.selected .positron-notebook-editor-container::after,
+.positron-notebook-cell.editing .positron-notebook-editor-container::after {
+	background-color: var(--vscode-focusBorder);
 	border-bottom-left-radius: 0;
 }
 
-/* Specific positioning for outputs bar */
-.positron-notebook-cell.selected .positron-notebook-cell-outputs::before,
-.positron-notebook-cell.editing .positron-notebook-cell-outputs::before {
+/* Selection bar for outputs - conditional (only when selected/editing) */
+.positron-notebook-cell.selected .positron-notebook-cell-outputs::after,
+.positron-notebook-cell.editing .positron-notebook-cell-outputs::after {
+	content: "";
+	position: absolute;
+	width: var(--_positron-notebook-selection-bar-width);
+	background-color: var(--vscode-focusBorder);
 	/* Push bar down slightly so there's a gap between the editor and outputs */
 	top: var(--_positron-notebook-selection-bar-top-offset);
+	bottom: var(--_positron-notebook-selection-bar-inset);
+	left: var(--_positron-notebook-selection-bar-gap);
+	border: var(--vscode-positronNotebook-border);
+	border-right: none;
 	/* Don't round top corner for outputs bar to make it look like a continuous bar*/
 	border-top-left-radius: 0;
 	border-bottom-left-radius: var(--_positron-notebook-selection-bar-radius);
 }
 
-/* When outputs are empty, round both corners of editor bar */
-.positron-notebook-cell.selected:has(.positron-notebook-cell-outputs.no-outputs) .positron-notebook-editor-container::before,
-.positron-notebook-cell.editing:has(.positron-notebook-cell-outputs.no-outputs) .positron-notebook-editor-container::before {
+/* When selected/editing AND no outputs: restore bottom rounding */
+.positron-notebook-cell.selected:has(.positron-notebook-cell-outputs.no-outputs) .positron-notebook-editor-container::after,
+.positron-notebook-cell.editing:has(.positron-notebook-cell-outputs.no-outputs) .positron-notebook-editor-container::after {
 	border-bottom-left-radius: var(--_positron-notebook-selection-bar-radius);
 }
 
 /* When editor container is empty, round both corners of outputs bar (This is the case when markdown has been rendered but doesn't have an editor open) */
-.positron-notebook-cell.selected:has(.positron-notebook-editor-container.editor-hidden) .positron-notebook-cell-outputs::before {
+.positron-notebook-cell.selected:has(.positron-notebook-editor-container.editor-hidden) .positron-notebook-cell-outputs::after {
 	top: var(--_positron-notebook-selection-bar-inset);
 	border-top-left-radius: var(--_positron-notebook-selection-bar-radius);
 }
+
+/* Hide the editor selection bar when the editor is hidden (e.g., collapsed markdown cells) */
+.positron-notebook-editor-container.editor-hidden::after {
+	display: none;
+}
+
+/* Error state styling - use error color instead of focus color when selected */
+.positron-notebook-cell[data-has-error="true"]:is(.selected, .editing) .positron-notebook-editor-container::after,
+.positron-notebook-cell[data-has-error="true"]:is(.selected, .editing) .positron-notebook-cell-outputs::after {
+	background-color: var(--vscode-errorForeground);
+}
+
+/* Respect user's motion preferences */
+@media (prefers-reduced-motion: reduce) {
+	.positron-notebook-editor-container::after {
+		transition: none;
+	}
+}

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

@@ -4,6 +4,10 @@
  *--------------------------------------------------------------------------------------------*/
 
 .positron-notebook-cell {
+	/* Override the default editor background */
+	--vscode-editor-background: var(--vscode-checkbox-background, #f8f8f8);
+	--vscode-editorGutter-background: var(--vscode-editor-background);
+
 	position: relative;
 	margin-left: 12px;
 

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

@@ -23,10 +23,11 @@ import { useCellContextKeys } from './useCellContextKeys.js';
 import { CellScopedContextKeyServiceProvider } from './CellContextKeyServiceProvider.js';
 import { ScreenReaderOnly } from '../../../../../base/browser/ui/positronComponents/ScreenReaderOnly.js';
 
-export function NotebookCellWrapper({ cell, actionBarChildren, children }: {
+export function NotebookCellWrapper({ cell, actionBarChildren, children, hasError }: {
 	cell: IPositronNotebookCell;
 	actionBarChildren?: React.ReactNode;
 	children: React.ReactNode;
+	hasError?: boolean;
 }) {
 	const cellRef = React.useRef<HTMLDivElement>(null);
 	const notebookInstance = useNotebookInstance();
@@ -85,6 +86,7 @@ export function NotebookCellWrapper({ cell, actionBarChildren, children }: {
 		aria-label={localize('notebookCell', '{0} cell', cellType)}
 		aria-selected={isSelected}
 		className={`positron-notebook-cell positron-notebook-${cell.kind === CellKind.Code ? 'code' : 'markdown'}-cell ${selectionStatus}`}
+		data-has-error={hasError}
 		data-is-running={executionStatus === 'running'}
 		data-testid='notebook-cell'
 		role='article'

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

@@ -39,18 +39,20 @@ function CellOutputsSection({ outputs }: CellOutputsSectionProps) {
 
 export function NotebookCodeCell({ cell }: { cell: PositronNotebookCodeCell }) {
 	const outputContents = useObservedValue(cell.outputs);
+	const hasError = outputContents.some(o => o.parsed.type === 'error');
 
 	return (
 		<NotebookCellWrapper
 			cell={cell}
+			hasError={hasError}
 		>
 			<div className='positron-notebook-code-cell-contents'>
 				<div className='positron-notebook-editor-container'>
 					<CellEditorMonacoWidget cell={cell} />
 				</div>
 				<CellOutputsSection outputs={outputContents} />
 			</div>
-			<CellLeftActionMenu cell={cell} />
+			<CellLeftActionMenu cell={cell} hasError={hasError} />
 		</NotebookCellWrapper>
 	);
 }

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

@@ -3,7 +3,7 @@
  *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
  *--------------------------------------------------------------------------------------------*/
 
-.positron-notebook-markup-rendered {
+.positron-notebook-markdown-rendered {
 	padding-inline: 1rem;
 	padding-block: 0.5rem;
 

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

@@ -15,11 +15,9 @@ import { useObservedValue } from '../useObservedValue.js';
 import { Markdown } from './Markdown.js';
 import { NotebookCellWrapper } from './NotebookCellWrapper.js';
 import { PositronNotebookMarkdownCell } from '../PositronNotebookCells/PositronNotebookMarkdownCell.js';
-import { useNotebookInstance } from '../NotebookInstanceProvider.js';
 
 export function NotebookMarkdownCell({ cell }: { cell: PositronNotebookMarkdownCell }) {
 
-	const notebookInstance = useNotebookInstance();
 	const markdownString = useObservedValue(cell.markdownString);
 	const editorShown = useObservedValue(cell.editorShown);
 
@@ -31,16 +29,9 @@ export function NotebookMarkdownCell({ cell }: { cell: PositronNotebookMarkdownC
 				{editorShown ? <CellEditorMonacoWidget cell={cell} /> : null}
 			</div>
 			<div className='cell-contents positron-notebook-cell-outputs'>
-				<div
-					className='positron-notebook-markup-rendered'
-					onDoubleClick={(e) => {
-						// Prevent bubbling to wrapper's onClick and default browser behavior
-						e.stopPropagation();
-						e.preventDefault();
-						// Enter edit mode for this cell
-						notebookInstance.selectionStateMachine.enterEditor(cell);
-					}}
-				>
+				<div className='positron-notebook-markdown-rendered' onDoubleClick={() => {
+					cell.toggleEditor();
+				}}>
 					{
 						markdownString.length > 0 ?
 							<Markdown content={markdownString} />

src/vs/workbench/contrib/positronNotebook/browser/notebookCells/actionBar/NotebookCellMoreActionsMenu.tsx

@@ -3,10 +3,6 @@
  *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
  *--------------------------------------------------------------------------------------------*/
 
-
-// CSS.
-import '../../../../../../base/browser/ui/positronComponents/button/button.css';
-
 // React.
 import React, { useRef } from 'react';
 
@@ -18,6 +14,7 @@ import { IPositronNotebookCell } from '../../PositronNotebookCells/IPositronNote
 import { buildMoreActionsMenuItems } from './actionBarMenuItems.js';
 import { INotebookCellActionBarItem } from './actionBarRegistry.js';
 import { usePositronReactServicesContext } from '../../../../../../base/browser/positronReactRendererContext.js';
+import { ActionButton } from '../../utilityComponents/ActionButton.js';
 
 interface NotebookCellMoreActionsMenuProps {
 	instance: IPositronNotebookInstance;
@@ -39,10 +36,7 @@ export function NotebookCellMoreActionsMenu({
 	const buttonRef = useRef<HTMLButtonElement>(null);
 	const [isMenuOpen, setIsMenuOpen] = React.useState(false);
 	const commandService = usePositronReactServicesContext().commandService;
-	const showMoreActionsMenu = (e: React.MouseEvent<HTMLButtonElement>) => {
-		// Prevent click from bubbling to cell wrapper which would deselect the cell
-		e.preventDefault();
-		e.stopPropagation();
+	const showMoreActionsMenu = () => {
 
 		if (!buttonRef.current) {
 			return;
@@ -74,15 +68,14 @@ export function NotebookCellMoreActionsMenu({
 	};
 
 	return (
-		<button
+		<ActionButton
 			ref={buttonRef}
 			aria-expanded={isMenuOpen}
 			aria-haspopup='menu'
-			aria-label={localize('moreActions', 'More actions')}
-			className='positron-button'
-			onClick={showMoreActionsMenu}
+			ariaLabel={localize('moreActions', 'More actions')}
+			onPressed={showMoreActionsMenu}
 		>
 			<div className='button-icon codicon codicon-ellipsis' />
-		</button>
+		</ActionButton>
 	);
 }

src/vs/workbench/contrib/positronNotebook/browser/utilityComponents/ActionButton.tsx

@@ -7,17 +7,19 @@
 import './ActionButton.css';
 
 // React.
-import React from 'react';
+import React, { PropsWithChildren } from 'react';
 
 // Other dependencies.
-import { Button } from '../../../../../base/browser/ui/positronComponents/button/button.js';
+import { Button, ButtonProps } from '../../../../../base/browser/ui/positronComponents/button/button.js';
 
 /**
  * Plain classed button for actions in notebook with common styles.
  * @param props The props for the button
  * @return A button with `action` and `action-button` classes added to it.
  */
-// eslint-disable-next-line react/prop-types
-export function ActionButton({ className, ...props }: React.ComponentProps<typeof Button>) {
-	return <Button className={`action action-button ${className}`} {...props} />;
-}
+export const ActionButton = React.forwardRef<HTMLButtonElement, PropsWithChildren<ButtonProps>>(
+	({ className, ...props }, ref) => {
+		return <Button ref={ref} className={`action action-button ${className}`} {...props} />;
+	}
+);
+ActionButton.displayName = 'ActionButton';