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

Support dynamic toolbar definition #11963

Merged
merged 5 commits into from Feb 17, 2022
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
193 changes: 99 additions & 94 deletions packages/apputils/src/toolbar/factory.ts
@@ -1,7 +1,7 @@
import { IObservableList, ObservableList } from '@jupyterlab/observables';
import { ISettingRegistry, SettingRegistry } from '@jupyterlab/settingregistry';
import { ITranslator, TranslationBundle } from '@jupyterlab/translation';
import { findIndex, toArray } from '@lumino/algorithm';
import { toArray } from '@lumino/algorithm';
import { JSONExt, PartialJSONObject } from '@lumino/coreutils';
import { Widget } from '@lumino/widgets';
import { Dialog, showDialog } from '../dialog';
Expand Down Expand Up @@ -38,22 +38,26 @@ async function displayInformation(trans: TranslationBundle): Promise<void> {
}

/**
* Accumulate the toolbar definition from all settings and set the default value from it.
* Set the toolbar definition by accumulating all settings definition.
*
* The list will be populated only with the enabled items.
*
* @param toolbarItems Observable list to populate
* @param registry Application settings registry
* @param factoryName Widget factory name that needs a toolbar
* @param pluginId Settings plugin id
* @param translator Translator object
* @param propertyId Property holding the toolbar definition in the settings; default 'toolbar'
* @returns List of toolbar items
*/
async function getToolbarItems(
async function setToolbarItems(
toolbarItems: IObservableList<ISettingRegistry.IToolbarItem>,
registry: ISettingRegistry,
factoryName: string,
pluginId: string,
translator: ITranslator,
propertyId: string = 'toolbar'
): Promise<IObservableList<ISettingRegistry.IToolbarItem>> {
): Promise<void> {
const trans = translator.load('jupyterlab');
let canonical: ISettingRegistry.ISchema | null;
let loaded: { [name: string]: ISettingRegistry.IToolbarItem[] } = {};
Expand Down Expand Up @@ -93,13 +97,11 @@ async function getToolbarItems(
pluginDefaults,
schema.properties![propertyId].default as any[],
true
)!
// flatten one level
.sort(
(a, b) =>
(a.rank ?? DEFAULT_TOOLBAR_ITEM_RANK) -
(b.rank ?? DEFAULT_TOOLBAR_ITEM_RANK)
);
)!.sort(
(a, b) =>
(a.rank ?? DEFAULT_TOOLBAR_ITEM_RANK) -
(b.rank ?? DEFAULT_TOOLBAR_ITEM_RANK)
);
}

// Transform the plugin object to return different schema than the default.
Expand All @@ -119,12 +121,17 @@ async function getToolbarItems(
// Overrides the value with using the aggregated default for the toolbar property
user[propertyId] =
(plugin.data.user[propertyId] as ISettingRegistry.IToolbarItem[]) ?? [];
composite[propertyId] =
composite[propertyId] = (
SettingRegistry.reconcileToolbarItems(
defaults as ISettingRegistry.IToolbarItem[],
user[propertyId] as ISettingRegistry.IToolbarItem[],
false
) ?? [];
) ?? []
).sort(
(a, b) =>
(a.rank ?? DEFAULT_TOOLBAR_ITEM_RANK) -
(b.rank ?? DEFAULT_TOOLBAR_ITEM_RANK)
);

plugin.data = { composite, user };

Expand Down Expand Up @@ -153,21 +160,12 @@ async function getToolbarItems(

const settings = await registry.load(pluginId);

const toolbarItems: IObservableList<ISettingRegistry.IToolbarItem> = new ObservableList(
{
values: JSONExt.deepCopy(settings.composite[propertyId] as any) ?? [],
itemCmp: (a, b) => JSONExt.deepEqual(a, b)
}
);

// React to customization by the user
settings.changed.connect(() => {
// As extension may change the toolbar through API,
// prompt the user to reload if the toolbar definition has been updated.
const newItems = (settings.composite[propertyId] as any) ?? [];
if (!JSONExt.deepEqual(toArray(toolbarItems.iter()), newItems)) {
void displayInformation(trans);
}
const newItems: ISettingRegistry.IToolbarItem[] =
(settings.composite[propertyId] as any) ?? [];

transferSettings(newItems);
});

// React to plugin changes
Expand All @@ -190,31 +188,33 @@ async function getToolbarItems(
} else {
// The plugin was not yet loaded => update the toolbar items list
loaded[plugin] = JSONExt.deepCopy(newItems);
const newList =
const newList = (
SettingRegistry.reconcileToolbarItems(
toArray(toolbarItems),
newItems,
false
) ?? [];

// Existing items cannot be removed.
newList?.forEach(item => {
const index = findIndex(
toolbarItems,
value => item.name === value.name
);
if (index < 0) {
toolbarItems.push(item);
} else {
toolbarItems.set(index, item);
}
});
) ?? []
).sort(
(a, b) =>
(a.rank ?? DEFAULT_TOOLBAR_ITEM_RANK) -
(b.rank ?? DEFAULT_TOOLBAR_ITEM_RANK)
);
transferSettings(newList);
}
}
}
});

return toolbarItems;
const transferSettings = (newItems: ISettingRegistry.IToolbarItem[]) => {
// This is not optimal but safer because a toolbar item with the same
// name cannot be inserted (it will be a no-op). But that could happen
// if the settings are changing the items order.
toolbarItems.clear();
toolbarItems.pushAll(newItems.filter(item => !item.disabled));
};

// Initialize the toolbar
transferSettings((settings.composite[propertyId] as any) ?? []);
}

/**
Expand All @@ -236,66 +236,71 @@ export function createToolbarFactory(
pluginId: string,
translator: ITranslator,
propertyId: string = 'toolbar'
): (widget: Widget) => ToolbarRegistry.IToolbarItem[] {
const items: ToolbarRegistry.IWidget[] = [];
let rawItems: IObservableList<ToolbarRegistry.IWidget>;

const transfer = (
list: IObservableList<ToolbarRegistry.IWidget>,
change: IObservableList.IChangedArgs<ToolbarRegistry.IWidget>
) => {
switch (change.type) {
case 'move':
break;
case 'add':
case 'remove':
case 'set':
items.length = 0;
items.push(
...toArray(list)
.filter(item => !item.disabled)
.sort(
(a, b) =>
(a.rank ?? DEFAULT_TOOLBAR_ITEM_RANK) -
(b.rank ?? DEFAULT_TOOLBAR_ITEM_RANK)
)
);
break;
}
};
): (widget: Widget) => IObservableList<ToolbarRegistry.IToolbarItem> {
const items = new ObservableList<ISettingRegistry.IToolbarItem>({
itemCmp: (a, b) => JSONExt.deepEqual(a as any, b as any)
});

// Get toolbar definition from the settings
getToolbarItems(
setToolbarItems(
items,
settingsRegistry,
factoryName,
pluginId,
translator,
propertyId
)
.then(candidates => {
rawItems = candidates;
rawItems.changed.connect(transfer);
// Force initialization of items
transfer(rawItems, {
type: 'add',
newIndex: 0,
newValues: [],
oldIndex: 0,
oldValues: []
});
})
.catch(reason => {
console.error(
`Failed to load toolbar items for factory ${factoryName} from ${pluginId}`,
reason
);
).catch(reason => {
console.error(
`Failed to load toolbar items for factory ${factoryName} from ${pluginId}`,
reason
);
});

return (widget: Widget) => {
const updateToolbar = (
list: IObservableList<ToolbarRegistry.IWidget>,
change: IObservableList.IChangedArgs<ToolbarRegistry.IWidget>
) => {
switch (change.type) {
case 'move':
toolbar.move(change.oldIndex, change.newIndex);
break;
case 'add':
change.newValues.forEach(item =>
toolbar.push({
name: item.name,
widget: toolbarRegistry.createWidget(factoryName, widget, item)
})
);
break;
case 'remove':
change.oldValues.forEach(() => toolbar.remove(change.oldIndex));
break;
case 'set':
change.newValues.forEach(item =>
toolbar.set(change.newIndex, {
name: item.name,
widget: toolbarRegistry.createWidget(factoryName, widget, item)
})
);
break;
}
};

const toolbar = new ObservableList<ToolbarRegistry.IToolbarItem>({
values: toArray(items).map(item => {
return {
name: item.name,
widget: toolbarRegistry.createWidget(factoryName, widget, item)
};
})
});

return (widget: Widget) =>
items.map(item => {
return {
name: item.name,
widget: toolbarRegistry.createWidget(factoryName, widget, item)
};
items.changed.connect(updateToolbar);
widget.disposed.connect(() => {
items.changed.disconnect(updateToolbar);
});

return toolbar;
};
}
29 changes: 25 additions & 4 deletions packages/apputils/test/toolbar.spec.ts
Expand Up @@ -252,7 +252,13 @@ describe('@jupyterlab/apputils', () => {
rank: 20
},
{ name: 'spacer', type: 'spacer', rank: 100 },
{ name: 'cut', command: 'notebook:cut-cell', rank: 21 }
{ name: 'cut', command: 'notebook:cut-cell', rank: 21 },
{
name: 'clear-all',
command: 'notebook:clear-all-cell-outputs',
rank: 60,
disabled: true
}
]
},
'jupyter.lab.transform': true,
Expand Down Expand Up @@ -307,8 +313,11 @@ describe('@jupyterlab/apputils', () => {
// factory in `createToolbarFactory`
await Promise.resolve();

const items = factory(null as any);
const items = factory(new Widget());
expect(items).toHaveLength(3);
expect(items.get(0).name).toEqual('insert');
expect(items.get(1).name).toEqual('cut');
expect(items.get(2).name).toEqual('spacer');
});

it('should update the toolbar items with late settings load', async () => {
Expand All @@ -328,7 +337,12 @@ describe('@jupyterlab/apputils', () => {
schema: {
'jupyter.lab.toolbars': {
dummyFactory: [
{ name: 'cut', command: 'notebook:cut-cell', rank: 21 }
{ name: 'cut', command: 'notebook:cut-cell', rank: 21 },
{ name: 'insert', rank: 40 },
{
name: 'clear-all',
disabled: true
}
]
},
type: 'object'
Expand All @@ -349,6 +363,11 @@ describe('@jupyterlab/apputils', () => {
name: 'insert',
command: 'notebook:insert-cell-below',
rank: 20
},
{
name: 'clear-all',
command: 'notebook:clear-all-cell-outputs',
rank: 60
}
]
},
Expand Down Expand Up @@ -407,8 +426,10 @@ describe('@jupyterlab/apputils', () => {
await Promise.resolve();
await settingRegistry.load(foo.id);

const items = factory(null as any);
const items = factory(new Widget());
expect(items).toHaveLength(2);
expect(items.get(0).name).toEqual('cut');
expect(items.get(1).name).toEqual('insert');
});
});
});
1 change: 1 addition & 0 deletions packages/csvviewer-extension/package.json
Expand Up @@ -43,6 +43,7 @@
"@jupyterlab/docregistry": "^4.0.0-alpha.4",
"@jupyterlab/documentsearch": "^4.0.0-alpha.4",
"@jupyterlab/mainmenu": "^4.0.0-alpha.4",
"@jupyterlab/observables": "^5.0.0-alpha.4",
"@jupyterlab/settingregistry": "^4.0.0-alpha.4",
"@jupyterlab/translation": "^4.0.0-alpha.4",
"@lumino/datagrid": "^0.35.1",
Expand Down
9 changes: 7 additions & 2 deletions packages/csvviewer-extension/src/index.ts
Expand Up @@ -27,6 +27,7 @@ import {
import { DocumentRegistry, IDocumentWidget } from '@jupyterlab/docregistry';
import { ISearchProviderRegistry } from '@jupyterlab/documentsearch';
import { IMainMenu } from '@jupyterlab/mainmenu';
import { IObservableList } from '@jupyterlab/observables';
import { ISettingRegistry } from '@jupyterlab/settingregistry';
import { ITranslator } from '@jupyterlab/translation';
import { DataGrid } from '@lumino/datagrid';
Expand Down Expand Up @@ -98,7 +99,9 @@ function activateCsv(
): void {
const { commands, shell } = app;
let toolbarFactory:
| ((widget: IDocumentWidget<CSVViewer>) => DocumentRegistry.IToolbarItem[])
| ((
widget: IDocumentWidget<CSVViewer>
) => IObservableList<DocumentRegistry.IToolbarItem>)
| undefined;

if (toolbarRegistry) {
Expand Down Expand Up @@ -240,7 +243,9 @@ function activateTsv(
): void {
const { commands, shell } = app;
let toolbarFactory:
| ((widget: IDocumentWidget<CSVViewer>) => DocumentRegistry.IToolbarItem[])
| ((
widget: IDocumentWidget<CSVViewer>
) => IObservableList<DocumentRegistry.IToolbarItem>)
| undefined;

if (toolbarRegistry) {
Expand Down
3 changes: 3 additions & 0 deletions packages/csvviewer-extension/tsconfig.json
Expand Up @@ -24,6 +24,9 @@
{
"path": "../mainmenu"
},
{
"path": "../observables"
},
{
"path": "../settingregistry"
},
Expand Down