Skip to content

Commit

Permalink
Fix disappearing cells (heal offsets after updating estimated sizes) (j…
Browse files Browse the repository at this point in the history
…upyterlab#17000)

* Add a galata test against jupyterlab#16978

* Fix offsets of items after updating estimated sizes

* Do not touch outputs if cell model is disposed

* Rename `h` to `notebook`
Please enter the commit message for your changes. Lines starting
  • Loading branch information
krassowski authored Dec 2, 2024
1 parent 3734d46 commit 5cd092d
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 3 deletions.
49 changes: 49 additions & 0 deletions galata/test/jupyterlab/windowed-notebook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,55 @@ test('should remove all cells including hidden outputs artifacts', async ({
expect(found).toEqual(false);
});

test('should display cells below on scrolling after inserting a cell on top', async ({
page,
tmpPath
}) => {
// Regression test against "disappearing cells" issue:
// https://github.com/jupyterlab/jupyterlab/issues/16978
await page.notebook.openByPath(`${tmpPath}/${fileName}`);

const notebook = await page.notebook.getNotebookInPanelLocator()!;
const firstCell = notebook.locator('.jp-Cell[data-windowed-list-index="1"]');
const lastCell = notebook.locator('.jp-Cell[data-windowed-list-index="18"]');
await firstCell.waitFor();

const bbox = await notebook.boundingBox();
await page.mouse.move(bbox!.x, bbox!.y);

// Needs to be two separate mouse wheel events.
await page.mouse.wheel(0, 3000);
await page.mouse.wheel(0, 3000);

// Scroll down to reveal last cell to ensure these all items have been measured...
await Promise.all([
firstCell.waitFor({ state: 'hidden' }),
lastCell.waitFor()
]);

await page.mouse.wheel(0, -3000);
await page.mouse.wheel(0, -3000);

// ...then scroll back up and select first cell.
await Promise.all([
lastCell.waitFor({ state: 'hidden' }),
firstCell.waitFor()
]);
await page.notebook.selectCells(0);

// Insert cell below.
await page.keyboard.press('b');
await page.mouse.wheel(0, 3000);
await page.mouse.wheel(0, 3000);

// Scroll down again.
await Promise.all([
firstCell.waitFor({ state: 'hidden' }),
lastCell.waitFor()
]);
await expect(lastCell).toBeVisible();
});

test('should center on next cell after rendering markdown cell and advancing', async ({
page,
tmpPath
Expand Down
2 changes: 1 addition & 1 deletion packages/notebook/src/windowing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class NotebookViewModel extends WindowedListModel {

const nLines = model.sharedModel.getSource().split('\n').length;
let outputsLines = 0;
if (model instanceof CodeCellModel) {
if (model instanceof CodeCellModel && !model.isDisposed) {
for (let outputIdx = 0; outputIdx < model.outputs.length; outputIdx++) {
const output = model.outputs.get(outputIdx);
const data = output.data['text/plain'];
Expand Down
9 changes: 7 additions & 2 deletions packages/ui-components/src/components/windowedlist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -663,8 +663,13 @@ export abstract class WindowedListModel implements WindowedList.IModel {

offset += size;
}

this._measuredAllUntilIndex = index;
// Because the loop above updates estimated sizes,
// we need to fix (heal) offsets of the remaining items.
for (let i = index + 1; i < this._widgetSizers.length; i++) {
const sizer = this._widgetSizers[i];
const previous = this._widgetSizers[i - 1];
sizer.offset = previous.offset + previous.size;
}
}

for (let i = 0; i <= this._measuredAllUntilIndex; i++) {
Expand Down

0 comments on commit 5cd092d

Please sign in to comment.