-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ui): fix toolbar layout on file load #31
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
Changes from all commits
5393cb3
05f7a51
38d4300
15169a0
02c7ee0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ | |
| "coreutils", | ||
| "csstree", | ||
| "deepnote", | ||
| "exenv", | ||
| "ipynb", | ||
| "Jakubowski", | ||
| "jlpm", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| // Copyright (c) Deepnote | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // 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) | ||
| }) | ||
| }) | ||
| }) | ||
| ); | ||
| }); | ||
andyjakubowski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| it('should not call fromJSON if selected notebook is invalid', async () => { | ||
| const getMetadata = panel.context.model.getMetadata as jest.Mock; | ||
| getMetadata.mockReturnValue({ notebooks: {}, notebook_names: [] }); | ||
|
|
||
| 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'); | ||
| }); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| 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('-'); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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; | ||||||||||||||||||||||||||||||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard against null parent.
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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| render(): JSX.Element { | ||||||||||||||||||||||||||||||
| const deepnoteMetadata = this.panel.context.model.getMetadata('deepnote'); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -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' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.