Browse Source

Backport PR #11963: Support dynamic toolbar definition (#12078)

Co-authored-by: Eric Charles <eric@datalayer.io>
Frédéric Collonval 3 years ago
parent
commit
3e449cd2cf

+ 99 - 94
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';
@@ -38,8 +38,11 @@ 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
@@ -47,13 +50,14 @@ async function displayInformation(trans: TranslationBundle): Promise<void> {
  * @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[] } = {};
@@ -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.
@@ -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 };
 
@@ -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
@@ -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) ?? []);
 }
 
 /**
@@ -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;
+  };
 }

+ 308 - 6
packages/apputils/test/toolbar.spec.ts

@@ -3,27 +3,33 @@
 
 import {
   CommandToolbarButton,
+  createToolbarFactory,
   ReactiveToolbar,
   SessionContext,
   Toolbar,
-  ToolbarButton
+  ToolbarButton,
+  ToolbarRegistry,
+  ToolbarWidgetRegistry
 } from '@jupyterlab/apputils';
+import { ISettingRegistry, SettingRegistry } from '@jupyterlab/settingregistry';
+import { IDataConnector } from '@jupyterlab/statedb';
 import {
   createSessionContext,
   framePromise,
   JupyterServer
 } from '@jupyterlab/testutils';
-import { toArray } from '@lumino/algorithm';
-import { CommandRegistry } from '@lumino/commands';
-import { ReadonlyPartialJSONObject } from '@lumino/coreutils';
-import { PanelLayout, Widget } from '@lumino/widgets';
-import { simulate } from 'simulate-event';
+import { ITranslator } from '@jupyterlab/translation';
 import {
   blankIcon,
   bugDotIcon,
   bugIcon,
   jupyterIcon
 } from '@jupyterlab/ui-components';
+import { toArray } from '@lumino/algorithm';
+import { CommandRegistry } from '@lumino/commands';
+import { ReadonlyPartialJSONObject } from '@lumino/coreutils';
+import { PanelLayout, Widget } from '@lumino/widgets';
+import { simulate } from 'simulate-event';
 
 const server = new JupyterServer();
 
@@ -741,4 +747,300 @@ describe('@jupyterlab/apputils', () => {
       });
     });
   });
+
+  describe('ToolbarWidgetRegistry', () => {
+    describe('#constructor', () => {
+      it('should set a default factory', () => {
+        const dummy = jest.fn();
+        const registry = new ToolbarWidgetRegistry({
+          defaultFactory: dummy
+        });
+
+        expect(registry.defaultFactory).toBe(dummy);
+      });
+    });
+
+    describe('#defaultFactory', () => {
+      it('should set a default factory', () => {
+        const dummy = jest.fn();
+        const dummy2 = jest.fn();
+        const registry = new ToolbarWidgetRegistry({
+          defaultFactory: dummy
+        });
+
+        registry.defaultFactory = dummy2;
+
+        expect(registry.defaultFactory).toBe(dummy2);
+      });
+    });
+
+    describe('#createWidget', () => {
+      it('should call the default factory as fallback', () => {
+        const documentWidget = new Widget();
+        const dummyWidget = new Widget();
+        const dummy = jest.fn().mockReturnValue(dummyWidget);
+        const registry = new ToolbarWidgetRegistry({
+          defaultFactory: dummy
+        });
+
+        const item: ToolbarRegistry.IWidget = {
+          name: 'test'
+        };
+
+        const widget = registry.createWidget('factory', documentWidget, item);
+
+        expect(widget).toBe(dummyWidget);
+        expect(dummy).toBeCalledWith('factory', documentWidget, item);
+      });
+
+      it('should call the registered factory', () => {
+        const documentWidget = new Widget();
+        const dummyWidget = new Widget();
+        const defaultFactory = jest.fn().mockReturnValue(dummyWidget);
+        const dummy = jest.fn().mockReturnValue(dummyWidget);
+        const registry = new ToolbarWidgetRegistry({
+          defaultFactory
+        });
+
+        const item: ToolbarRegistry.IWidget = {
+          name: 'test'
+        };
+
+        registry.registerFactory('factory', item.name, dummy);
+
+        const widget = registry.createWidget('factory', documentWidget, item);
+
+        expect(widget).toBe(dummyWidget);
+        expect(dummy).toBeCalledWith(documentWidget);
+        expect(defaultFactory).toBeCalledTimes(0);
+      });
+    });
+
+    describe('#registerFactory', () => {
+      it('should return the previous registered factory', () => {
+        const defaultFactory = jest.fn();
+        const dummy = jest.fn();
+        const dummy2 = jest.fn();
+        const registry = new ToolbarWidgetRegistry({
+          defaultFactory
+        });
+
+        const item: ToolbarRegistry.IWidget = {
+          name: 'test'
+        };
+
+        expect(
+          registry.registerFactory('factory', item.name, dummy)
+        ).toBeUndefined();
+        expect(registry.registerFactory('factory', item.name, dummy2)).toBe(
+          dummy
+        );
+      });
+    });
+  });
+
+  describe('createToolbarFactory', () => {
+    it('should return the toolbar items', async () => {
+      const factoryName = 'dummyFactory';
+      const pluginId = 'test-plugin:settings';
+      const toolbarRegistry = new ToolbarWidgetRegistry({
+        defaultFactory: jest.fn()
+      });
+
+      const bar: ISettingRegistry.IPlugin = {
+        data: {
+          composite: {},
+          user: {}
+        },
+        id: pluginId,
+        raw: '{}',
+        schema: {
+          'jupyter.lab.toolbars': {
+            dummyFactory: [
+              {
+                name: 'insert',
+                command: 'notebook:insert-cell-below',
+                rank: 20
+              },
+              { name: 'spacer', type: 'spacer', rank: 100 },
+              { 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,
+          properties: {
+            toolbar: {
+              type: 'array'
+            }
+          },
+          type: 'object'
+        },
+        version: 'test'
+      };
+
+      const connector: IDataConnector<
+        ISettingRegistry.IPlugin,
+        string,
+        string,
+        string
+      > = {
+        fetch: jest.fn().mockImplementation((id: string) => {
+          switch (id) {
+            case bar.id:
+              return bar;
+            default:
+              return {};
+          }
+        }),
+        list: jest.fn(),
+        save: jest.fn(),
+        remove: jest.fn()
+      };
+
+      const settingRegistry = new SettingRegistry({
+        connector
+      });
+
+      const translator: ITranslator = {
+        load: jest.fn()
+      };
+
+      const factory = createToolbarFactory(
+        toolbarRegistry,
+        settingRegistry,
+        factoryName,
+        pluginId,
+        translator
+      );
+
+      await settingRegistry.load(bar.id);
+      // Trick push this test after all other promise in the hope they get resolve
+      // before going further - in particular we are looking at the update of the items
+      // factory in `createToolbarFactory`
+      await Promise.resolve();
+
+      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 () => {
+      const factoryName = 'dummyFactory';
+      const pluginId = 'test-plugin:settings';
+      const toolbarRegistry = new ToolbarWidgetRegistry({
+        defaultFactory: jest.fn()
+      });
+
+      const foo: ISettingRegistry.IPlugin = {
+        data: {
+          composite: {},
+          user: {}
+        },
+        id: 'foo',
+        raw: '{}',
+        schema: {
+          'jupyter.lab.toolbars': {
+            dummyFactory: [
+              { name: 'cut', command: 'notebook:cut-cell', rank: 21 },
+              { name: 'insert', rank: 40 },
+              {
+                name: 'clear-all',
+                disabled: true
+              }
+            ]
+          },
+          type: 'object'
+        },
+        version: 'test'
+      };
+      const bar: ISettingRegistry.IPlugin = {
+        data: {
+          composite: {},
+          user: {}
+        },
+        id: pluginId,
+        raw: '{}',
+        schema: {
+          'jupyter.lab.toolbars': {
+            dummyFactory: [
+              {
+                name: 'insert',
+                command: 'notebook:insert-cell-below',
+                rank: 20
+              },
+              {
+                name: 'clear-all',
+                command: 'notebook:clear-all-cell-outputs',
+                rank: 60
+              }
+            ]
+          },
+          'jupyter.lab.transform': true,
+          properties: {
+            toolbar: {
+              type: 'array'
+            }
+          },
+          type: 'object'
+        },
+        version: 'test'
+      };
+
+      const connector: IDataConnector<
+        ISettingRegistry.IPlugin,
+        string,
+        string,
+        string
+      > = {
+        fetch: jest.fn().mockImplementation((id: string) => {
+          switch (id) {
+            case bar.id:
+              return bar;
+            case foo.id:
+              return foo;
+            default:
+              return {};
+          }
+        }),
+        list: jest.fn(),
+        save: jest.fn(),
+        remove: jest.fn()
+      };
+
+      const settingRegistry = new SettingRegistry({
+        connector
+      });
+
+      const translator: ITranslator = {
+        load: jest.fn()
+      };
+
+      const factory = createToolbarFactory(
+        toolbarRegistry,
+        settingRegistry,
+        factoryName,
+        pluginId,
+        translator
+      );
+
+      await settingRegistry.load(bar.id);
+      // Trick push this test after all other promise in the hope they get resolve
+      // before going further - in particular we are looking at the update of the items
+      // factory in `createToolbarFactory`
+      await Promise.resolve();
+      await settingRegistry.load(foo.id);
+
+      const items = factory(new Widget());
+      expect(items).toHaveLength(2);
+      expect(items.get(0).name).toEqual('cut');
+      expect(items.get(1).name).toEqual('insert');
+    });
+  });
 });

+ 1 - 0
packages/csvviewer-extension/package.json

@@ -43,6 +43,7 @@
     "@jupyterlab/docregistry": "^3.3.0-beta.0",
     "@jupyterlab/documentsearch": "^3.3.0-beta.0",
     "@jupyterlab/mainmenu": "^3.3.0-beta.0",
+    "@jupyterlab/observables": "^4.3.0-beta.0",
     "@jupyterlab/settingregistry": "^3.3.0-beta.0",
     "@jupyterlab/translation": "^3.3.0-beta.0",
     "@lumino/datagrid": "^0.20.0",

+ 7 - 2
packages/csvviewer-extension/src/index.ts

@@ -27,6 +27,7 @@ import {
 import { DocumentRegistry, IDocumentWidget } from '@jupyterlab/docregistry';
 import { ISearchProviderRegistry } from '@jupyterlab/documentsearch';
 import { IEditMenu, IMainMenu } from '@jupyterlab/mainmenu';
+import { IObservableList } from '@jupyterlab/observables';
 import { ISettingRegistry } from '@jupyterlab/settingregistry';
 import { ITranslator } from '@jupyterlab/translation';
 import { DataGrid } from '@lumino/datagrid';
@@ -113,7 +114,9 @@ function activateCsv(
   toolbarRegistry: IToolbarWidgetRegistry | null
 ): void {
   let toolbarFactory:
-    | ((widget: IDocumentWidget<CSVViewer>) => DocumentRegistry.IToolbarItem[])
+    | ((
+        widget: IDocumentWidget<CSVViewer>
+      ) => IObservableList<DocumentRegistry.IToolbarItem>)
     | undefined;
 
   if (toolbarRegistry) {
@@ -224,7 +227,9 @@ function activateTsv(
   toolbarRegistry: IToolbarWidgetRegistry | null
 ): void {
   let toolbarFactory:
-    | ((widget: IDocumentWidget<CSVViewer>) => DocumentRegistry.IToolbarItem[])
+    | ((
+        widget: IDocumentWidget<CSVViewer>
+      ) => IObservableList<DocumentRegistry.IToolbarItem>)
     | undefined;
 
   if (toolbarRegistry) {

+ 3 - 0
packages/csvviewer-extension/tsconfig.json

@@ -24,6 +24,9 @@
     {
       "path": "../mainmenu"
     },
+    {
+      "path": "../observables"
+    },
     {
       "path": "../settingregistry"
     },

+ 83 - 9
packages/docregistry/src/default.ts

@@ -5,10 +5,11 @@ import { MainAreaWidget } from '@jupyterlab/apputils';
 import { CodeEditor } from '@jupyterlab/codeeditor';
 import { Mode } from '@jupyterlab/codemirror';
 import { IChangedArgs, PathExt } from '@jupyterlab/coreutils';
-import { IModelDB } from '@jupyterlab/observables';
+import { IModelDB, IObservableList } from '@jupyterlab/observables';
 import { Contents } from '@jupyterlab/services';
 import * as models from '@jupyterlab/shared-models';
 import { ITranslator, nullTranslator } from '@jupyterlab/translation';
+import { findIndex, toArray } from '@lumino/algorithm';
 import { PartialJSONValue } from '@lumino/coreutils';
 import { ISignal, Signal } from '@lumino/signaling';
 import { Title, Widget } from '@lumino/widgets';
@@ -419,15 +420,84 @@ export abstract class ABCWidgetFactory<
     const widget = this.createNewWidget(context, source);
 
     // Add toolbar items
-    let items: DocumentRegistry.IToolbarItem[];
-    if (this._toolbarFactory) {
-      items = this._toolbarFactory(widget);
+    const items:
+      | DocumentRegistry.IToolbarItem[]
+      | IObservableList<DocumentRegistry.IToolbarItem> = (
+      this._toolbarFactory?.bind(this) ?? this.defaultToolbarFactory.bind(this)
+    )(widget);
+
+    if (Array.isArray(items)) {
+      items.forEach(({ name, widget: item }) => {
+        widget.toolbar.addItem(name, item);
+      });
     } else {
-      items = this.defaultToolbarFactory(widget);
+      const updateToolbar = (
+        list: IObservableList<DocumentRegistry.IToolbarItem>,
+        changes: IObservableList.IChangedArgs<DocumentRegistry.IToolbarItem>
+      ) => {
+        switch (changes.type) {
+          case 'add':
+            changes.newValues.forEach((item, index) => {
+              widget.toolbar.insertItem(
+                changes.newIndex + index,
+                item.name,
+                item.widget
+              );
+            });
+            break;
+          case 'move':
+            changes.oldValues.forEach(item => {
+              item.widget.parent = null;
+            });
+            changes.newValues.forEach((item, index) => {
+              widget.toolbar.insertItem(
+                changes.newIndex + index,
+                item.name,
+                item.widget
+              );
+            });
+            break;
+          case 'remove':
+            changes.oldValues.forEach(item => {
+              item.widget.parent = null;
+            });
+            break;
+          case 'set':
+            changes.oldValues.forEach(item => {
+              item.widget.parent = null;
+            });
+
+            changes.newValues.forEach((item, index) => {
+              const existingIndex = findIndex(
+                widget.toolbar.names(),
+                name => item.name === name
+              );
+              if (existingIndex >= 0) {
+                toArray(widget.toolbar.children())[existingIndex].parent = null;
+              }
+
+              widget.toolbar.insertItem(
+                changes.newIndex + index,
+                item.name,
+                item.widget
+              );
+            });
+            break;
+        }
+      };
+
+      updateToolbar(items, {
+        newIndex: 0,
+        newValues: toArray(items),
+        oldIndex: 0,
+        oldValues: [],
+        type: 'add'
+      });
+      items.changed.connect(updateToolbar);
+      widget.disposed.connect(() => {
+        items.changed.disconnect(updateToolbar);
+      });
     }
-    items.forEach(({ name, widget: item }) => {
-      widget.toolbar.addItem(name, item);
-    });
 
     // Emit widget created signal
     this._widgetCreated.emit(widget);
@@ -451,7 +521,11 @@ export abstract class ABCWidgetFactory<
   }
 
   private _toolbarFactory:
-    | ((widget: T) => DocumentRegistry.IToolbarItem[])
+    | ((
+        widget: T
+      ) =>
+        | DocumentRegistry.IToolbarItem[]
+        | IObservableList<DocumentRegistry.IToolbarItem>)
     | undefined;
   private _isDisposed = false;
   private _translator: ITranslator;

+ 6 - 2
packages/docregistry/src/registry.ts

@@ -11,7 +11,7 @@ import {
   IChangedArgs as IChangedArgsGeneric,
   PathExt
 } from '@jupyterlab/coreutils';
-import { IModelDB } from '@jupyterlab/observables';
+import { IModelDB, IObservableList } from '@jupyterlab/observables';
 import { IRenderMime } from '@jupyterlab/rendermime-interfaces';
 import { Contents, Kernel } from '@jupyterlab/services';
 import * as models from '@jupyterlab/shared-models';
@@ -1100,7 +1100,11 @@ export namespace DocumentRegistry {
     /**
      * A function producing toolbar widgets, overriding the default toolbar widgets.
      */
-    readonly toolbarFactory?: (widget: T) => DocumentRegistry.IToolbarItem[];
+    readonly toolbarFactory?: (
+      widget: T
+    ) =>
+      | DocumentRegistry.IToolbarItem[]
+      | IObservableList<DocumentRegistry.IToolbarItem>;
   }
 
   /**

+ 1 - 0
packages/fileeditor-extension/package.json

@@ -48,6 +48,7 @@
     "@jupyterlab/fileeditor": "^3.3.0-beta.0",
     "@jupyterlab/launcher": "^3.3.0-beta.0",
     "@jupyterlab/mainmenu": "^3.3.0-beta.0",
+    "@jupyterlab/observables": "^4.3.0-beta.0",
     "@jupyterlab/settingregistry": "^3.3.0-beta.0",
     "@jupyterlab/statusbar": "^3.3.0-beta.0",
     "@jupyterlab/translation": "^3.3.0-beta.0",

+ 4 - 1
packages/fileeditor-extension/src/index.ts

@@ -29,6 +29,7 @@ import {
 } from '@jupyterlab/fileeditor';
 import { ILauncher } from '@jupyterlab/launcher';
 import { IMainMenu } from '@jupyterlab/mainmenu';
+import { IObservableList } from '@jupyterlab/observables';
 import { ISettingRegistry } from '@jupyterlab/settingregistry';
 import { IStatusBar } from '@jupyterlab/statusbar';
 import { ITranslator } from '@jupyterlab/translation';
@@ -165,7 +166,9 @@ function activate(
   const trans = translator.load('jupyterlab');
   const namespace = 'editor';
   let toolbarFactory:
-    | ((widget: IDocumentWidget<FileEditor>) => DocumentRegistry.IToolbarItem[])
+    | ((
+        widget: IDocumentWidget<FileEditor>
+      ) => IObservableList<DocumentRegistry.IToolbarItem>)
     | undefined;
 
   if (toolbarRegistry) {

+ 3 - 0
packages/fileeditor-extension/tsconfig.json

@@ -39,6 +39,9 @@
     {
       "path": "../mainmenu"
     },
+    {
+      "path": "../observables"
+    },
     {
       "path": "../settingregistry"
     },

+ 1 - 0
packages/htmlviewer-extension/package.json

@@ -38,6 +38,7 @@
     "@jupyterlab/apputils": "^3.3.0-beta.0",
     "@jupyterlab/docregistry": "^3.3.0-beta.0",
     "@jupyterlab/htmlviewer": "^3.3.0-beta.0",
+    "@jupyterlab/observables": "^4.3.0-beta.0",
     "@jupyterlab/settingregistry": "^3.3.0-beta.0",
     "@jupyterlab/translation": "^3.3.0-beta.0",
     "@jupyterlab/ui-components": "^3.3.0-beta.0"

+ 2 - 1
packages/htmlviewer-extension/src/index.tsx

@@ -25,6 +25,7 @@ import {
   IHTMLViewerTracker,
   ToolbarItems
 } from '@jupyterlab/htmlviewer';
+import { IObservableList } from '@jupyterlab/observables';
 import { ISettingRegistry } from '@jupyterlab/settingregistry';
 import { ITranslator } from '@jupyterlab/translation';
 import { html5Icon } from '@jupyterlab/ui-components';
@@ -70,7 +71,7 @@ function activateHTMLViewer(
   toolbarRegistry: IToolbarWidgetRegistry | null
 ): IHTMLViewerTracker {
   let toolbarFactory:
-    | ((widget: HTMLViewer) => DocumentRegistry.IToolbarItem[])
+    | ((widget: HTMLViewer) => IObservableList<DocumentRegistry.IToolbarItem>)
     | undefined;
   const trans = translator.load('jupyterlab');
 

+ 3 - 0
packages/htmlviewer-extension/tsconfig.json

@@ -18,6 +18,9 @@
     {
       "path": "../htmlviewer"
     },
+    {
+      "path": "../observables"
+    },
     {
       "path": "../settingregistry"
     },

+ 5 - 1
packages/notebook-extension/src/index.ts

@@ -818,7 +818,11 @@ function activateWidgetFactory(
 ): NotebookWidgetFactory.IFactory {
   const { commands } = app;
   let toolbarFactory:
-    | ((widget: NotebookPanel) => DocumentRegistry.IToolbarItem[])
+    | ((
+        widget: NotebookPanel
+      ) =>
+        | DocumentRegistry.IToolbarItem[]
+        | IObservableList<DocumentRegistry.IToolbarItem>)
     | undefined;
 
   // Register notebook toolbar widgets

+ 6 - 0
packages/rendermime-interfaces/src/index.ts

@@ -72,7 +72,13 @@ export namespace IRenderMime {
    * A toolbar item.
    */
   export interface IToolbarItem {
+    /**
+     * Item name
+     */
     name: string;
+    /**
+     * Toolbar widget
+     */
     widget: Widget;
   }