Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to overlay webviews for notebooks #5829

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 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
@@ -0,0 +1,37 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

// React.
import React, { PropsWithChildren, createContext, useContext } from 'react';

// Other dependencies.
import { ISettableObservable } from '../../../../base/common/observableInternal/base.js';

/**
* Create the notebook visibility context.
*/
const NotebookVisibilityContext = createContext<ISettableObservable<boolean>>(undefined!);

/**
* Provider component for notebook visibility state
*/
export const NotebookVisibilityProvider = ({
isVisible,
children
}: PropsWithChildren<{ isVisible: ISettableObservable<boolean> }>) => {
return (
<NotebookVisibilityContext.Provider value={isVisible}>
{children}
</NotebookVisibilityContext.Provider>
);
};

/**
* Hook to access the notebook visibility state
* @returns The current visibility state as a boolean
*/
export const useNotebookVisibility = () => {
return useContext(NotebookVisibilityContext)
};
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,16 @@ export function PositronNotebookComponent() {
const notebookInstance = useNotebookInstance();
const notebookCells = useObservedValue(notebookInstance.cells);
const fontStyles = useFontStyles();
const containerRef = React.useRef<HTMLDivElement>(null);

React.useEffect(() => {
notebookInstance.setCellsContainer(containerRef.current);
}, [notebookInstance]);

return (
<div className='positron-notebook' style={{ ...fontStyles }}>
<PositronNotebookHeader notebookInstance={notebookInstance} />
<div className='positron-notebook-cells-container'>
<div className='positron-notebook-cells-container' ref={containerRef}>
{notebookCells?.length ? notebookCells?.map((cell, index) => <>
<NotebookCell key={cell.handleId} cell={cell as PositronNotebookCellGeneral} />
<AddCellButtons index={index + 1} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import { IOpenerService } from '../../../../platform/opener/common/opener.js';
import { INotificationService } from '../../../../platform/notification/common/notification.js';
import { IPositronNotebookOutputWebviewService } from '../../positronOutputWebview/browser/notebookOutputWebviewService.js';
import { IPositronWebviewPreloadService } from '../../../services/positronWebviewPreloads/browser/positronWebviewPreloadService.js';
import { NotebookVisibilityProvider } from './NotebookVisibilityContext.js';


interface NotebookLayoutInfo {
Expand Down Expand Up @@ -219,11 +220,22 @@ export class PositronNotebookEditor extends EditorPane {
*/
private _size = observableValue<ISize>('size', { width: 0, height: 0 });

/**
* Observable tracking if the editor is currently visible
*/
private readonly _isVisible = observableValue<boolean>('isVisible', false);


// Getter for notebook instance to avoid having to cast the input every time.
get notebookInstance() {
return (this.input as PositronNotebookEditorInput)?.notebookInstance;
}

protected override setEditorVisible(visible: boolean): void {
this._isVisible.set(visible, undefined);
super.setEditorVisible(visible);
}

protected override createEditor(parent: HTMLElement): void {

this._logService.info(this._identifier, 'createEditor');
Expand Down Expand Up @@ -315,14 +327,19 @@ export class PositronNotebookEditor extends EditorPane {
override clearInput(): void {
this._logService.info(this._identifier, 'clearInput');


// Clear the input observable.
this._input = undefined;
if (this.notebookInstance && this._parentDiv) {
this.notebookInstance.detachView();
console.log('isVisible', this._isVisible.get());
}

if (this.notebookInstance) {
this._saveEditorViewState();
this.notebookInstance.detachView();
}

// Clear the input observable.
this._input = undefined;

this._disposeReactRenderer();

// Call the base class's method.
Expand Down Expand Up @@ -444,24 +461,26 @@ export class PositronNotebookEditor extends EditorPane {
const reactRenderer: PositronReactRenderer = this._positronReactRenderer ?? new PositronReactRenderer(this._parentDiv);

reactRenderer.render(
<NotebookInstanceProvider instance={notebookInstance}>
<ServicesProvider services={{
configurationService: this._configurationService,
instantiationService: this._instantiationService,
textModelResolverService: this._textModelResolverService,
webviewService: this._webviewService,
notebookWebviewService: this._notebookWebviewService,
webviewPreloadService: this._positronWebviewPreloadService,
commandService: this._commandService,
logService: this._logService,
openerService: this._openerService,
notificationService: this._notificationService,
sizeObservable: this._size,
scopedContextKeyProviderCallback: container => scopedContextKeyService.createScoped(container)
}}>
<PositronNotebookComponent />
</ServicesProvider>
</NotebookInstanceProvider>
<NotebookVisibilityProvider isVisible={this._isVisible}>
<NotebookInstanceProvider instance={notebookInstance}>
<ServicesProvider services={{
configurationService: this._configurationService,
instantiationService: this._instantiationService,
textModelResolverService: this._textModelResolverService,
webviewService: this._webviewService,
notebookWebviewService: this._notebookWebviewService,
webviewPreloadService: this._positronWebviewPreloadService,
commandService: this._commandService,
logService: this._logService,
openerService: this._openerService,
notificationService: this._notificationService,
sizeObservable: this._size,
scopedContextKeyProviderCallback: container => scopedContextKeyService.createScoped(container)
}}>
<PositronNotebookComponent />
</ServicesProvider>
</NotebookInstanceProvider>
</NotebookVisibilityProvider>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { Emitter } from '../../../../base/common/event.js';
import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js';
import { Disposable, DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js';
import { ISettableObservable, observableValue } from '../../../../base/common/observableInternal/base.js';
import { URI } from '../../../../base/common/uri.js';
import { localize } from '../../../../nls.js';
Expand Down Expand Up @@ -109,6 +109,16 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
*/
private _container: HTMLElement | undefined = undefined;

/**
* The DOM element that contains the cells for the notebook.
*/
private _cellsContainer: HTMLElement | undefined = undefined;

/**
* Disposables for the current cells container event listeners
*/
private readonly _cellsContainerListeners = this._register(new DisposableStore());

/**
* Callback to clear the keyboard navigation listeners. Set when listeners are attached.
*/
Expand Down Expand Up @@ -159,6 +169,12 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
private readonly _onDidChangeContent = this._register(new Emitter<void>());
readonly onDidChangeContent = this._onDidChangeContent.event;

/**
* Event emitter for when the cells container is scrolled
*/
private readonly _onDidScrollCellsContainer = this._register(new Emitter<void>());
readonly onDidScrollCellsContainer = this._onDidScrollCellsContainer.event;

// =============================================================================================
// #region Public Properties

Expand All @@ -167,6 +183,59 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
*/
private _id: string;

/**
* The DOM element that contains the cells for the notebook.
*/
get cellsContainer(): HTMLElement | undefined {
return this._cellsContainer;
}

/**
* Sets the DOM element that contains the cells for the notebook.
* @param container The container element to set, or undefined to clear
*/
setCellsContainer(container: HTMLElement | undefined | null): void {
// Clean up any existing listeners
this._cellsContainerListeners.clear();

if (!container) { return; }

this._cellsContainer = container;

// Fire initial scroll event after a small delay to ensure layout has settled
const initialScrollTimeout = setTimeout(() => {
this._onDidScrollCellsContainer.fire();
}, 50);

// Set up scroll listener
const scrollListener = DOM.addDisposableListener(container, 'scroll', () => {
this._onDidScrollCellsContainer.fire();
});

// Set up mutation observer to watch for DOM changes
const observer = new MutationObserver(() => {
// Small delay to let the DOM changes settle
setTimeout(() => {
this._onDidScrollCellsContainer.fire();
}, 0);
});

observer.observe(container, {
childList: true,
subtree: true,
attributes: true,
attributeFilter: ['style', 'class']
});

// Add all the disposables to our store
this._cellsContainerListeners.add(toDisposable(() => clearTimeout(initialScrollTimeout)));
this._cellsContainerListeners.add(scrollListener);
this._cellsContainerListeners.add(toDisposable(() => observer.disconnect()));

// Fire initial scroll event
this._onDidScrollCellsContainer.fire();
}

/**
* User facing cells wrapped in an observerable for the UI to react to changes
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,76 @@
import * as React from 'react';
import { getWindow } from '../../../../../../base/browser/dom.js';
import { INotebookOutputWebview } from '../../../../positronOutputWebview/browser/notebookOutputWebviewService.js';
import { IWebviewElement } from '../../../../webview/browser/webview.js';
import { assertIsStandardPositronWebview } from '../../../../positronOutputWebview/browser/notebookOutputWebviewServiceImpl.js';
import { isHTMLOutputWebviewMessage } from '../../../../positronWebviewPreloads/browser/notebookOutputUtils.js';
import { useNotebookInstance } from '../../NotebookInstanceProvider.js';
import { IOverlayWebview } from '../../../../webview/browser/webview.js';
import { IDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js';
import { useNotebookVisibility } from '../../NotebookVisibilityContext.js';
import { Event } from '../../../../../../base/common/event.js';


export function useWebviewMount(webview: Promise<INotebookOutputWebview>) {
const [isLoading, setIsLoading] = React.useState(true);
const [error, setError] = React.useState<Error | null>(null);
const containerRef = React.useRef<HTMLDivElement>(null);
const notebookInstance = useNotebookInstance();
const visibilityObservable = useNotebookVisibility();

React.useEffect(() => {
const controller = new AbortController();
let webviewElement: IWebviewElement | undefined;
let webviewElement: IOverlayWebview | undefined;
let scrollDisposable: IDisposable | undefined;
let visibilityObserver: IDisposable | undefined;

/**
* Updates the layout of the webview element if both the webview and container are available
*/
function updateWebviewLayout() {
if (!webviewElement || !containerRef.current) { return; }
webviewElement.layoutWebviewOverElement(
containerRef.current,
undefined,
notebookInstance.cellsContainer
);
}

function claimWebview() {
if (!webviewElement || !containerRef.current) { return; }
webviewElement.claim(
containerRef,
getWindow(containerRef.current),
undefined
);
}

function releaseWebview() {
webviewElement?.release(containerRef)
}

async function mountWebview() {
const emptyDisposable = toDisposable(() => { });
try {
// If not visible, don't mount the webview
if (!visibilityObservable) {
return emptyDisposable;
}

const resolvedWebview = await webview;

if (controller.signal.aborted || !containerRef.current) {
return;
return emptyDisposable;
}

setIsLoading(false);
assertIsStandardPositronWebview(resolvedWebview);
webviewElement = resolvedWebview.webview;
webviewElement.mountTo(
containerRef.current,
getWindow(containerRef.current)
);

claimWebview();

// Initial layout
updateWebviewLayout();

// Update layout on scroll and visibility changes
scrollDisposable = notebookInstance.onDidScrollCellsContainer(updateWebviewLayout);

webviewElement.onMessage((x) => {
const { message } = x;
Expand All @@ -51,19 +92,33 @@ export function useWebviewMount(webview: Promise<INotebookOutputWebview>) {
containerRef.current.style.height = `${boundedHeight}px`;
});

return scrollDisposable;

} catch (err) {
setError(err instanceof Error ? err : new Error('Failed to mount webview'));
setIsLoading(false);
return emptyDisposable;
}
}


Event.fromObservable(visibilityObservable)((isVisible) => {
if (isVisible) {
claimWebview();
} else {
releaseWebview();
}
});

mountWebview();

return () => {
controller.abort();
webviewElement?.dispose();
releaseWebview();
scrollDisposable?.dispose();
visibilityObserver?.dispose();
};
}, [webview]);
}, [webview, notebookInstance, visibilityObservable]);

return { containerRef, isLoading, error };
}
Loading
Loading