From b433701e625f2ca0d93737c7ea1a9ecf3551544a Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Mon, 18 Nov 2024 11:36:58 +0100 Subject: [PATCH 1/5] fixed open path error with timeline --- .../docprovider-extension/src/filebrowser.ts | 72 +++++++++++-------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/packages/docprovider-extension/src/filebrowser.ts b/packages/docprovider-extension/src/filebrowser.ts index bcea4f9a..d68ecd90 100644 --- a/packages/docprovider-extension/src/filebrowser.ts +++ b/packages/docprovider-extension/src/filebrowser.ts @@ -158,35 +158,37 @@ export const statusBarTimeline: JupyterFrontEndPlugin = { documentPath: string, documentId: string ) => { - if (drive) { - // Remove 'RTC:' from document path - documentPath = documentPath.slice(drive.name.length + 1); - // Dispose of the previous timelineWidget if it exists - if (timelineWidget) { - timelineWidget.dispose(); - timelineWidget = null; - } + if (documentId && documentPath.split(':')[0] === 'RTC') { + if (drive) { + // Remove 'RTC:' from document path + documentPath = documentPath.slice(drive.name.length + 1); + // Dispose of the previous timelineWidget if it exists + if (timelineWidget) { + timelineWidget.dispose(); + timelineWidget = null; + } - const [format, type] = documentId.split(':'); - const provider = drive.providers.get( - `${format}:${type}:${documentPath}` - ) as unknown as IForkProvider; - const fullPath = URLExt.join( - app.serviceManager.serverSettings.baseUrl, - DOCUMENT_TIMELINE_URL, - documentPath - ); + const [format, type] = documentId.split(':'); + const provider = drive.providers.get( + `${format}:${type}:${documentPath}` + ) as unknown as IForkProvider; + const fullPath = URLExt.join( + app.serviceManager.serverSettings.baseUrl, + DOCUMENT_TIMELINE_URL, + documentPath + ); - timelineWidget = new TimelineWidget( - fullPath, - provider, - provider.contentType, - provider.format - ); + timelineWidget = new TimelineWidget( + fullPath, + provider, + provider.contentType, + provider.format + ); - const elt = document.getElementById('jp-slider-status-bar'); - if (elt && !timelineWidget.isAttached) { - Widget.attach(timelineWidget, elt); + const elt = document.getElementById('jp-slider-status-bar'); + if (elt && !timelineWidget.isAttached) { + Widget.attach(timelineWidget, elt); + } } } }; @@ -222,8 +224,22 @@ export const statusBarTimeline: JupyterFrontEndPlugin = { align: 'left', rank: 4, isActive: () => { - const currentWidget = app.shell.currentWidget; - return !!currentWidget && 'context' in currentWidget; + const currentWidget = app.shell + .currentWidget as DocumentWidget | null; + + if ( + currentWidget && + currentWidget.context && + typeof currentWidget.context.path === 'string' + ) { + const documentPath = currentWidget.context.path; + const documentId = + currentWidget.context.model.sharedModel.getState( + 'document_id' + ) as string; + return !!documentId && documentPath.split(':')[0] === 'RTC'; + } + return false; } }); } From 2d26b9ea705b2ed05021e2d18efd7c6f770af92d Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Tue, 19 Nov 2024 13:00:10 +0100 Subject: [PATCH 2/5] test added for timeline --- ui-tests/tests/timeline-slider.spec.ts | 52 ++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 ui-tests/tests/timeline-slider.spec.ts diff --git a/ui-tests/tests/timeline-slider.spec.ts b/ui-tests/tests/timeline-slider.spec.ts new file mode 100644 index 00000000..56b97ffc --- /dev/null +++ b/ui-tests/tests/timeline-slider.spec.ts @@ -0,0 +1,52 @@ +/* + * Copyright (c) Jupyter Development Team. + * Distributed under the terms of the Modified BSD License. + */ + +import { expect, test } from '@jupyterlab/galata'; + +test.describe('Timeline Slider', () => { + test('should open a notebook and interact with the timeline slider', async ({ page, tmpPath }) => { + await page.notebook.createNew(); + await page.notebook.save(); + await page.notebook.close(); + + // Step 1: Click on the "File" menu + await page.click('text=File'); + await page.waitForSelector('.lm-Menu-itemLabel:text("Open from Path…")', { state: 'visible' }); + + // Step 2: Select "Open from Path..." + await page.click('.lm-Menu-itemLabel:text("Open from Path…")'); + + // Step 3: Enter the path for the notebook + await page.waitForSelector('input[placeholder="/path/relative/to/jlab/root"]'); + await page.fill('input[placeholder="/path/relative/to/jlab/root"]', `${tmpPath}/Untitled.ipynb`); + await page.click('.jp-Dialog-buttonLabel:text("Open")'); + + // Step 4: Wait for the notebook to load + await page.waitForSelector('.jp-Notebook', { state: 'visible' }); + + // Step 5: Click the history icon in the status bar + const historyIcon = page.locator('#jp-slider-status-bar'); + await expect(historyIcon).toBeVisible(); + await historyIcon.click(); + + // Step 6: Assert that no error is thrown + const consoleMessages: any[] = []; + page.on('console', (msg) => { + if (msg.type() === 'error') { + consoleMessages.push(msg.text()); + } + }); + + // Give some time for potential errors to appear + await page.waitForTimeout(1000); + + // Ensure there are no errors in the console + expect(consoleMessages).toHaveLength(0); + + // Clean up by closing the notebook + await page.click('text=File'); + await page.click('text=Close All Tabs'); + }); +}); From aa0277f3399b1945a81718ce5c058f8aeb89b11e Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Tue, 19 Nov 2024 13:52:21 +0100 Subject: [PATCH 3/5] modified test --- ui-tests/tests/timeline-slider.spec.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/ui-tests/tests/timeline-slider.spec.ts b/ui-tests/tests/timeline-slider.spec.ts index 56b97ffc..973ea428 100644 --- a/ui-tests/tests/timeline-slider.spec.ts +++ b/ui-tests/tests/timeline-slider.spec.ts @@ -6,7 +6,7 @@ import { expect, test } from '@jupyterlab/galata'; test.describe('Timeline Slider', () => { - test('should open a notebook and interact with the timeline slider', async ({ page, tmpPath }) => { + test('should not show errors when opening file from path', async ({ page, tmpPath }) => { await page.notebook.createNew(); await page.notebook.save(); await page.notebook.close(); @@ -23,13 +23,8 @@ test.describe('Timeline Slider', () => { await page.fill('input[placeholder="/path/relative/to/jlab/root"]', `${tmpPath}/Untitled.ipynb`); await page.click('.jp-Dialog-buttonLabel:text("Open")'); - // Step 4: Wait for the notebook to load await page.waitForSelector('.jp-Notebook', { state: 'visible' }); - // Step 5: Click the history icon in the status bar - const historyIcon = page.locator('#jp-slider-status-bar'); - await expect(historyIcon).toBeVisible(); - await historyIcon.click(); // Step 6: Assert that no error is thrown const consoleMessages: any[] = []; @@ -38,15 +33,10 @@ test.describe('Timeline Slider', () => { consoleMessages.push(msg.text()); } }); - - // Give some time for potential errors to appear await page.waitForTimeout(1000); // Ensure there are no errors in the console expect(consoleMessages).toHaveLength(0); - // Clean up by closing the notebook - await page.click('text=File'); - await page.click('text=Close All Tabs'); }); }); From 1f4ad46c1aa5dbcfb896eaadfc77f46338f65ae1 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Tue, 19 Nov 2024 15:27:28 +0100 Subject: [PATCH 4/5] added test that fails without PR but passes with it --- ui-tests/tests/timeline-slider.spec.ts | 47 ++++++++++++-------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/ui-tests/tests/timeline-slider.spec.ts b/ui-tests/tests/timeline-slider.spec.ts index 973ea428..330f42e6 100644 --- a/ui-tests/tests/timeline-slider.spec.ts +++ b/ui-tests/tests/timeline-slider.spec.ts @@ -4,39 +4,36 @@ */ import { expect, test } from '@jupyterlab/galata'; +import { Page } from '@playwright/test'; test.describe('Timeline Slider', () => { - test('should not show errors when opening file from path', async ({ page, tmpPath }) => { - await page.notebook.createNew(); - await page.notebook.save(); - await page.notebook.close(); - // Step 1: Click on the "File" menu - await page.click('text=File'); - await page.waitForSelector('.lm-Menu-itemLabel:text("Open from Path…")', { state: 'visible' }); + async function capturePageErrors(page: Page) { + const pageErrors: string[] = []; + page.on('pageerror', (error) => pageErrors.push(error.message)); + return pageErrors; + } - // Step 2: Select "Open from Path..." + async function openNotebook(page: Page, notebookPath: string) { + await page.click('text=File'); await page.click('.lm-Menu-itemLabel:text("Open from Path…")'); + await page.fill( + 'input[placeholder="/path/relative/to/jlab/root"]', + notebookPath + ); + await page.click('.jp-Dialog-buttonLabel:text("Open")'); + await page.waitForSelector('.jp-Notebook', { state: 'visible' }); + } - // Step 3: Enter the path for the notebook - await page.waitForSelector('input[placeholder="/path/relative/to/jlab/root"]'); - await page.fill('input[placeholder="/path/relative/to/jlab/root"]', `${tmpPath}/Untitled.ipynb`); - await page.click('.jp-Dialog-buttonLabel:text("Open")'); + test('should fail if there are console errors', async ({ page, tmpPath }) => { + const pageErrors = await capturePageErrors(page); - await page.waitForSelector('.jp-Notebook', { state: 'visible' }); - - - // Step 6: Assert that no error is thrown - const consoleMessages: any[] = []; - page.on('console', (msg) => { - if (msg.type() === 'error') { - consoleMessages.push(msg.text()); - } - }); - await page.waitForTimeout(1000); + await page.notebook.createNew(); + await page.notebook.save(); + await page.notebook.close(); - // Ensure there are no errors in the console - expect(consoleMessages).toHaveLength(0); + await openNotebook(page, `${tmpPath}/Untitled.ipynb`); + expect(pageErrors).toHaveLength(0); }); }); From 7b44441f1fe4c6af9751119952129b66a2b49fc0 Mon Sep 17 00:00:00 2001 From: Meriem-BenIsmail Date: Tue, 19 Nov 2024 15:35:13 +0100 Subject: [PATCH 5/5] added ui test that passes with PR but fails without it. --- ui-tests/tests/timeline-slider.spec.ts | 36 +++++++++++++------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/ui-tests/tests/timeline-slider.spec.ts b/ui-tests/tests/timeline-slider.spec.ts index 330f42e6..4a052787 100644 --- a/ui-tests/tests/timeline-slider.spec.ts +++ b/ui-tests/tests/timeline-slider.spec.ts @@ -6,24 +6,24 @@ import { expect, test } from '@jupyterlab/galata'; import { Page } from '@playwright/test'; -test.describe('Timeline Slider', () => { - - async function capturePageErrors(page: Page) { - const pageErrors: string[] = []; - page.on('pageerror', (error) => pageErrors.push(error.message)); - return pageErrors; - } - - async function openNotebook(page: Page, notebookPath: string) { - await page.click('text=File'); - await page.click('.lm-Menu-itemLabel:text("Open from Path…")'); - await page.fill( - 'input[placeholder="/path/relative/to/jlab/root"]', - notebookPath - ); - await page.click('.jp-Dialog-buttonLabel:text("Open")'); - await page.waitForSelector('.jp-Notebook', { state: 'visible' }); - } +async function capturePageErrors(page: Page) { + const pageErrors: string[] = []; + page.on('pageerror', (error) => pageErrors.push(error.message)); + return pageErrors; +} + +async function openNotebook(page: Page, notebookPath: string) { + await page.click('text=File'); + await page.click('.lm-Menu-itemLabel:text("Open from Path…")'); + await page.fill( + 'input[placeholder="/path/relative/to/jlab/root"]', + notebookPath + ); + await page.click('.jp-Dialog-buttonLabel:text("Open")'); + await page.waitForSelector('.jp-Notebook', { state: 'visible' }); +} + +test.describe('Open from Path', () => { test('should fail if there are console errors', async ({ page, tmpPath }) => { const pageErrors = await capturePageErrors(page);