(core) One more phase of ACL UI revision.

Summary:
- Add ACLColumnList widget for a list of column IDs.
- Replace autocomplete widgets with simpler dropdowns.
- Add select dropdown for the Attribute of UserAttribute rules.
- Switch formula to use ACE editor.
- Factor out customized completion logic from AceEditor.js into a separate file.
- Implement completions for ACL formulas.
- Collect ACL UI files in app/client/aclui

Test Plan: Updated test case, some behavior (like formula autocomplete) only tested manually.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2697

app/client/aclui/ACLColumnList.ts

@@ -0,0 +1,135 @@
+/**
+ * Implements a widget for showing and editing a list of colIds. It offers a select dropdown to
+ * add a new column, and allows removing already-added columns.
+ */
+import {aclSelect, cssSelect} from 'app/client/aclui/ACLSelect';
+import {colors} from 'app/client/ui2018/cssVars';
+import {icon} from 'app/client/ui2018/icons';
+import {Computed, dom, Observable, styled} from 'grainjs';
+
+export function aclColumnList(colIds: Observable<string[]>, validColIds: string[]) {
+  // Define some helpers functions.
+  function removeColId(colId: string) {
+    colIds.set(colIds.get().filter(c => (c !== colId)));
+  }
+  function addColId(colId: string) {
+    colIds.set([...colIds.get(), colId]);
+    selectBox.focus();
+  }
+  function onFocus(ev: FocusEvent) {
+    editing.set(true);
+    // Focus the select box, except when focus just moved from it, e.g. after Shift-Tab.
+    if (ev.relatedTarget !== selectBox) {
+      selectBox.focus();
+    }
+  }
+  function onBlur() {
+    if (!selectBox.matches('.weasel-popup-open') && colIds.get().length > 0) {
+      editing.set(false);
+    }
+  }
+
+  // The observable for the selected element is a Computed, with a callback for being set, which
+  // adds the selected colId to the list.
+  const newColId = Computed.create(null, (use) => '')
+    .onWrite((value) => { setTimeout(() => addColId(value), 0); });
+
+  // We don't allow adding the same column twice, so for the select dropdown build a list of
+  // unused colIds.
+  const unusedColIds = Computed.create(null, colIds, (use, _colIds) => {
+    const used = new Set(_colIds);
+    return validColIds.filter(c => !used.has(c));
+  });
+
+  // The "editing" observable determines which of two states is active: to show or to edit.
+  const editing = Observable.create(null, !colIds.get().length);
+
+  let selectBox: HTMLElement;
+  return cssColListWidget({tabIndex: '0'},
+    dom.autoDispose(unusedColIds),
+    cssColListWidget.cls('-editing', editing),
+    dom.on('focus', onFocus),
+    dom.forEach(colIds, colId =>
+      cssColItem(
+        cssColId(colId),
+        cssColItemIcon(icon('CrossSmall'),
+          dom.on('click', () => removeColId(colId))
+        )
+      )
+    ),
+    cssNewColItem(
+      dom.update(
+        selectBox = aclSelect(newColId, unusedColIds, {defaultLabel: '[Add Column]'}),
+        cssSelect.cls('-active'),
+        dom.on('blur', onBlur),
+        dom.onKeyDown({Escape: onBlur}),
+        // If starting out in edit mode, focus the select box.
+        (editing.get() ? (elem) => { setTimeout(() => elem.focus(), 0); } : null)
+      ),
+    )
+  );
+}
+
+
+const cssColListWidget = styled('div', `
+  display: flex;
+  flex-direction: column;
+  gap: 4px;
+  position: relative;
+  outline: none;
+  margin: 6px 8px;
+  cursor: pointer;
+  border-radius: 4px;
+
+  border: 1px solid transparent;
+  &:not(&-editing):hover {
+    border: 1px solid ${colors.darkGrey};
+  }
+`);
+
+const cssColItem = styled('div', `
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  border-radius: 3px;
+  padding-left: 6px;
+  padding-right: 2px;
+
+  .${cssColListWidget.className}-editing & {
+    background-color: ${colors.mediumGreyOpaque};
+  }
+`);
+
+const cssColId = styled('div', `
+  flex: auto;
+  height: 24px;
+  line-height: 24px;
+  min-width: 0;
+  overflow: hidden;
+  text-overflow: ellipsis;
+`);
+
+const cssNewColItem = styled('div', `
+  margin-top: 2px;
+  display: none;
+  .${cssColListWidget.className}-editing & {
+    display: flex;
+  }
+`);
+
+const cssColItemIcon = styled('div', `
+  flex: none;
+  height: 16px;
+  width: 16px;
+  border-radius: 16px;
+  display: none;
+  cursor: default;
+  --icon-color: ${colors.slate};
+  &:hover {
+    background-color: ${colors.slate};
+    --icon-color: ${colors.light};
+  }
+  .${cssColListWidget.className}-editing & {
+    display: flex;
+  }
+`);

app/client/aclui/ACLFormulaEditor.ts

@@ -0,0 +1,125 @@
+import {setupAceEditorCompletions} from 'app/client/components/AceEditorCompletions';
+import {colors} from 'app/client/ui2018/cssVars';
+import * as ace from 'brace';
+import {dom, DomArg, Observable, styled} from 'grainjs';
+
+export interface ACLFormulaOptions {
+  initialValue: string;
+  readOnly: boolean;
+  placeholder: DomArg;
+  setValue: (value: string) => void;
+  getSuggestions: (prefix: string) => string[];
+}
+
+export function aclFormulaEditor(options: ACLFormulaOptions) {
+  // Create an element and an editor within it.
+  const editorElem = dom('div');
+  const editor: ace.Editor = ace.edit(editorElem);
+
+  // Set various editor options.
+  editor.setTheme('ace/theme/chrome');
+  editor.setOptions({enableLiveAutocompletion: true});
+  editor.renderer.setShowGutter(false);       // Default line numbers to hidden
+  editor.renderer.setPadding(0);
+  editor.$blockScrolling = Infinity;
+  editor.setReadOnly(options.readOnly);
+  editor.setFontSize('12');
+  editor.setHighlightActiveLine(false);
+
+  const session = editor.getSession();
+  session.setMode('ace/mode/python');
+  session.setTabSize(2);
+  session.setUseWrapMode(false);
+
+  // Implement placeholder text since the version of ACE we use doesn't support one.
+  const showPlaceholder = Observable.create(null, !options.initialValue.length);
+  editor.renderer.scroller.appendChild(
+    cssAcePlaceholder(dom.show(showPlaceholder), options.placeholder)
+  );
+  editor.on("change", () => showPlaceholder.set(!editor.getValue().length));
+
+  async function getSuggestions(prefix: string) {
+    return [
+      // The few Python keywords and constants we support.
+      'and', 'or', 'not', 'in', 'is', 'True', 'False', 'None',
+      // The common variables.
+      'user', 'rec',
+      // Other completions that depend on doc schema or other rules.
+      ...options.getSuggestions(prefix),
+    ];
+  }
+  setupAceEditorCompletions(editor, {getSuggestions});
+
+  // Save on blur.
+  editor.on("blur", () => options.setValue(editor.getValue()));
+
+  // Blur (and save) on Enter key.
+  editor.commands.addCommand({
+    name: 'onEnter',
+    bindKey: {win: 'Enter', mac: 'Enter'},
+    exec: () => editor.blur(),
+  });
+  // Disable Tab/Shift+Tab commands to restore their regular behavior.
+  (editor.commands as any).removeCommands(['indent', 'outdent']);
+
+  function resize() {
+    if (editor.renderer.lineHeight === 0) {
+      // Reschedule the resize, since it's not ready yet. Seems to happen occasionally.
+      setTimeout(resize, 50);
+    }
+    editorElem.style.width = 'auto';
+    editorElem.style.height = (Math.max(1, session.getScreenLength()) * editor.renderer.lineHeight) + 'px';
+    editor.resize();
+  }
+
+  // Set the editor's initial value.
+  editor.setValue(options.initialValue);
+
+  // Resize the editor on change, and initially once it's attached to the page.
+  editor.on('change', resize);
+  setTimeout(resize, 0);
+
+  return cssConditionInputAce(
+    cssConditionInputAce.cls('-disabled', options.readOnly),
+    dom.onDispose(() => editor.destroy()),
+    editorElem,
+  );
+}
+
+const cssConditionInputAce = styled('div', `
+  width: 100%;
+  min-height: 28px;
+  padding: 5px 6px 5px 6px;
+  border-radius: 3px;
+  border: 1px solid transparent;
+  cursor: pointer;
+
+  &:hover {
+    border: 1px solid ${colors.darkGrey};
+  }
+  &:not(&-disabled):focus-within {
+    box-shadow: inset 0 0 0 1px ${colors.cursor};
+    border-color: ${colors.cursor};
+  }
+  &:not(:focus-within) .ace_scroller, &-disabled .ace_scroller {
+    cursor: unset;
+  }
+  &-disabled, &-disabled:hover {
+    background-color: ${colors.mediumGreyOpaque};
+    box-shadow: unset;
+    border-color: transparent;
+  }
+  &-disabled .ace-chrome {
+    background-color: ${colors.mediumGreyOpaque};
+  }
+  & .ace_marker-layer, & .ace_cursor-layer {
+    display: none;
+  }
+  &:not(&-disabled) .ace_focus .ace_marker-layer, &:not(&-disabled) .ace_focus .ace_cursor-layer {
+    display: block;
+  }
+`);
+
+const cssAcePlaceholder = styled('div', `
+  opacity: 0.5;
+`);

app/client/aclui/ACLSelect.ts

@@ -0,0 +1,37 @@
+import {colors} from 'app/client/ui2018/cssVars';
+import {icon} from 'app/client/ui2018/icons';
+import {IOption, select} from 'app/client/ui2018/menus';
+import {MaybeObsArray, Observable, styled} from 'grainjs';
+import * as weasel from 'popweasel';
+
+/**
+ * A styled version of select() from ui2018/menus, for use in the AccessRules page.
+ */
+export function aclSelect<T>(obs: Observable<T>, optionArray: MaybeObsArray<IOption<T>>,
+                             options: weasel.ISelectUserOptions = {}) {
+  return cssSelect(obs, optionArray, {buttonArrow: cssSelectArrow('Collapse'), ...options});
+}
+
+export const cssSelect = styled(select, `
+  height: 28px;
+  width: 100%;
+  border: 1px solid transparent;
+  cursor: pointer;
+
+  &:hover, &:focus, &.weasel-popup-open, &-active {
+    border: 1px solid ${colors.darkGrey};
+    box-shadow: none;
+  }
+`);
+
+const cssSelectCls = cssSelect.className;
+
+const cssSelectArrow = styled(icon, `
+  margin: 0 2px;
+  pointer-events: none;
+  display: none;
+
+  .${cssSelectCls}:hover &, .${cssSelectCls}:focus &, .weasel-popup-open &, .${cssSelectCls}-active & {
+    display: flex;
+  }
+`);

app/client/aclui/AccessRules.ts

@@ -1,17 +1,20 @@
 /**
  * UI for managing granular ACLs.
  */
+import {aclColumnList} from 'app/client/aclui/ACLColumnList';
+import {aclFormulaEditor} from 'app/client/aclui/ACLFormulaEditor';
+import {aclSelect} from 'app/client/aclui/ACLSelect';
+import {PermissionKey, permissionsWidget} from 'app/client/aclui/PermissionsWidget';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {createObsArray} from 'app/client/lib/koArrayWrap';
 import {reportError, UserError} from 'app/client/models/errors';
 import {TableData} from 'app/client/models/TableData';
-import {PermissionKey, permissionsWidget} from 'app/client/ui/PermissionsWidget';
 import {shadowScroll} from 'app/client/ui/shadowScroll';
 import {bigBasicButton, bigPrimaryButton} from 'app/client/ui2018/buttons';
 import {colors, testId} from 'app/client/ui2018/cssVars';
-import {cssTextInput, textInput} from 'app/client/ui2018/editableLabel';
+import {textInput} from 'app/client/ui2018/editableLabel';
 import {cssIconButton, icon} from 'app/client/ui2018/icons';
-import {autocomplete, menu, menuItemAsync} from 'app/client/ui2018/menus';
+import {IOptionFull, menu, menuItemAsync} from 'app/client/ui2018/menus';
 import {emptyPermissionSet} from 'app/common/ACLPermissions';
 import {PartialPermissionSet, permissionSetToText} from 'app/common/ACLPermissions';
 import {ACLRuleCollection} from 'app/common/ACLRuleCollection';
@@ -20,7 +23,7 @@ import {RulePart, RuleSet, UserAttributeRule} from 'app/common/GranularAccessCla
 import {isHiddenCol} from 'app/common/gristTypes';
 import {isObject} from 'app/common/gutil';
 import {SchemaTypes} from 'app/common/schema';
-import {BaseObservable, Computed, Disposable, MaybeObsArray, MutableObsArray, obsArray, Observable} from 'grainjs';
+import {BaseObservable, Computed, Disposable, MutableObsArray, obsArray, Observable} from 'grainjs';
 import {dom, DomElementArg, styled} from 'grainjs';
 import isEqual = require('lodash/isEqual');
 
@@ -41,6 +44,12 @@ enum RuleStatus {
   CheckPending,
 }
 
+// Option for UserAttribute select() choices. RuleIndex is used to filter for only those user
+// attributes made available by the previous rules.
+interface IAttrOption extends IOptionFull<string> {
+  ruleIndex: number;
+}
+
 /**
  * Top-most container managing state and dom-building for the ACL rule UI.
  */
@@ -63,6 +72,10 @@ export class AccessRules extends Disposable {
   // Array of all UserAttribute rules.
   private _userAttrRules = this.autoDispose(obsArray<ObsUserAttributeRule>());
 
+  // Array of all user-attribute choices created by UserAttribute rules. Used for lookup items in
+  // rules, and for ACLFormula completions.
+  private _userAttrChoices: Computed<IAttrOption[]>;
+
   // Whether the save button should be enabled.
   private _savingEnabled: Computed<boolean>;
 
@@ -85,10 +98,31 @@ export class AccessRules extends Disposable {
 
     this._savingEnabled = Computed.create(this, this._ruleStatus, (use, s) => (s === RuleStatus.ChangedValid));
 
+    this._userAttrChoices = Computed.create(this, this._userAttrRules, (use, rules) => {
+      const result: IAttrOption[] = [
+        {ruleIndex: -1, value: 'Access', label: 'user.Access'},
+        {ruleIndex: -1, value: 'Email', label: 'user.Email'},
+        {ruleIndex: -1, value: 'UserID', label: 'user.UserID'},
+        {ruleIndex: -1, value: 'Name', label: 'user.Name'},
+        {ruleIndex: -1, value: 'Link', label: 'user.Link'},
+        {ruleIndex: -1, value: 'Origin', label: 'user.Origin'},
+      ];
+      for (const [i, rule] of rules.entries()) {
+        const tableId = use(rule.tableId);
+        const name = use(rule.name);
+        for (const colId of this.getValidColIds(tableId) || []) {
+          result.push({ruleIndex: i, value: `${name}.${colId}`, label: `user.${name}.${colId}`});
+        }
+      }
+      return result;
+    });
+
     this.update().catch(reportError);
   }
 
   public get allTableIds() { return this._allTableIds; }
+  public get userAttrRules() { return this._userAttrRules; }
+  public get userAttrChoices() { return this._userAttrChoices; }
 
   /**
    * Replace internal state from the rules in DocData.
@@ -245,7 +279,7 @@ export class AccessRules extends Disposable {
                 cssCell1(cssCell.cls('-rborder'), cssCell.cls('-center'), cssColHeaderCell('Name')),
                 cssCell4(
                   cssColumnGroup(
-                    cssCell1(cssColHeaderCell('User Attribute')),
+                    cssCell1(cssColHeaderCell('Attribute to Look Up')),
                     cssCell1(cssColHeaderCell('Lookup Table')),
                     cssCell1(cssColHeaderCell('Lookup Column')),
                     cssCellIcon(),
@@ -463,9 +497,7 @@ class TableRules extends Disposable {
   }
 
   private _addColumnRuleSet() {
-    this._columnRuleSets.push(ColumnObsRuleSet.create(this._columnRuleSets, this._accessRules, this, undefined, [],
+    this._columnRuleSets.push(ColumnObsRuleSet.create(this._columnRuleSets, this._accessRules, this, undefined, []));
-      {focus: true}
-    ));
   }
 
   private _addDefaultRuleSet() {
@@ -579,6 +611,14 @@ abstract class ObsRuleSet extends Disposable {
       return ['read', 'update', 'create', 'delete', 'schemaEdit'];
     }
   }
+
+  /**
+   * Get valid colIds for the table that this RuleSet is for.
+   */
+  public getValidColIds(): string[] {
+    const tableId = this._tableRules?.tableId;
+    return (tableId && this.accessRules.getValidColIds(tableId)) || [];
+  }
 }
 
 class ColumnObsRuleSet extends ObsRuleSet {
@@ -586,10 +626,9 @@ class ColumnObsRuleSet extends ObsRuleSet {
   public formulaError: Computed<string>;
 
   private _colIds = Observable.create<string[]>(this, this._initialColIds);
-  private _colIdStr = Computed.create(this, (use) => use(this._colIds).join(", "));
 
   constructor(accessRules: AccessRules, tableRules: TableRules, ruleSet: RuleSet|undefined,
-              private _initialColIds: string[], private _options: {focus?: boolean} = {}) {
+              private _initialColIds: string[]) {
     super(accessRules, tableRules, ruleSet);
 
     this.formulaError = Computed.create(this, (use) => {
@@ -607,15 +646,7 @@ class ColumnObsRuleSet extends ObsRuleSet {
   }
 
   public buildResourceDom() {
-    const saveColIds = async (colIdStr: string) => {
+    return aclColumnList(this._colIds, this.getValidColIds());
-      this._colIds.set(colIdStr.split(/\W+/).map(val => val.trim()).filter(Boolean));
-    };
-
-    return cssCellContent(
-      cssInput(this._colIdStr, saveColIds, {placeholder: 'Enter Columns'},
-        (this._options.focus ? (elem) => { setTimeout(() => elem.focus(), 0); } : null),
-      )
-    );
   }
 
   public getColIdList(): string[] {
@@ -640,7 +671,9 @@ class DefaultObsRuleSet extends ObsRuleSet {
   public buildResourceDom() {
     return [
       cssCenterContent.cls(''),
+      cssDefaultLabel(
         dom.text(use => this._haveColumnRules && use(this._haveColumnRules) ? 'All Other' : 'All'),
+      )
     ];
   }
 }
@@ -658,6 +691,8 @@ class ObsUserAttributeRule extends Disposable {
   private _validColIds = Computed.create(this, this._tableId, (use, tableId) =>
     this._accessRules.getValidColIds(tableId) || []);
 
+  private _userAttrChoices: Computed<IAttrOption[]>;
+
   constructor(private _accessRules: AccessRules, private _userAttr?: UserAttributeRule,
               private _options: {focus?: boolean} = {}) {
     super();
@@ -681,7 +716,22 @@ class ObsUserAttributeRule extends Disposable {
 
     // Reset lookupColId when tableId changes, since a colId from a different table would usually be wrong
     this.autoDispose(this._tableId.addListener(() => this._lookupColId.set('')));
+
+    this._userAttrChoices = Computed.create(this, _accessRules.userAttrRules, (use, rules) => {
+      // Filter for only those choices created by previous rules.
+      const index = rules.indexOf(this);
+      const result = use(this._accessRules.userAttrChoices).filter(c => (c.ruleIndex < index));
+
+      // If the currently-selected option isn't one of the choices, insert it too.
+      if (!result.some(choice => (choice.value === this._charId.get()))) {
+        result.unshift({ruleIndex: -1, value: this._charId.get(), label: `user.${this._charId.get()}`});
       }
+      return result;
+    });
+  }
+
+  public get name() { return this._name; }
+  public get tableId() { return this._tableId; }
 
   public buildUserAttrDom() {
     return cssTableRow(
@@ -697,23 +747,20 @@ class ObsUserAttributeRule extends Disposable {
       cssCell4(cssRuleBody.cls(''),
         cssColumnGroup(
           cssCell1(
-            cssInput(this._charId, async (val) => this._charId.set(val),
+            aclSelect(this._charId, this._userAttrChoices,
-              {placeholder: 'Attribute to look up'},
+              {defaultLabel: '[Select Attribute]'}),
             testId('rule-userattr-attr'),
           ),
-          ),
           cssCell1(
-            inputAutocomplete(this._tableId, this._accessRules.allTableIds,
+            aclSelect(this._tableId, this._accessRules.allTableIds,
-              cssTextInput.cls(''), cssInput.cls(''), {placeholder: 'Table'},
+              {defaultLabel: '[Select Table]'}),
             testId('rule-userattr-table'),
           ),
-          ),
           cssCell1(
-            inputAutocomplete(this._lookupColId, this._validColIds,
+            aclSelect(this._lookupColId, this._validColIds,
-              cssTextInput.cls(''), cssInput.cls(''), {placeholder: 'Column'},
+              {defaultLabel: '[Select Column]'}),
             testId('rule-userattr-col'),
           ),
-          ),
           cssCellIcon(
             cssIconButton(icon('Remove'),
               dom.on('click', () => this._accessRules.removeUserAttributes(this)))
@@ -751,9 +798,15 @@ class ObsRulePart extends Disposable {
   // Whether the rule part, and if it's valid or being checked.
   public ruleStatus: Computed<RuleStatus>;
 
-  // Formula to show in the "advanced" UI.
+  // Formula to show in the formula editor.
   private _aclFormula = Observable.create<string>(this, this._rulePart?.aclFormula || "");
 
+  // Rule-specific completions for editing the formula, e.g. "user.Email" or "rec.City".
+  private _completions = Computed.create<string[]>(this, (use) => [
+    ...use(this._ruleSet.accessRules.userAttrChoices).map(opt => opt.label),
+    ...this._ruleSet.getValidColIds().map(colId => `rec.${colId}`),
+  ]);
+
   // The permission bits.
   private _permissions = Observable.create<PartialPermissionSet>(
     this, this._rulePart?.permissions || emptyPermissionSet());
@@ -800,19 +853,21 @@ class ObsRulePart extends Disposable {
         ),
       ),
       cssCell2(
-        cssInput(
+        aclFormulaEditor({
-          this._aclFormula, this._setAclFormula.bind(this),
+          initialValue: this._aclFormula.get(),
-          dom.prop('disabled', this.isBuiltIn()),
+          readOnly: this.isBuiltIn(),
-          dom.prop('placeholder', (use) => {
+          setValue: (value) => this._setAclFormula(value),
+          placeholder: dom.text((use) => {
             return (
               this._ruleSet.isSoleCondition(use, this) ? 'Everyone' :
               this._ruleSet.isLastCondition(use, this) ? 'Everyone Else' :
               'Enter Condition'
             );
           }),
+          getSuggestions: (prefix) => this._completions.get(),
+        }),
         testId('rule-acl-formula'),
       ),
-      ),
       cssCell1(cssCell.cls('-stretch'),
         permissionsWidget(this._ruleSet.getAvailableBits(), this._permissions,
           {disabled: this.isBuiltIn()},
@@ -842,6 +897,7 @@ class ObsRulePart extends Disposable {
   }
 
   private async _setAclFormula(text: string) {
+    if (text === this._aclFormula.get()) { return; }
     this._aclFormula.set(text);
     this._checkPending.set(true);
     this._formulaError.set('');
@@ -855,7 +911,6 @@ class ObsRulePart extends Disposable {
   }
 }
 
-
 /**
  * Produce UserActions to create/update/remove records, to replace data in tableData
  * with newRecords. Records are matched on uniqueId(record), which defaults to returning
@@ -971,24 +1026,6 @@ function getChangedStatus(value: boolean): RuleStatus {
   return value ? RuleStatus.ChangedValid : RuleStatus.Unchanged;
 }
 
-function inputAutocomplete(value: Observable<string>, choices: MaybeObsArray<string>, ...args: DomElementArg[]) {
-  function doSet() {
-    value.set(elem.value);
-  }
-  const elem = autocomplete(
-    dom('input', {type: 'text'},
-      dom.attr('value', value),
-      dom.on('change', doSet),
-      dom.on('blur', doSet),
-      ...args
-    ),
-    choices,
-    {onClick: doSet},
-  );
-  dom.onKeyElem(elem, 'keydown', {Enter: doSet});
-  return elem;
-}
-
 const cssOuter = styled('div', `
   height: 100%;
   width: 100%;
@@ -1033,15 +1070,15 @@ const cssInput = styled(textInput, `
     border: 1px solid ${colors.darkGrey};
   }
   &:focus {
-    box-shadow: inset 0 0 0 1px var(--grist-color-cursor);
+    box-shadow: inset 0 0 0 1px ${colors.cursor};
-    border: 1px solid var(--grist-color-cursor);
+    border-color: ${colors.cursor};
     cursor: unset;
   }
   &[disabled] {
     color: ${colors.dark};
     background-color: ${colors.mediumGreyOpaque};
     box-shadow: unset;
-    border: unset;
+    border-color: transparent;
   }
 `);
 
@@ -1118,7 +1155,6 @@ const cssColumnGroup = styled('div', `
 const cssRuleBody = styled('div', `
   display: flex;
   flex-direction: column;
-  justify-content: center;
   gap: 4px;
   margin: 4px 0;
 `);
@@ -1132,3 +1168,8 @@ const cssCenterContent = styled('div', `
   align-items: center;
   justify-content: center;
 `);
+
+const cssDefaultLabel = styled('div', `
+  color: ${colors.slate};
+  font-weight: bold;
+`);

app/client/components/AceEditor.js

@@ -1,10 +1,10 @@
 var ace = require('brace');
-var ko = require('knockout');
 var _ = require('underscore');
 // Used to load python language settings and 'chrome' ace style
 require('brace/mode/python');
 require('brace/theme/chrome');
 require('brace/ext/language_tools');
+var {setupAceEditorCompletions} = require('./AceEditorCompletions');
 var dom = require('../lib/dom');
 var dispose = require('../lib/dispose');
 var modelUtil = require('../models/modelUtil');
@@ -168,61 +168,11 @@ AceEditor.prototype._setup = function() {
   // Standard editor setup
   this.editor = this.autoDisposeWith('destroy', ace.edit(this.editorDom));
   if (this.gristDoc) {
-    // Add some autocompletion with partial access to document
+    const getSuggestions = (prefix) => {
-    const aceLanguageTools = ace.acequire('ace/ext/language_tools');
+      const tableId = this.gristDoc.viewModel.activeSection().table().tableId();
-    const gristDoc = this.gristDoc;
+      return this.gristDoc.docComm.autocomplete(prefix, tableId);
-    aceLanguageTools.setCompleters([]);
+    };
-    aceLanguageTools.addCompleter({
+    setupAceEditorCompletions(this.editor, {getSuggestions});
-      // Default regexp stops at periods, which doesn't let autocomplete
-      // work on members.  So we expand it to include periods.
-      // We also include $, which grist uses for column names.
-      identifierRegexps: [/[a-zA-Z_0-9.$\u00A2-\uFFFF]/],
-
-      // For autocompletion we ship text to the sandbox and run standard completion there.
-      getCompletions: function(editor, session, pos, prefix, callback) {
-        if (prefix.length === 0) { callback(null, []); return; }
-        const tableId = gristDoc.viewModel.activeSection().table().tableId();
-        gristDoc.docComm.autocomplete(prefix, tableId)
-        .then(suggestions => {
-          // ACE autocompletions are very poorly documented. This is somewhat helpful:
-          // https://prog.world/implementing-code-completion-in-ace-editor/
-          callback(null, suggestions.map(suggestion => {
-            if (Array.isArray(suggestion)) {
-              const [funcname, argSpec, isGrist] = suggestion;
-              const meta = isGrist ? 'grist' : 'python';
-              return {value: funcname + '(', caption: funcname + argSpec, score: 1, meta, funcname};
-            } else {
-              return {value: suggestion, score: 1, meta: "python"};
-            }
-          }));
-        });
-      },
-    });
-
-    // Create Autocomplete object at this point so we can turn autoSelect off.
-    // There doesn't seem to be any way to get ace to respect autoSelect otherwise.
-    // It is important for autoSelect to be off so that hitting enter doesn't automatically
-    // use a suggestion, a change of behavior that doesn't seem particularly desirable and
-    // which also breaks several existing tests.
-    const {Autocomplete} = ace.acequire('ace/autocomplete'); // lives in brace/ext/language_tools
-    const completer = new Autocomplete();
-    this.editor.completer = completer;
-    this.editor.completer.autoSelect = false;
-    aceCompleterAddHelpLinks(completer);
-
-    // Explicitly destroy the auto-completer on disposal, since it doesn't not remove the element
-    // it adds to body even when it detaches itself. Ace's AutoCompleter doesn't expose any
-    // interface for this, so this takes some hacking. (One reason for this is that Ace seems to
-    // expect that a single AutoCompleter would be used for all editor instances.)
-    this.autoDisposeCallback(() => {
-      if (completer.editor) {
-        completer.detach();
-      }
-      if (completer.popup) {
-        completer.popup.destroy();                // This is not enough, but seems relevant to call.
-        ko.removeNode(completer.popup.container); // Removes the element and cleans up JQuery state if any.
-      }
-    });
   }
   this.editor.setOptions({
     enableLiveAutocompletion: true,   // use autocompletion without needing special activation.
@@ -296,107 +246,4 @@ AceEditor.makeRange = function(a,b,c,d) {
   return new _RangeConstructor(a,b,c,d);
 };
 
-/**
- * When autocompleting a known function (with funcname received from the server call), turn the
- * function name into a link to Grist documentation.
- *
- * ACE autocomplete is poorly documented, and poorly customizable, so this is accomplished by
- * monkey-patching it. Further, the only text styling is done via styled tokens, but we can style
- * them to look like links, and handle clicks to open the destination URL.
- *
- * This implementation relies a lot on the details of the implementation in
- * node_modules/brace/ext/language_tools.js. Updates to brace module may easily break it.
- */
-function aceCompleterAddHelpLinks(completer) {
-  // Replace the $init function in order to intercept the creation of the autocomplete popup.
-  const init = completer.$init;
-  completer.$init = function() {
-    const popup = init.apply(this, arguments);
-    customizeAceCompleterPopup(this, popup);
-    return popup;
-  };
-}
-
-function customizeAceCompleterPopup(completer, popup) {
-  // Replace the $tokenizeRow function to produce customized tokens to style the link part.
-  const origTokenize = popup.session.bgTokenizer.$tokenizeRow;
-  popup.session.bgTokenizer.$tokenizeRow = function(row) {
-    const tokens = origTokenize(row);
-    return retokenizeAceCompleterRow(popup.data[row], tokens);
-  };
-
-  // Replace the click handler with one that handles link clicks.
-  popup.removeAllListeners("click");
-  popup.on("click", function(e) {
-    if (!maybeAceCompleterLinkClick(e)) {
-      completer.insertMatch();
-    }
-    e.stop();
-  });
-}
-
-function retokenizeAceCompleterRow(rowData, tokens) {
-  if (!rowData.funcname) {
-    // Not a special completion, pass through the result of ACE's original tokenizing.
-    return tokens;
-  }
-
-  // ACE's original tokenizer splits rowData.caption into tokens to highlight matching portions.
-  // We jump in, and further divide the tokens so that those that form the link get an extra CSS
-  // class. ACE's will turn token.type into CSS classes by splitting the type on "." and prefixing
-  // the resulting substrings with "ace_".
-
-  // Funcname may be the recognized name itself (e.g. "UPPER"), or a method (like
-  // "Table1.lookupOne"), in which case only the portion after the dot is the recognized name.
-
-  // Figure out the portion that should be linkified.
-  const dot = rowData.funcname.lastIndexOf(".");
-  const linkStart = dot < 0 ? 0 : dot + 1;
-  const linkEnd = rowData.funcname.length;
-
-  const newTokens = [];
-
-  // Include into new tokens a special token that will be hidden, but include the link URL. On
-  // click, we find it to know what URL to open.
-  const href = 'https://support.getgrist.com/functions/#' +
-    rowData.funcname.slice(linkStart, linkEnd).toLowerCase();
-  newTokens.push({value: href, type: 'grist_link_hidden'});
-
-  // Go through tokens, splitting them if needed, and modifying those that form the link part.
-  let position = 0;
-  for (const t of tokens) {
-    // lStart/lEnd are indices of the link within the token, possibly negative.
-    const lStart = linkStart - position, lEnd = linkEnd - position;
-    if (lStart > 0) {
-      const beforeLink = t.value.slice(0, lStart);
-      newTokens.push({value: beforeLink, type: t.type});
-    }
-    if (lEnd > 0) {
-      const inLink = t.value.slice(Math.max(0, lStart), lEnd);
-      const newType = t.type + (t.type ? '.' : '') + 'grist_link';
-      newTokens.push({value: inLink, type: newType});
-    }
-    if (lEnd < t.value.length) {
-      const afterLink = t.value.slice(lEnd);
-      newTokens.push({value: afterLink, type: t.type});
-    }
-    position += t.value.length;
-  }
-  return newTokens;
-}
-
-// On any click on AceCompleter popup, we check if we happened to click .ace_grist_link class. If
-// so, we should be able to find the URL and open another window to it.
-function maybeAceCompleterLinkClick(event) {
-  const tgt = event.domEvent.target;
-  if (tgt && tgt.matches('.ace_grist_link')) {
-    const dest = tgt.parentElement.querySelector('.ace_grist_link_hidden');
-    if (dest) {
-      window.open(dest.textContent, "_blank");
-      return true;
-    }
-  }
-  return false;
-}
-
 module.exports = AceEditor;

app/client/components/AceEditorCompletions.ts

@@ -0,0 +1,189 @@
+import * as ace from 'brace';
+
+// Suggestion may be a string, or a tuple [funcname, argSpec, isGrist], where:
+//  - funcname (e.g. "DATEADD") will be auto-completed with "(", AND linked to Grist
+//    documentation.
+//  - argSpec (e.g. "(start_date, days=0, ...)") is to be shown as autocomplete caption.
+//  - isGrist determines whether to tag this suggestion as "grist" or "python".
+export type ISuggestion = string | [string, string, boolean];
+
+export interface ICompletionOptions {
+  getSuggestions(prefix: string): Promise<ISuggestion[]>;
+}
+
+const completionOptions = new WeakMap<ace.Editor, ICompletionOptions>();
+
+export function setupAceEditorCompletions(editor: ace.Editor, options: ICompletionOptions) {
+  initCustomCompleter();
+  completionOptions.set(editor, options);
+
+  // Create Autocomplete object at this point so we can turn autoSelect off.
+  // There doesn't seem to be any way to get ace to respect autoSelect otherwise.
+  // It is important for autoSelect to be off so that hitting enter doesn't automatically
+  // use a suggestion, a change of behavior that doesn't seem particularly desirable and
+  // which also breaks several existing tests.
+  const {Autocomplete} = ace.acequire('ace/autocomplete'); // lives in brace/ext/language_tools
+  const completer = new Autocomplete();
+  completer.autoSelect = false;
+  (editor as any).completer = completer;
+
+  aceCompleterAddHelpLinks(completer);
+
+  // Explicitly destroy the auto-completer on disposal, since it doesn't not remove the element
+  // it adds to body even when it detaches itself. Ace's AutoCompleter doesn't expose any
+  // interface for this, so this takes some hacking. (One reason for this is that Ace seems to
+  // expect that a single AutoCompleter would be used for all editor instances.)
+  editor.on('destroy', () => {
+    if (completer.editor) {
+      completer.detach();
+    }
+    if (completer.popup) {
+      completer.popup.destroy();                // This is not enough, but seems relevant to call.
+      completer.popup.container.remove();       // Removes the element from DOM.
+    }
+  });
+}
+
+let _initialized = false;
+function initCustomCompleter() {
+  if (_initialized) { return; }
+  _initialized = true;
+
+  // Add some autocompletion with partial access to document
+  const aceLanguageTools = ace.acequire('ace/ext/language_tools');
+  aceLanguageTools.setCompleters([]);
+  aceLanguageTools.addCompleter({
+    // Default regexp stops at periods, which doesn't let autocomplete
+    // work on members.  So we expand it to include periods.
+    // We also include $, which grist uses for column names.
+    identifierRegexps: [/[a-zA-Z_0-9.$\u00A2-\uFFFF]/],
+
+    // For autocompletion we ship text to the sandbox and run standard completion there.
+    async getCompletions(editor: ace.Editor, session: ace.IEditSession, pos: number, prefix: string, callback: any) {
+      const options = completionOptions.get(editor);
+      if (!options || prefix.length === 0) { callback(null, []); return; }
+      const suggestions = await options.getSuggestions(prefix);
+      // ACE autocompletions are very poorly documented. This is somewhat helpful:
+      // https://prog.world/implementing-code-completion-in-ace-editor/
+      callback(null, suggestions.map(suggestion => {
+        if (Array.isArray(suggestion)) {
+          const [funcname, argSpec, isGrist] = suggestion;
+          const meta = isGrist ? 'grist' : 'python';
+          return {value: funcname + '(', caption: funcname + argSpec, score: 1, meta, funcname};
+        } else {
+          return {value: suggestion, score: 1, meta: "python"};
+        }
+      }));
+    },
+  });
+}
+
+/**
+ * When autocompleting a known function (with funcname received from the server call), turn the
+ * function name into a link to Grist documentation.
+ *
+ * This is only applied for items returned from getCompletions() that include a our custom
+ * `funcname` attribute.
+ *
+ * ACE autocomplete is poorly documented, and poorly customizable, so this is accomplished by
+ * monkey-patching it. Further, the only text styling is done via styled tokens, but we can style
+ * them to look like links, and handle clicks to open the destination URL.
+ *
+ * This implementation relies a lot on the details of the implementation in
+ * node_modules/brace/ext/language_tools.js. Updates to brace module may easily break it.
+ */
+function aceCompleterAddHelpLinks(completer: any) {
+  // Replace the $init function in order to intercept the creation of the autocomplete popup.
+  const init = completer.$init;
+  completer.$init = function() {
+    const popup = init.apply(this, arguments);
+    customizeAceCompleterPopup(this, popup);
+    return popup;
+  };
+}
+
+function customizeAceCompleterPopup(completer: any, popup: any) {
+  // Replace the $tokenizeRow function to produce customized tokens to style the link part.
+  const origTokenize = popup.session.bgTokenizer.$tokenizeRow;
+  popup.session.bgTokenizer.$tokenizeRow = function(row: any) {
+    const tokens = origTokenize(row);
+    return retokenizeAceCompleterRow(popup.data[row], tokens);
+  };
+
+  // Replace the click handler with one that handles link clicks.
+  popup.removeAllListeners("click");
+  popup.on("click", function(e: any) {
+    if (!maybeAceCompleterLinkClick(e.domEvent)) {
+      completer.insertMatch();
+    }
+    e.stop();
+  });
+}
+
+interface TokenInfo extends ace.TokenInfo {
+  type: string;
+}
+
+function retokenizeAceCompleterRow(rowData: any, tokens: TokenInfo[]): TokenInfo[] {
+  if (!rowData.funcname) {
+    // Not a special completion, pass through the result of ACE's original tokenizing.
+    return tokens;
+  }
+
+  // ACE's original tokenizer splits rowData.caption into tokens to highlight matching portions.
+  // We jump in, and further divide the tokens so that those that form the link get an extra CSS
+  // class. ACE's will turn token.type into CSS classes by splitting the type on "." and prefixing
+  // the resulting substrings with "ace_".
+
+  // Funcname may be the recognized name itself (e.g. "UPPER"), or a method (like
+  // "Table1.lookupOne"), in which case only the portion after the dot is the recognized name.
+
+  // Figure out the portion that should be linkified.
+  const dot = rowData.funcname.lastIndexOf(".");
+  const linkStart = dot < 0 ? 0 : dot + 1;
+  const linkEnd = rowData.funcname.length;
+
+  const newTokens = [];
+
+  // Include into new tokens a special token that will be hidden, but include the link URL. On
+  // click, we find it to know what URL to open.
+  const href = 'https://support.getgrist.com/functions/#' +
+    rowData.funcname.slice(linkStart, linkEnd).toLowerCase();
+  newTokens.push({value: href, type: 'grist_link_hidden'});
+
+  // Go through tokens, splitting them if needed, and modifying those that form the link part.
+  let position = 0;
+  for (const t of tokens) {
+    // lStart/lEnd are indices of the link within the token, possibly negative.
+    const lStart = linkStart - position, lEnd = linkEnd - position;
+    if (lStart > 0) {
+      const beforeLink = t.value.slice(0, lStart);
+      newTokens.push({value: beforeLink, type: t.type});
+    }
+    if (lEnd > 0) {
+      const inLink = t.value.slice(Math.max(0, lStart), lEnd);
+      const newType = t.type + (t.type ? '.' : '') + 'grist_link';
+      newTokens.push({value: inLink, type: newType});
+    }
+    if (lEnd < t.value.length) {
+      const afterLink = t.value.slice(lEnd);
+      newTokens.push({value: afterLink, type: t.type});
+    }
+    position += t.value.length;
+  }
+  return newTokens;
+}
+
+// On any click on AceCompleter popup, we check if we happened to click .ace_grist_link class. If
+// so, we should be able to find the URL and open another window to it.
+function maybeAceCompleterLinkClick(domEvent: Event) {
+  const tgt = domEvent.target as HTMLElement;
+  if (tgt && tgt.matches('.ace_grist_link')) {
+    const dest = tgt.parentElement?.querySelector('.ace_grist_link_hidden');
+    if (dest) {
+      window.open(dest.textContent!, "_blank");
+      return true;
+    }
+  }
+  return false;
+}

app/client/components/GristDoc.ts

@@ -3,6 +3,7 @@
  */
 // tslint:disable:no-console
 
+import {AccessRules} from 'app/client/aclui/AccessRules';
 import {ActionLog} from 'app/client/components/ActionLog';
 import * as CodeEditorPanel from 'app/client/components/CodeEditorPanel';
 import * as commands from 'app/client/components/commands';
@@ -29,7 +30,6 @@ import {DocPageModel} from 'app/client/models/DocPageModel';
 import {UserError} from 'app/client/models/errors';
 import {urlState} from 'app/client/models/gristUrlState';
 import {QuerySetManager} from 'app/client/models/QuerySet';
-import {AccessRules} from 'app/client/ui/AccessRules';
 import {App} from 'app/client/ui/App';
 import {DocHistory} from 'app/client/ui/DocHistory';
 import {IPageWidget, toPageWidget} from 'app/client/ui/PageWidgetPicker';

app/client/ui2018/icons.ts

@@ -87,7 +87,7 @@ export const cssIconButton = styled('div', `
   line-height: 0px;
   cursor: default;
   --icon-color: ${colors.slate};
-  &:hover {
+  &:hover, &.weasel-popup-open {
     background-color: ${colors.darkGrey};
     --icon-color: ${colors.slate};
   }