Skip to content

Commit c9ff264

Browse files
committed
fix: debounce notebook cell outputs and status to prevent flash on re-execution
1 parent 9853e94 commit c9ff264

3 files changed

Lines changed: 51 additions & 9 deletions

File tree

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { useEffect, useRef, useState } from 'react';
1212
// Other dependencies.
1313
import { localize } from '../../../../../nls.js';
1414
import * as DOM from '../../../../../base/browser/dom.js';
15-
import { useObservedValue } from '../useObservedValue.js';
15+
import { useObservedValue, useDebouncedObservedValue } from '../useObservedValue.js';
1616
import { PositronNotebookCodeCell } from '../PositronNotebookCells/PositronNotebookCodeCell.js';
1717
import { formatCellDuration, formatTimestamp, getRelativeTime, isMoreThanOneHourAgo } from './cellExecutionUtils.js';
1818
import { Icon } from '../../../../../platform/positronActionBar/browser/components/icon.js';
@@ -23,17 +23,20 @@ interface CodeCellStatusFooterProps {
2323
hasError: boolean;
2424
}
2525

26+
const isRunningOrPending = (s: string) => s === 'running' || s === 'pending';
27+
2628
/**
2729
* Footer component that displays cell execution status information between
2830
* the editor and outputs sections. Shows execution state, duration, and timestamp.
2931
*/
3032
export function CodeCellStatusFooter({ cell, hasError }: CodeCellStatusFooterProps) {
31-
// Observe cell execution state
32-
const executionStatus = useObservedValue(cell.executionStatus);
33+
// Debounce "clearing" transitions to prevent visual flash during fast re-executions.
34+
// Only delay transitions to running/pending/undefined; new values propagate immediately.
35+
const executionStatus = useDebouncedObservedValue(cell.executionStatus, isRunningOrPending);
3336
const executionOrder = useObservedValue(cell.lastExecutionOrder);
34-
const duration = useObservedValue(cell.lastExecutionDuration);
35-
const lastRunEndTime = useObservedValue(cell.lastRunEndTime);
36-
const lastRunSuccess = useObservedValue(cell.lastRunSuccess);
37+
const duration = useDebouncedObservedValue(cell.lastExecutionDuration);
38+
const lastRunEndTime = useDebouncedObservedValue(cell.lastRunEndTime);
39+
const lastRunSuccess = useDebouncedObservedValue(cell.lastRunSuccess);
3740

3841
/**
3942
* `lastRunEndTime` doesn't change after execution completes, which means the

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import React, { useMemo } from 'react';
1212
// Other dependencies.
1313
import { NotebookCellOutputs, ParsedTextOutput } from '../PositronNotebookCells/IPositronNotebookCell.js';
1414
import { isParsedTextOutput } from '../getOutputContents.js';
15-
import { useObservedValue } from '../useObservedValue.js';
15+
import { useObservedValue, useDebouncedObservedValue } from '../useObservedValue.js';
1616
import { CellEditorMonacoWidget } from './CellEditorMonacoWidget.js';
1717
import { localize } from '../../../../../nls.js';
1818
import { positronClassNames } from '../../../../../base/common/positronUtilities.js';
@@ -209,8 +209,11 @@ const CellOutputsSection = React.memo(function CellOutputsSection({ cell, output
209209
return prevProps.outputs === nextProps.outputs;
210210
});
211211

212+
const isEmptyArray = (a: unknown[]) => a.length === 0;
213+
212214
export const NotebookCodeCell = React.memo(function NotebookCodeCell({ cell }: { cell: PositronNotebookCodeCell }) {
213-
const outputContents = useObservedValue(cell.outputs);
215+
// Debounce only transitions to empty so re-execution doesn't flash.
216+
const outputContents = useDebouncedObservedValue(cell.outputs, isEmptyArray);
214217
const hasError = outputContents.some(o => o.parsed.type === 'error');
215218

216219
return (

src/vs/workbench/contrib/positronNotebook/browser/useObservedValue.tsx

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
import React from 'react';
88

99
// Other dependencies.
10-
import { IObservable, runOnChange } from '../../../../base/common/observable.js';
10+
import { IObservable, debouncedObservable, runOnChange } from '../../../../base/common/observable.js';
11+
import { isUndefinedOrNull } from '../../../../base/common/types.js';
1112

1213
/**
1314
* Automatically updates the component when the observable changes.
@@ -39,3 +40,38 @@ export function useObservedValue<T>(observable: IObservable<T> | undefined, defa
3940

4041
return value;
4142
}
43+
44+
/**
45+
* Like {@link useObservedValue}, but debounces value transitions where the
46+
* provided `shouldDebounce` predicate returns true. Non-debounced transitions
47+
* propagate immediately.
48+
*
49+
* Useful for suppressing transient UI flashes during cell re-execution without
50+
* delaying meaningful updates.
51+
*
52+
* @param observable The observable to subscribe to.
53+
* @param shouldDebounce Predicate receiving the new value. Return `true` to
54+
* delay propagation by the specified `delayMs`, `false` to propagate
55+
* immediately. Defaults to nullish check which covers `undefined` and `null`
56+
* but not valid falsy values like `0` or `false`. Note that this is used in
57+
* the dependency array of the `useMemo` call that creates the debounced
58+
* observable, so it should be memoized if it is not a stable reference or
59+
* otherwise the memo will be invalidated on every render, defeating the
60+
* purpose of debouncing.
61+
* @param delayMs Optional debounce delay in milliseconds. Defaults to 150ms.
62+
*/
63+
64+
export function useDebouncedObservedValue<T>(
65+
observable: IObservable<T>,
66+
shouldDebounce: (next: T) => boolean = isUndefinedOrNull,
67+
delayMs: number = 150,
68+
): T {
69+
const debounced = React.useMemo(
70+
() => debouncedObservable(observable, (_prev, next) => shouldDebounce(next) ? delayMs : 0),
71+
// We dont need to worry about the delayMs causing invalidation because
72+
// pure number types are compared by value not reference in the
73+
// dependency arrays of hooks.
74+
[observable, shouldDebounce, delayMs]
75+
);
76+
return useObservedValue(debounced);
77+
}

0 commit comments

Comments
 (0)