Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"coreutils",
"csstree",
"deepnote",
"exenv",
"ipynb",
"Jakubowski",
"jlpm",
Expand Down
5 changes: 3 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ const jestJupyterLab = require('@jupyterlab/testutils/lib/jest-config');

const esModules = [
'@codemirror',
'@jupyter/ydoc',
'@jupyterlab/',
'@jupyter',
'@microsoft',
'exenv-es6',
'lib0',
'nanoid',
'vscode-ws-jsonrpc',
Expand Down
111 changes: 111 additions & 0 deletions src/__tests__/NotebookPicker.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright (c) Deepnote
// Distributed under the terms of the Modified BSD License.

import { NotebookPicker } from '../../src/components/NotebookPicker';
import { framePromise } from '@jupyterlab/testing';
import { NotebookPanel } from '@jupyterlab/notebook';
import { INotebookModel } from '@jupyterlab/notebook';
import { Widget } from '@lumino/widgets';
import { simulate } from 'simulate-event';

describe('NotebookPicker', () => {
let panel: NotebookPanel;
let model: INotebookModel;

beforeEach(async () => {
// Mock model + metadata
model = {
fromJSON: jest.fn(),
get cells() {
return [];
},
dirty: true
} as any;

panel = {
context: {
ready: Promise.resolve(),
model: {
getMetadata: jest.fn().mockReturnValue({
notebooks: {
nb1: { id: 'nb1', name: 'nb1', cells: [{ source: 'code' }] },
nb2: { id: 'nb2', name: 'nb2', cells: [] }
},
notebook_names: ['nb1', 'nb2']
})
}
},
model
} as any;

// Attach to DOM
const widget = new NotebookPicker(panel);
// Override onAfterAttach to avoid errors from this.parent being null
(widget as any).onAfterAttach = jest.fn();
Widget.attach(widget, document.body);
await framePromise();
});

afterEach(() => {
document.body.innerHTML = '';
jest.restoreAllMocks();
});

it('should render a select element', async () => {
await framePromise(); // wait for rendering
const select = document.querySelector('select') as HTMLSelectElement;
expect(select).not.toBeNull();
expect(select.options.length).toBe(2);
expect(select.options[0] && select.options[0].value).toBe('nb1');
});

it('should call fromJSON when selecting a notebook', async () => {
const select = document.querySelector('select') as HTMLSelectElement;
simulate(select, 'change', { target: { value: 'nb2' } });
await framePromise();
expect(model.fromJSON).toHaveBeenCalledWith(
expect.objectContaining({
cells: expect.any(Array),
metadata: expect.objectContaining({
deepnote: expect.objectContaining({
notebooks: expect.any(Object)
})
})
})
);
});

it('should not call fromJSON if selected notebook is invalid', async () => {
const getMetadata = panel.context.model.getMetadata as jest.Mock;
getMetadata.mockReturnValue({ notebooks: {} });

const select = document.querySelector('select') as HTMLSelectElement;
simulate(select, 'change', { target: { value: 'nonexistent' } });
await framePromise();
expect(model.fromJSON).not.toHaveBeenCalled();
});

it('should update UI after selection', async () => {
const select = document.querySelector('select') as HTMLSelectElement;
select.value = 'nb2';
simulate(select, 'change');
await framePromise();
expect(select.value).toBe('nb2');
});

it('should handle empty metadata gracefully', async () => {
const getMetadata = panel.context.model.getMetadata as jest.Mock;
getMetadata.mockReturnValue({ notebooks: {}, notebook_names: [] });

document.body.innerHTML = '';
const widget = new NotebookPicker(panel);
// Override onAfterAttach to avoid errors from this.parent being null
(widget as any).onAfterAttach = jest.fn();
Widget.attach(widget, document.body);
await framePromise();

const select = document.querySelector('select') as HTMLSelectElement;
expect(select.options.length).toBeGreaterThanOrEqual(1);
expect(select.options[0] && select.options[0].value).toBe('-');
});
});
11 changes: 10 additions & 1 deletion src/components/NotebookPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { ReactWidget } from '@jupyterlab/apputils';
import { NotebookPanel } from '@jupyterlab/notebook';
import { HTMLSelect } from '@jupyterlab/ui-components';
import { deepnoteMetadataSchema } from '../types';
import { Widget } from '@lumino/widgets';
import { Message, MessageLoop } from '@lumino/messaging';

export class NotebookPicker extends ReactWidget {
private selected: string | null = null;
Expand Down Expand Up @@ -63,6 +65,13 @@ export class NotebookPicker extends ReactWidget {
this.update();
};

protected onAfterAttach(msg: Message): void {
super.onAfterAttach(msg);
requestAnimationFrame(() => {
MessageLoop.sendMessage(this.parent!, Widget.ResizeMessage.UnknownSize);
});
}
Comment on lines +68 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard against null parent.

this.parent! will throw if no parent exists at attach time. Add a null check.

 protected onAfterAttach(msg: Message): void {
   super.onAfterAttach(msg);
   requestAnimationFrame(() => {
-    MessageLoop.sendMessage(this.parent!, Widget.ResizeMessage.UnknownSize);
+    if (this.parent) {
+      MessageLoop.sendMessage(this.parent, Widget.ResizeMessage.UnknownSize);
+    }
   });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected onAfterAttach(msg: Message): void {
super.onAfterAttach(msg);
requestAnimationFrame(() => {
MessageLoop.sendMessage(this.parent!, Widget.ResizeMessage.UnknownSize);
});
}
protected onAfterAttach(msg: Message): void {
super.onAfterAttach(msg);
requestAnimationFrame(() => {
if (this.parent) {
MessageLoop.sendMessage(this.parent, Widget.ResizeMessage.UnknownSize);
}
});
}
🤖 Prompt for AI Agents
In src/components/NotebookPicker.tsx around lines 68 to 73, the code uses a
non-null assertion this.parent! when sending a resize message which will throw
if parent is null; update the method to check that this.parent is non-null
before calling MessageLoop.sendMessage (e.g., if (this.parent) {
MessageLoop.sendMessage(this.parent, Widget.ResizeMessage.UnknownSize); }) and
remove the non-null assertion so the resize is only sent when a parent exists.


render(): JSX.Element {
const deepnoteMetadata = this.panel.context.model.getMetadata('deepnote');

Expand All @@ -81,7 +90,7 @@ export class NotebookPicker extends ReactWidget {
aria-label="Select active notebook"
title="Select active notebook"
style={{
maxWidth: '120px',
width: '120px',
textOverflow: 'ellipsis',
whiteSpace: 'nowrap',
overflow: 'hidden'
Expand Down