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

Remove deregisterProvider from SearchProviderRegistry interface #6282

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/csvviewer-extension/src/index.ts
Expand Up @@ -139,7 +139,7 @@ function activateCsv(

addMenuEntries(mainMenu, tracker);
if (searchregistry) {
searchregistry.registerProvider('csv', CSVSearchProvider);
searchregistry.register('csv', CSVSearchProvider);
}
}

Expand Down Expand Up @@ -209,7 +209,7 @@ function activateTsv(

addMenuEntries(mainMenu, tracker);
if (searchregistry) {
searchregistry.registerProvider('tsv', CSVSearchProvider);
searchregistry.register('tsv', CSVSearchProvider);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/documentsearch-extension/package.json
Expand Up @@ -32,7 +32,8 @@
"@jupyterlab/application": "^1.0.0-alpha.6",
"@jupyterlab/apputils": "^1.0.0-alpha.6",
"@jupyterlab/documentsearch": "^1.0.0-alpha.7",
"@jupyterlab/mainmenu": "^1.0.0-alpha.6"
"@jupyterlab/mainmenu": "^1.0.0-alpha.6",
"@phosphor/widgets": "^1.6.0"
},
"devDependencies": {
"rimraf": "~2.6.2",
Expand Down
31 changes: 27 additions & 4 deletions packages/documentsearch-extension/src/index.ts
Expand Up @@ -16,6 +16,7 @@ import {
} from '@jupyterlab/documentsearch';

import { IMainMenu } from '@jupyterlab/mainmenu';
import { Widget } from '@phosphor/widgets';

const SEARCHABLE_CLASS = 'jp-mod-searchable';

Expand All @@ -28,15 +29,37 @@ const labShellWidgetListener: JupyterFrontEndPlugin<void> = {
labShell: ILabShell,
registry: ISearchProviderRegistry
) => {
// If a given widget is searchable, apply the searchable class.
// If it's not searchable, remove the class.
const transformWidgetSearchability = (widget: Widget) => {
if (!widget) {
return;
}
const providerForWidget = registry.getProviderForWidget(widget);
if (providerForWidget) {
widget.addClass(SEARCHABLE_CLASS);
}
if (!providerForWidget) {
widget.removeClass(SEARCHABLE_CLASS);
}
};

// Update searchability of the active widget when the registry
// changes, in case a provider for the current widget was added
// or removed
registry.changed.connect(() =>
transformWidgetSearchability(labShell.activeWidget)
);

// Apply the searchable class only to the active widget if it is actually
// searchable. Remove the searchable class from a widget when it's
// no longer active.
labShell.activeChanged.connect((_, args) => {
const oldWidget = args.oldValue;
const newWidget = args.newValue;
if (newWidget && registry.getProviderForWidget(newWidget) !== undefined) {
newWidget.addClass(SEARCHABLE_CLASS);
}
if (oldWidget) {
oldWidget.removeClass(SEARCHABLE_CLASS);
}
transformWidgetSearchability(args.newValue);
});
}
};
Expand Down
46 changes: 26 additions & 20 deletions packages/documentsearch/src/searchproviderregistry.ts
Expand Up @@ -6,6 +6,8 @@ import { NotebookSearchProvider } from './providers/notebooksearchprovider';

import { Token } from '@phosphor/coreutils';
import { Widget } from '@phosphor/widgets';
import { IDisposable, DisposableDelegate } from '@phosphor/disposable';
import { ISignal, Signal } from '@phosphor/signaling';

/* tslint:disable */
/**
Expand All @@ -21,16 +23,9 @@ export interface ISearchProviderRegistry {
* Add a provider to the registry.
*
* @param key - The provider key.
* @returns A disposable delegate that, when disposed, deregisters the given search provider
*/
registerProvider(key: string, provider: ISearchProviderConstructor): void;

/**
* Remove provider from registry.
*
* @param key - The provider key.
* @returns true if removed, false if key did not exist in map.
*/
deregisterProvider(key: string): boolean;
register(key: string, provider: ISearchProviderConstructor): IDisposable;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing doc for the return value?


/**
* Returns a matching provider for the widget.
Expand All @@ -39,6 +34,12 @@ export interface ISearchProviderRegistry {
* @returns the search provider, or undefined if none exists.
*/
getProviderForWidget(widget: any): ISearchProvider | undefined;

/**
* Signal that emits when a new search provider has been registered
* or removed.
*/
changed: ISignal<ISearchProviderRegistry, void>;
}

export class SearchProviderRegistry implements ISearchProviderRegistry {
Expand All @@ -57,19 +58,15 @@ export class SearchProviderRegistry implements ISearchProviderRegistry {
* Add a provider to the registry.
*
* @param key - The provider key.
* @returns A disposable delegate that, when disposed, deregisters the given search provider
*/
registerProvider(key: string, provider: ISearchProviderConstructor): void {
register(key: string, provider: ISearchProviderConstructor): IDisposable {
this._customProviders.set(key, provider);
}

/**
* Remove provider from registry.
*
* @param key - The provider key.
* @returns true if removed, false if key did not exist in map.
*/
deregisterProvider(key: string): boolean {
return this._customProviders.delete(key);
this._changed.emit();
return new DisposableDelegate(() => {
this._customProviders.delete(key);
this._changed.emit();
});
}

/**
Expand All @@ -85,6 +82,14 @@ export class SearchProviderRegistry implements ISearchProviderRegistry {
);
}

/**
* Signal that emits when a new search provider has been registered
* or removed.
*/
get changed(): ISignal<this, void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docstring?

return this._changed;
}

private _registerDefaultProviders(
key: string,
provider: ISearchProviderConstructor
Expand All @@ -106,6 +111,7 @@ export class SearchProviderRegistry implements ISearchProviderRegistry {
return undefined;
}

private _changed = new Signal<this, void>(this);
private _defaultProviders: Private.ProviderMap = new Map<
string,
ISearchProviderConstructor
Expand Down