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

Bump to JupyterLab 4.1.0a4 #7166

Closed
wants to merge 5 commits into from
Closed

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Nov 30, 2023

Update to the freshly released 4.1.0a4: https://github.com/jupyterlab/jupyterlab/releases/tag/v4.1.0a4

Copy link
Contributor

Binder 👈 Launch a Binder on branch jtpio/notebook/update-lab-410a4

@jtpio jtpio added this to the 7.1 milestone Nov 30, 2023
@jtpio
Copy link
Member Author

jtpio commented Nov 30, 2023

Ouch a couple of breakages in that one it seems...

Breaking change in Galata?

image

Icons are now placed on the left?

image

Some extra label:

image

@krassowski
Copy link
Member

I believe this is mostly jupyterlab/jupyterlab#15021 CC @brichet

@brichet
Copy link
Collaborator

brichet commented Nov 30, 2023

I believe this is mostly jupyterlab/jupyterlab#15021 CC @brichet

Yes probably, I'll look at it @jtpio

@jtpio
Copy link
Member Author

jtpio commented Nov 30, 2023

Bumping the latest @jupyterlab/galata helps get past the await page.filebrowser.refresh() error, which makes sense as the selector was changed in jupyterlab/jupyterlab#15021.

Now some tests are failing when trying to do file manipulations via the file browser toolbar:

test('Select one folder', async ({ page, tmpPath }) => {
await page.filebrowser.refresh();
await page.getByText('folder1').last().click();
const toolbar = page.getByRole('navigation');
expect(toolbar.getByText('Rename')).toBeVisible();
expect(toolbar.getByText('Delete')).toBeVisible();
});
test('Select one file', async ({ page, tmpPath }) => {
await page.filebrowser.refresh();
await page.getByText('empty.ipynb').last().click();
const toolbar = page.getByRole('navigation');
['Rename', 'Delete', 'Open', 'Download', 'Delete'].forEach(async (text) => {
expect(toolbar.getByText(text)).toBeVisible();
});
});
test('Select files and folders', async ({ page, tmpPath }) => {
await page.filebrowser.refresh();
await page.keyboard.down('Control');
await page.getByText('folder1').last().click();
await page.getByText('folder2').last().click();
await page.getByText('empty.ipynb').last().click();
const toolbar = page.getByRole('navigation');
expect(toolbar.getByText('Rename')).toBeHidden();
expect(toolbar.getByText('Open')).toBeHidden();
expect(toolbar.getByText('Delete')).toBeVisible();
});
test('Select files and open', async ({ page, tmpPath }) => {
// upload an additional notebook
await page.contents.uploadFile(
path.resolve(__dirname, './notebooks/simple.ipynb'),
`${tmpPath}/simple.ipynb`
);
await page.filebrowser.refresh();
await page.keyboard.down('Control');
await page.getByText('simple.ipynb').last().click();
await page.getByText('empty.ipynb').last().click();
const toolbar = page.getByRole('navigation');
const [nb1, nb2] = await Promise.all([
page.waitForEvent('popup'),
page.waitForEvent('popup'),
toolbar.getByText('Open').last().click(),
]);
await nb1.waitForLoadState();
await nb1.close();
await nb2.waitForLoadState();
await nb2.close();
});

d3efbeff9e3336758817295039be2073bc8d89e4.webm

@jtpio
Copy link
Member Author

jtpio commented Dec 1, 2023

So the remaining issue now are:

Refresh File List button label

Probably related to the switch to jupyter-ui-toolkit?

image

Left icon for the interface switcher toolbar button

If this is the way to go or if there are reasons to do this w.r.t accessibility then it's fine to leave it this way (we'll just update the reference snapshots)

image

Some CSS inconsistencies on the file action buttons (wrong height)

image


Some other tests are failing due to snapshot comparison because of some minor style update, but we can just update the reference snapshots.

@brichet
Copy link
Collaborator

brichet commented Dec 1, 2023

Icons are now placed on the left?

The web components embed the content into shadow DOM, which change the CSS selectors.

To fix it for the Switch to jupyterlab icon, the selector .jp-nb-interface-switcher-button > .jp-ToolbarButtonComponent must be change with .jp-nb-interface-switcher-button > .jp-ToolbarButtonComponent::part(content) at

.jp-nb-interface-switcher-button > .jp-ToolbarButtonComponent {

@brichet
Copy link
Collaborator

brichet commented Dec 1, 2023

Refresh File List button label

Also a selector to change at

button[data-command='filebrowser:refresh'] .jp-ToolbarButtonComponent-label {

to jp-button[data-command='filebrowser:refresh'] .jp-ToolbarButtonComponent-label

@brichet
Copy link
Collaborator

brichet commented Dec 1, 2023

There will probably be some other side effect, like the padding left of the interface switcher icon

.jp-nb-interface-switcher-button
> .jp-ToolbarButtonComponent
> .jp-ToolbarButtonComponent-icon {
padding-left: 3px;
}

which should be

.jp-nb-interface-switcher-button
  > .jp-ToolbarButtonComponent
  > svg {
  padding-left: 3px;
}

@brichet
Copy link
Collaborator

brichet commented Dec 1, 2023

Some CSS inconsistencies on the file action buttons (wrong height)

That one is more tricky, since it's not only about style but also about accessibility.

Currently these FileActions buttons are included in a widget, adding a dom node to the main toolbar.
Therefore, the buttons cannot be focused like the other toolbar button.

We should include these buttons as direct child of the filebrowser toolbar.

@jtpio
Copy link
Member Author

jtpio commented Dec 1, 2023

OK that makes sense, thanks @brichet for providing these details 👍

@krassowski
Copy link
Member

There is also #7137 about the interface switcher that was created during review.

@brichet would you have time to add the example style changes that extension authors may need to make to the extension migration guide section? https://jupyterlab.readthedocs.io/en/latest/extension/extension_migration.html#use-of-ui-toolkit-for-toolbar-and-toolbarbuttoncomponent

@jtpio
Copy link
Member Author

jtpio commented Dec 4, 2023

Closing in favor of #7172.

@jtpio jtpio closed this Dec 4, 2023
@jtpio jtpio deleted the update-lab-410a4 branch January 4, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants