Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ export interface IPositronNotebookCell extends Disposable {
* Detach the editor from the cell
*/
detachEditor(): void;

/**
* Whether the cell has error output
*/
readonly hasError: IObservable<boolean>;
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export abstract class PositronNotebookCellGeneral extends Disposable implements
public readonly executionStatus;
public readonly selectionStatus = observableValue<CellSelectionStatus, void>('cellSelectionStatus', CellSelectionStatus.Unselected);
public readonly editorFocusRequested: IObservableSignal<void> = this._editorFocusRequested;
public readonly hasError = observableValue<boolean, void>('hasError', false);

constructor(
public readonly cellModel: NotebookCellTextModel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ export class PositronNotebookCodeCell extends PositronNotebookCellGeneral implem

this.outputs = observableFromEvent(this, this.cellModel.onDidChangeOutputs, () => {
/** @description cellOutputs */
return this.parseCellOutputs();
const parsedOutputs = this.parseCellOutputs();
// Update hasError when outputs change
const hasError = parsedOutputs.some(o => o.parsed.type === 'error');
this.hasError.set(hasError, undefined);
return parsedOutputs;
});
Comment on lines +40 to 45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could keep decouple these observables

this.outputs = observableFromEvent(this, this.cellModel.onDidChangeOutputs, () => {
    /** @description cellOutputs */
    return this.parseCellOutputs();
});
this.hasError = this.outputs.map((outputs) => outputs.some(o => o.parsed.type === 'error'));

or even move hasError to the React component, derived from the outputs state.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW the @descriptions were something I saw upstream that are apparently used in debugging logs, but I haven't actually checked where those logs live, so we can decide whether to keep em.


// Execution timing observables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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% {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export function CellLeftActionMenu({ cell }: CellLeftActionMenuProps) {
const executionStatus = useObservedValue(cell.executionStatus);
const duration = useObservedValue(cell.lastExecutionDuration);
const lastRunEndTime = useObservedValue(cell.lastRunEndTime);
const hasError = useObservedValue(cell.hasError);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to avoid tracking this state both here and in NotebookCellWrapper and making one a prop? We might be doing that in a couple places


// Derived state
const isRunning = executionStatus === 'running';
Expand Down Expand Up @@ -110,6 +111,7 @@ export function CellLeftActionMenu({ cell }: CellLeftActionMenuProps) {
cellSelected={isSelected}
executionOrder={executionOrder}
executionStatus={dataExecutionStatus}
hasError={hasError}
isHovered={isHovered}
showPending={showPending}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function NotebookCellWrapper({ cell, actionBarChildren, children }: {
const environment = useEnvironment();
const selectionStatus = useObservedValue(cell.selectionStatus);
const executionStatus = useObservedValue(cell.executionStatus);
const hasError = useObservedValue(cell.hasError);

React.useEffect(() => {
if (cellRef.current) {
Expand Down Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ 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={() => {
<div className='positron-notebook-markdown-rendered' onDoubleClick={() => {
cell.toggleEditor();
}}>
{
markdownString.length > 0 ?
<Markdown content={markdownString} />
: <div className='empty-output-msg'>
Empty markup cell. {editorShown ? '' : 'Double click to edit'}
Empty markdown cell. {editorShown ? '' : 'Double click to edit'}
</div>
}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
Expand Down Expand Up @@ -70,15 +67,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>
);
}
Loading