E4DC Backport PR #15443: Fix search coming back in notebook and editor (#1… · jupyterlab/jupyterlab@dd68b9c · GitHub
[go: up one dir, main page]
Skip to content

Commit dd68b9c

Browse files
krassowskiparmentelatgithub-actions[bot]
authored
Backport PR #15443: Fix search coming back in notebook and editor (#15562)
* a test case to illustrate issue #14871 * cleanup unused code * rename outline into highlight as it sounds more proper * fix typo in variable name _unrenderedByHighligh → _unrenderedByHighlight * Fix search coming back in notebook and editor * Add missing return * Rename snapshots and limit screenshot to notebook area Remove no-op await locator line * Update Playwright Snapshots * Improve async test implementation to avoid early closing * Update Playwright Snapshots * Remove visual snapshots, use locator counts instead and revert spurious snapshot updates --------- Co-authored-by: Thierry Parmentelat <thierry.parmentelat@inria.fr> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> (cherry picked from commit 746ce2d)
1 parent e68754b commit dd68b9c

File tree

9 files changed

+222
-9
lines changed

9 files changed

+222
-9
lines changed

‎galata/test/jupyterlab/notebook-search-highlight.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Copyright (c) Jupyter Development Team.
2+
// Distributed under the terms of the Modified BSD License.
3+
import { expect, galata, test } from '@jupyterlab/galata';
4+
import * as path from 'path';
5+
6+
const TEST_FILENAME = 'search_highlight_notebook.ipynb';
7+
const TEST_NEEDLE = 'come';
8+
9+
test.use({ tmpPath: 'test-search' });
10+
11+
test.beforeAll(async ({ request, tmpPath }) => {
12+
const contents = galata.newContentsHelper(request);
13+
await contents.uploadFile(
14+
path.resolve(__dirname, `./notebooks/${TEST_FILENAME}`),
15+
`${tmpPath}/${TEST_FILENAME}`
16+
);
17+
});
18+
19+
test.beforeEach(async ({ page, tmpPath }) => {
20+
await page.notebook.openByPath(`${tmpPath}/${TEST_FILENAME}`);
21+
await page.notebook.activate(TEST_FILENAME);
22+
});
23+
24+
const HIGHLIGHTS_LOCATOR = '.cm-searching';
25+
26+
test('Open and close Search dialog, then add new code cell', async ({
27+
page
28+
}) => {
29+
// search for our needle
30+
await page.evaluate(async searchText => {
31+
await window.jupyterapp.commands.execute('documentsearch:start', {
32+
searchText
33+
});
34+
}, TEST_NEEDLE);
35+
36+
// wait for the search to complete
37+
await page.waitForSelector('text=1/21');
38+
expect(await page.locator(HIGHLIGHTS_LOCATOR).count()).toBeGreaterThanOrEqual(
39+
4
40+
);
41+
42+
// cancel search
43+
await page.keyboard.press('Escape');
44+
45+
// expect the highlights to have gone
46+
expect(await page.locator(HIGHLIGHTS_LOCATOR).count()).toEqual(0);
47+
48+
// insert a new code cell
49+
await page.evaluate(async () =>
50+
window.jupyterapp.commands.execute('notebook:insert-cell-below')
51+
);
52+
53+
// wait an arbitrary amount of extra time
54+
// and expect the highlights to be still gone
55+
// regression-testing against #14871
56+
await new Promise(resolve => setTimeout(resolve, 1000));
57+
expect(await page.locator(HIGHLIGHTS_LOCATOR).count()).toEqual(0);
58+
});

‎galata/test/jupyterlab/notebooks/search_highlight_notebook.ipynb‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
{
2+
"cells": [
3+
{
4+
"cell_type": "markdown",
5+
"id": "e86bd77e-7e48-4941-8cb9-72410d8256aa",
6+
"metadata": {},
7+
"source": [
8+
"# regular notebook (not myst)"
9+
]
10+
},
11+
{
12+
"cell_type": "markdown",
13+
"id": "d06cb0c2-483b-4781-b283-8212ea667823",
14+
"metadata": {},
15+
"source": [
16+
"## a subtitle\n",
17+
"\n",
18+
"Where does it come from?\n",
19+
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
20+
]
21+
},
22+
{
23+
"cell_type": "markdown",
24+
"id": "8541f40d-4f71-48fc-92d4-708063b476ec",
25+
"metadata": {},
26+
"source": [
27+
"## another one\n",
28+
"\n",
29+
"Where does it come from?\n",
30+
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
31+
]
32+
},
33+
{
34+
"cell_type": "markdown",
35+
"id": "d896a643-8998-48cf-8a4f-ec2965656e73",
36+
"metadata": {},
37+
"source": [
38+
"## yet another\n",
39+
"\n",
40+
"Where does it come from?\n",
41+
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
42+
]
43+
},
44+
{
45+
"cell_type": "markdown",
46+
"id": "a089ea8f-02b8-4b09-af17-aca38782b3c7",
47+
"metadata": {},
48+
"source": [
49+
"## let's change\n",
50+
"\n",
51+
"Where does it come from?\n",
52+
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
53+
]
54+
},
55+
{
56+
"cell_type": "markdown",
57+
"id": "6f9ad439-3dc0-410c-9d98-2a9356f839a3",
58+
"metadata": {},
59+
"source": [
60+
"## to a less predictible\n",
61+
"\n",
62+
"Where does it come from?\n",
63+
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
64+
]
65+
},
66+
{
67+
"cell_type": "markdown",
68+
"id": "a783fb23-b79c-40c7-b8fe-be597130a3ae",
69+
"metadata": {},
70+
"source": [
71+
"## pattern in picking names\n",
72+
"\n",
73+
"Where does it come from?\n",
74+
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
75+
]
76+
},
77+
{
78+
"cell_type": "markdown",
79+
"id": "4e2ae1be-078e-4bf6-974b-6ea6750a74a9",
80+
"metadata": {},
81+
"source": [
82+
"## the last one\n",
83+
"\n",
84+
"Where does it come from?\n",
85+
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
86+
]
87+
}
88+
],
89+
"metadata": {
90+
"kernelspec": {
91+
"display_name": "Python 3 (ipykernel)",
92+
"language": "python",
93+
"name": "python3"
94+
},
95+
"language_info": {
96+
"codemirror_mode": {
97+
"name": "ipython",
98+
"version": 3
99+
},
100+
"file_extension": ".py",
101+
"mimetype": "text/x-python",
102+
"name": "python",
103+
"nbconvert_exporter": "python",
104+
"pygments_lexer": "ipython3",
105+
"version": "3.11.5"
106+
}
107+
},
108+
"nbformat": 4,
109+
"nbformat_minor": 5
110+
}

‎packages/cells/src/searchprovider.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
326326
const cell = this.cell as MarkdownCell;
327327
if (cell.rendered && this.matchesCount > 0) {
328328
// Unrender the cell
329-
this._unrenderedByHighligh = true;
329+
this._unrenderedByHighlight = true;
330330
const waitForRendered = signalToPromise(cell.renderedChanged);
331331
cell.rendered = false;
332332
await waitForRendered;
@@ -347,7 +347,7 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
347347
const cell = this.cell as MarkdownCell;
348348
if (cell.rendered && this.matchesCount > 0) {
349349
// Unrender the cell if there are matches within the cell
350-
this._unrenderedByHighligh = true;
350+
this._unrenderedByHighlight = true;
351351
const waitForRendered = signalToPromise(cell.renderedChanged);
352352
cell.rendered = false;
353353
await waitForRendered;
@@ -397,10 +397,10 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
397397
* @param rendered New rendered value
398398
*/
399399
protected onRenderedChanged(cell: MarkdownCell, rendered: boolean): void {
400-
if (!this._unrenderedByHighligh) {
400+
if (!this._unrenderedByHighlight) {
401401
this.currentIndex = null;
402402
}
403-
this._unrenderedByHighligh = false;
403+
this._unrenderedByHighlight = false;
404404
if (this.isActive) {
405405
if (rendered) {
406406
void this.renderedProvider.startQuery(this.query);
@@ -413,7 +413,7 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
413413
}
414414

415415
protected renderedProvider: GenericSearchProvider;
416-
private _unrenderedByHighligh = false;
416+
private _unrenderedByHighlight = false;
417417
}
418418

419419
/**

‎packages/documentsearch-extension/src/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ const extension: JupyterFrontEndPlugin<ISearchProviderRegistry> = {
308308
if (!currentWidget) {
309309
return;
310310
}
311-
312311
searchViews.get(currentWidget.id)?.close();
313312
}
314313
});

‎packages/documentsearch/src/searchmodel.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class SearchDocumentModel
3838
}
3939
}
4040

41-
searchProvider.stateChanged.connect(this.refresh, this);
41+
searchProvider.stateChanged.connect(this._onProviderStateChanged, this);
4242

4343
this._searchDebouncer = new Debouncer(() => {
4444
this._updateSearch().catch(reason => {
@@ -235,7 +235,10 @@ export class SearchDocumentModel
235235
});
236236
}
237237

238-
this.searchProvider.stateChanged.disconnect(this.refresh, this);
238+
this.searchProvider.stateChanged.disconnect(
239+
this._onProviderStateChanged,
240+
this
241+
);
239242

240243
this._searchDebouncer.dispose();
241244
super.dispose();
@@ -245,6 +248,7 @@ export class SearchDocumentModel
245248
* End the query.
246249
*/
247250
async endQuery(): Promise<void> {
251+
this._searchActive = false;
248252
await this.searchProvider.endQuery();
249253
this.stateChanged.emit();
250254
}
@@ -338,6 +342,7 @@ export class SearchDocumentModel
338342
)
339343
: null;
340344
if (query) {
345+
this._searchActive = true;
341346
await this.searchProvider.startQuery(query, this._filters);
342347
// Emit state change as the index needs to be updated
343348
this.stateChanged.emit();
@@ -352,13 +357,20 @@ export class SearchDocumentModel
352357
}
353358
}
354359

360+
private _onProviderStateChanged() {
361+
if (this._searchActive) {
362+
this.refresh();
363+
}
364+
}
365+
355366
private _caseSensitive = false;
356367
private _disposed = new Signal<this, void>(this);
357368
private _parsingError = '';
358369
private _preserveCase = false;
359370
private _initialQuery = '';
360371
private _filters: IFilters = {};
361372
private _replaceText: string = '';
373+
private _searchActive = false;
362374
private _searchDebouncer: Debouncer;
363375
private _searchExpression = '';
364376
private _useRegex = false;

‎packages/fileeditor/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
"watch": "tsc -b --watch"
4141
},
4242
"dependencies": {
43+
"@jupyter/ydoc": "^1.1.1",
4344
"@jupyterlab/apputils": "^4.1.9",
4445
"@jupyterlab/codeeditor": "^4.0.9",
4546
"@jupyterlab/codemirror": "^4.0.9",

‎packages/fileeditor/src/searchprovider.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import { ITranslator } from '@jupyterlab/translation';
1313
import { Widget } from< E746 /span> '@lumino/widgets';
1414
import { FileEditor } from './widget';
15+
import { ISharedText, SourceChange } from '@jupyter/ydoc';
1516

1617
/**
1718
* Helper type
@@ -64,6 +65,7 @@ export class FileEditorSearchProvider
6465
query: RegExp,
6566
filters: IFilters | undefined
6667
): Promise<void> {
68+
this._searchActive = true;
6769
await super.startQuery(query, filters);
6870
await this.highlightNext(true, {
6971
from: 'selection-start',
@@ -72,6 +74,29 @@ export class FileEditorSearchProvider
7274
});
7375
}
7476

77+
/**
78+
* Stop the search and clean any UI elements.
79+
*/
80+
async endQuery(): Promise<void> {
81+
this._searchActive = false;
82+
await super.endQuery();
83+
}
84+
85+
/**
86+
* Callback on source change
87+
*
88+
* @param emitter Source of the change
89+
* @param changes Source change
90+
*/
91+
protected async onSharedModelChanged(
92+
emitter: ISharedText,
93+
changes: SourceChange
94+
): Promise<void> {
95+
if (this._searchActive) {
96+
return super.onSharedModelChanged(emitter, changes);
97+
}
98+
}
99+
75100
/**
76101
* Instantiate a search provider for the widget.
77102
*
@@ -116,4 +141,6 @@ export class FileEditorSearchProvider
116141
);
117142
return selection;
118143
}
144+
145+
private _searchActive = false;
119146
}

‎packages/notebook/src/searchprovider.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
347347
return;
348348
}
349349
await this.endQuery();
350+
this._searchActive = true;
350351
let cells = this.widget.content.widgets;
351352

352353
this._query = query;
@@ -415,6 +416,7 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
415416
})
416417
);
417418

419+
this._searchActive = false;
418420
this._searchProviders.length = 0;
419421
this._currentProviderIndex = null;
420422
}
@@ -541,7 +543,9 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
541543
this.widget.content.isSelectedOrActive(cell)
542544
)
543545
.then(() => {
544-
void cellSearchProvider.startQuery(this._query, this._filters);
546+
if (this._searchActive) {
547+
void cellSearchProvider.startQuery(this._query, this._filters);
548+
}
545549
});
546550
}
547551

@@ -903,4 +907,5 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
903907
> | null = null;
904908
private _selectionSearchMode: 'cells' | 'text' = 'cells';
905909
private _selectionLock: boolean = false;
910+
private _searchActive: boolean = false;
906911
}

‎yarn.lock‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3410,6 +3410,7 @@ __metadata:
34103410
version: 0.0.0-use.local
34113411
resolution: "@jupyterlab/fileeditor@workspace:packages/fileeditor"
34123412
dependencies:
3413+
"@jupyter/ydoc": ^1.1.1
34133414
"@jupyterlab/apputils": ^4.1.9
34143415
"@jupyterlab/codeeditor": ^4.0.9
34153416
"@jupyterlab/codemirror": ^4.0.9

0 commit comments

Comments
 (0)
0