some cleanup

app/client/components/CustomCalendarView.ts

@@ -1,49 +1,8 @@
-// import {AccessLevel} from "app/common/CustomWidget";
-// import {ViewSectionRec} from "app/client/models/entities/ViewSectionRec";
 import { CustomView, CustomViewSettings } from "app/client/components/CustomView";
 import { AccessLevel } from "app/common/CustomWidget";
-// import {GristDoc} from "app/client/components/GristDoc";
-// import {reportError} from 'app/client/models/errors';
-
-//Abstract class for more future inheritances
-// abstract class CustomAttachedView extends CustomView {
-  /*
-  public override create(gristDoc: GristDoc, viewSectionModel: ViewSectionRec) {
-    super.create(gristDoc, viewSectionModel);
-    if (viewSectionModel.customDef.access.peek() !== AccessLevel.full) {
-      void viewSectionModel.customDef.access.setAndSave(AccessLevel.full).catch((err)=>{
-        if (err?.code === "ACL_DENY") {
-          // do nothing, we might be in a readonly mode.
-          return;
-        }
-        reportError(err);
-      });
-    }
-
-    const widgetsApi = this.gristDoc.app.topAppModel;
-    widgetsApi.getWidgets().then(async result=>{
-      const widget = result.find(w=>w.name == this.getWidgetName());
-      if (widget && this.customDef.url.peek() !== widget.url) {
-        await this.customDef.url.setAndSave(widget.url);
-      }
-    }).catch((err)=>{
-      if (err?.code !== "ACL_DENY") {
-        // TODO: revisit it later. getWidgets() is async call, and non of the code
-        // above is checking if we are still alive.
-        console.error(err);
-      } else {
-        // do nothing, we might be in a readonly mode.
-      }
-    });
-  }
-  */
-
-//  protected abstract getWidgetName(): string;
-
-// }
 
 export class CustomCalendarView extends CustomView {
-  protected getInitialSettings(): CustomViewSettings {
+  protected getBuiltInSettings(): CustomViewSettings {
     return {
       widgetId: '@gristlabs/widget-calendar',
       accessLevel: AccessLevel.full,

app/client/components/CustomView.ts

@@ -32,6 +32,13 @@ import {dom as grains} from 'grainjs';
 import * as ko from 'knockout';
 import defaults = require('lodash/defaults');
 
+/**
+ *
+ * Built in settings for a custom widget. Used when the custom
+ * widget is the implementation of a native-looking widget,
+ * for example the calendar widget.
+ *
+ */
 export interface CustomViewSettings {
   widgetId?: string;
   accessLevel?: AccessLevel;
@@ -106,42 +113,6 @@ export class CustomView extends Disposable {
 
     this.viewPane = this.autoDispose(this._buildDom());
     this._updatePluginInstance();
-
-    this.dealWithBundledWidgets(gristDoc, viewSectionModel);
-  }
-
-  public dealWithBundledWidgets(gristDoc: GristDoc, viewSectionModel: ViewSectionRec) {
-    const settings = this.getInitialSettings();
-    console.log("dealWith!", {settings});
-    if (!settings.widgetId) { return; }
-    if (viewSectionModel.customDef.access.peek() !== AccessLevel.full) {
-      void viewSectionModel.customDef.access.setAndSave(AccessLevel.full).catch((err)=>{
-        if (err?.code === "ACL_DENY") {
-          // do nothing, we might be in a readonly mode.
-          return;
-        }
-        reportError(err);
-      });
-    }
-    
-    const widgetsApi = this.gristDoc.app.topAppModel;
-    widgetsApi.getWidgets().then(async result=>{
-      const widget = result.find(w => w.widgetId === settings.widgetId);
-      console.log("FOUND", {widget});
-      if (widget && this.customDef.widgetId.peek() !== widget.widgetId) {
-        console.log("SET!!");
-        await this.customDef.widgetId.setAndSave(widget.widgetId);
-        await this.customDef.pluginId.setAndSave(widget.fromPlugin||'');
-      }
-    }).catch((err)=>{
-      if (err?.code !== "ACL_DENY") {
-        // TODO: revisit it later. getWidgets() is async call, and non of the code
-        // above is checking if we are still alive.
-        console.error(err);
-      } else {
-        // do nothing, we might be in a readonly mode.
-      }
-    });
   }
 
   public async triggerPrint() {
@@ -150,10 +121,10 @@ export class CustomView extends Disposable {
     }
   }
 
-  protected getInitialSettings(): CustomViewSettings {
+  protected getBuiltInSettings(): CustomViewSettings {
     return {};
   }
-  
+
   protected getEmptyWidgetPage(): string {
     return new URL("custom-widget.html", getGristConfig().homeUrl!).href;
   }
@@ -201,10 +172,7 @@ export class CustomView extends Disposable {
     const showPlugin = ko.pureComputed(() => this.customDef.mode() === "plugin");
     const showAfterReady = () => {
       // The empty widget page calls `grist.ready()`.
-      // Pending: URLs set now only when user actually enters a URL,
-      // so this could be breaking pages without grist.ready() call
-      // added to manifests.
-      if (!url()) { return true; }
+      if (!url() && !widgetId()) { return true; }
 
       return renderAfterReady();
     };
@@ -216,19 +184,25 @@ export class CustomView extends Disposable {
         // For the view to update when switching from one section to another one, the computed
         // observable must always notify.
         .extend({notify: 'always'});
+    // Some widgets have built-in settings that should override anything
+    // that is in the rest of the view options. Ideally, everything would
+    // be consistent. We could fix inconsistencies if we find them, but
+    // we are not guaranteed to have write privileges at this point.
+    const builtInSettings = this.getBuiltInSettings();
     return dom('div.flexauto.flexvbox.custom_view_container',
       dom.autoDispose(showPlugin),
       dom.autoDispose(showPluginNotification),
       dom.autoDispose(showSectionNotification),
       dom.autoDispose(showPluginContent),
       // todo: should display content in webview when running electron
-      kd.scope(() => [mode(), url(), access(), widgetId(), pluginId()], ([_mode, _url, _access, _widgetId, _pluginId]: string[]) =>
+      kd.scope(() => [mode(), url(), access(), widgetId(), pluginId()],
+               ([_mode, _url, _access, _widgetId, _pluginId]: string[]) =>
         _mode === "url" ?
           this._buildIFrame({
             baseUrl: _url,
-            access: (_access as AccessLevel || AccessLevel.none),
+            access: builtInSettings.accessLevel || (_access as AccessLevel || AccessLevel.none),
             showAfterReady: showAfterReady(),
-            widgetId: _widgetId,
+            widgetId: builtInSettings.widgetId || _widgetId,
             pluginId: _pluginId,
           })
           : null
@@ -267,7 +241,6 @@ export class CustomView extends Disposable {
       url: baseUrl || this.getEmptyWidgetPage(),
       widgetId,
       pluginId,
-      emptyUrl: this.getEmptyWidgetPage(),
       access,
       readonly: this.gristDoc.isReadonly.get(),
       showAfterReady,

app/client/components/WidgetFrame.ts

@@ -6,7 +6,8 @@ import {hooks} from 'app/client/Hooks';
 import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals';
 import {makeTestId} from 'app/client/lib/domUtils';
 import {ColumnRec, ViewSectionRec} from 'app/client/models/DocModel';
-import {AccessLevel, ICustomWidget, isSatisfied, matchWidget } from 'app/common/CustomWidget';
+import {reportError} from 'app/client/models/errors';
+import {AccessLevel, ICustomWidget, isSatisfied, matchWidget} from 'app/common/CustomWidget';
 import {DisposableWithEvents} from 'app/common/DisposableWithEvents';
 import {BulkColValues, fromTableDataAction, RowRecord} from 'app/common/DocActions';
 import {extractInfoFromColType, reencodeAsAny} from 'app/common/gristTypes';
@@ -19,7 +20,6 @@ import noop = require('lodash/noop');
 import debounce = require('lodash/debounce');
 import isEqual = require('lodash/isEqual');
 import flatMap = require('lodash/flatMap');
-import { reportError } from '../models/errors';
 
 const testId = makeTestId('test-custom-widget-');
 
@@ -44,9 +44,15 @@ export interface WidgetFrameOptions {
    * Url of external page. Iframe is rebuild each time the URL changes.
    */
   url: string;
+  /**
+   * ID of widget, if known. When set, the url for the specified widget
+   * in the WidgetRepository, if found, will take precedence.
+   */
   widgetId?: string|null;
+  /**
+   * ID of the plugin that provided the widget (if it came from a plugin).
+   */
   pluginId?: string;
-  emptyUrl: string;
   /**
    * Assigned access level. Iframe is rebuild each time access level is changed.
    */
@@ -77,7 +83,9 @@ export interface WidgetFrameOptions {
    * Optional handler to modify the iframe.
    */
   onElem?: (iframe: HTMLIFrameElement) => void;
-
+  /**
+   * The containing document.
+   */
   gristDoc: GristDoc;
 }
 
@@ -93,7 +101,8 @@ export class WidgetFrame extends DisposableWithEvents {
   private _readyCalled = Observable.create(this, false);
   // Whether the iframe is visible.
   private _visible = Observable.create(this, !this._options.showAfterReady);
-  public readonly _widgets = Observable.create<ICustomWidget[]>(this, []);
+  // An entry for this widget in the WidgetRepository, if available.
+  public readonly _widget = Observable.create<ICustomWidget|null>(this, null);
 
   constructor(private _options: WidgetFrameOptions) {
     super();
@@ -121,7 +130,7 @@ export class WidgetFrame extends DisposableWithEvents {
     // Call custom configuration handler.
     _options.configure?.(this);
 
-    this._fetchWidgets().catch(reportError);
+    this._checkWidgetRepository().catch(reportError);
   }
 
   /**
@@ -184,16 +193,14 @@ export class WidgetFrame extends DisposableWithEvents {
         dom.style('visibility', use => use(this._visible) ? 'visible' : 'hidden'),
         dom.cls('clipboard_focus'),
         dom.cls('custom_view'),
-        dom.attr('src', use => this._getUrl(use(this._widgets))),
-        {
-          ...hooks.iframeAttributes,
-        },
+        dom.attr('src', use => this._getUrl(use(this._widget))),
+        hooks.iframeAttributes,
         testId('ready', this._readyCalled),
       ))
     );
   }
 
-  private _getUrl(widgets: ICustomWidget[]): string {
+  private _getUrl(widget: ICustomWidget|null): string {
     // Append access level to query string.
     const urlWithAccess = (url: string) => {
       if (!url) {
@@ -204,20 +211,8 @@ export class WidgetFrame extends DisposableWithEvents {
       urlObj.searchParams.append('readonly', String(this._options.readonly));
       return urlObj.href;
     };
-    const {widgetId, pluginId} = this._options;
-    let url = this._options.url;
-    if (widgetId) {
-      console.log("Iframe match starting");
-      const widget = matchWidget(widgets, {widgetId, pluginId});
-      console.log("Iframe match done");
-      if (widget) {
-        url = widget.url;
-      } else {
-        return 'about:blank';
-      }
-    }
-    const fullUrl = urlWithAccess(url);
-    return fullUrl;
+    const url = widget?.url || this._options.url || 'about:blank';
+    return urlWithAccess(url);
   }
 
   private _onMessage(event: MessageEvent) {
@@ -245,12 +240,17 @@ export class WidgetFrame extends DisposableWithEvents {
     }
   }
 
-  private async _fetchWidgets() {
-    if (this.isDisposed()) { return; }
+  /**
+   * If we have a widgetId, look it up in the WidgetRepository and
+   * get the best URL we can for it.
+   */
+  private async _checkWidgetRepository() {
+    const {widgetId, pluginId} = this._options;
+    if (this.isDisposed() || !widgetId) { return; }
     const widgets = await this._options.gristDoc.app.topAppModel.getWidgets();
     if (this.isDisposed()) { return; }
-    this._widgets.set(widgets);
-    console.log("SAVED", {widgets});
+    const widget = matchWidget(widgets, {widgetId, pluginId});
+    this._widget.set(widget || null);
   }
 }