(core) Form kanban tasks

Summary:
- Open all links in a new tab
- Excluding not filled columns (to fix trigger formulas)
- Fixed Ref/RefList submission
- Removing redundant type definitions for Box
- Adding header menu item
- Default empty values in select control

Test Plan: Updated

Reviewers: georgegevoian

Reviewed By: georgegevoian

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

app/client/components/Forms/Columns.ts

@@ -1,10 +1,11 @@
 import {buildEditor} from 'app/client/components/Forms/Editor';
 import {buildMenu} from 'app/client/components/Forms/Menu';
-import {Box, BoxModel} from 'app/client/components/Forms/Model';
+import {BoxModel} from 'app/client/components/Forms/Model';
 import * as style from 'app/client/components/Forms/styles';
 import {makeTestId} from 'app/client/lib/domUtils';
 import {icon} from 'app/client/ui2018/icons';
 import * as menus from 'app/client/ui2018/menus';
+import {Box} from 'app/common/Forms';
 import {inlineStyle, not} from 'app/common/gutil';
 import {bundleChanges, Computed, dom, IDomArgs, MultiHolder, Observable, styled} from 'grainjs';
 
@@ -30,8 +31,7 @@ export class ColumnsModel extends BoxModel {
     if (!this.parent) { throw new Error('No parent'); }
 
     // We need to remove it from the parent, so find it first.
-    const droppedId = dropped.id;
-    const droppedRef = this.root().get(droppedId);
+    const droppedRef = dropped.id ? this.root().get(dropped.id) : null;
 
     // Now we simply insert it after this box.
     droppedRef?.removeSelf();
@@ -165,6 +165,10 @@ export class PlaceholderModel extends BoxModel {
   }
 }
 
+export function Paragraph(text: string, alignment?: 'left'|'right'|'center'): Box {
+  return {type: 'Paragraph', text, alignment};
+}
+
 export function Placeholder(): Box {
   return {type: 'Placeholder'};
 }

app/client/components/Forms/Field.ts

@@ -1,12 +1,13 @@
 import {buildEditor} from 'app/client/components/Forms/Editor';
 import {FormView} from 'app/client/components/Forms/FormView';
-import {Box, BoxModel, ignoreClick} from 'app/client/components/Forms/Model';
+import {BoxModel, ignoreClick} from 'app/client/components/Forms/Model';
 import * as css from 'app/client/components/Forms/styles';
 import {stopEvent} from 'app/client/lib/domUtils';
 import {refRecord} from 'app/client/models/DocModel';
 import {autoGrow} from 'app/client/ui/forms';
 import {squareCheckbox} from 'app/client/ui2018/checkbox';
 import {colors} from 'app/client/ui2018/cssVars';
+import {Box} from 'app/common/Forms';
 import {Constructor} from 'app/common/gutil';
 import {
   BindableValue,
@@ -63,7 +64,11 @@ export class FieldModel extends BoxModel {
    * Field row id.
    */
   public get leaf() {
-    return this.props['leaf'] as Observable<number>;
+    return this.prop('leaf') as Observable<number>;
+  }
+
+  public get required() {
+    return this.prop('formRequired', false) as Observable<boolean|undefined>;
   }
 
   /**
@@ -260,41 +265,47 @@ class TextModel extends Question {
 }
 
 class ChoiceModel extends Question {
-  public renderInput() {
+  protected choices: Computed<string[]> = Computed.create(this, use => {
+    // Read choices from field.
+    const list = use(use(use(this.model.field).origCol).widgetOptionsJson.prop('choices')) || [];
+
+    // Make sure it is array of strings.
+    if (!Array.isArray(list) || list.some((v) => typeof v !== 'string')) {
+      return [];
+    }
+    return list;
+  });
+
+  protected choicesWithEmpty = Computed.create(this, use => {
+    const list = Array.from(use(this.choices));
+    // Add empty choice if not present.
+    if (list.length === 0 || list[0] !== '') {
+      list.unshift('');
+    }
+    return list;
+  });
+
+  public renderInput(): HTMLElement {
     const field = this.model.field;
-    const choices: Computed<string[]> = Computed.create(this, use => {
-      return use(use(use(field).origCol).widgetOptionsJson.prop('choices')) || [];
-    });
-    const typedChoices = Computed.create(this, use => {
-      const value = use(choices);
-      // Make sure it is array of strings.
-      if (!Array.isArray(value) || value.some((v) => typeof v !== 'string')) {
-        return [];
-      }
-      return value;
-    });
     return css.cssSelect(
       {tabIndex: "-1"},
       ignoreClick,
       dom.prop('name', use => use(use(field).colId)),
-      dom.forEach(typedChoices, (choice) => dom('option', choice, {value: choice})),
+      dom.forEach(this.choicesWithEmpty, (choice) => dom('option', choice, {value: choice})),
     );
   }
 }
 
-class ChoiceListModel extends Question {
+class ChoiceListModel extends ChoiceModel {
   public renderInput() {
     const field = this.model.field;
-    const choices: Computed<string[]> = Computed.create(this, use => {
-      return use(use(use(field).origCol).widgetOptionsJson.prop('choices')) || [];
-    });
     return dom('div',
       dom.prop('name', use => use(use(field).colId)),
-      dom.forEach(choices, (choice) => css.cssCheckboxLabel(
+      dom.forEach(this.choices, (choice) => css.cssCheckboxLabel(
         squareCheckbox(observable(false)),
         choice
       )),
-      dom.maybe(use => use(choices).length === 0, () => [
+      dom.maybe(use => use(this.choices).length === 0, () => [
         dom('div', 'No choices defined'),
       ]),
     );
@@ -393,12 +404,19 @@ class RefListModel extends Question {
 }
 
 class RefModel extends RefListModel {
+  protected withEmpty = Computed.create(this, use => {
+    const list = Array.from(use(this.choices));
+    // Add empty choice if not present.
+    list.unshift([0, '']);
+    return list;
+  });
+
   public renderInput() {
     return css.cssSelect(
       {tabIndex: "-1"},
       ignoreClick,
       dom.prop('name', this.model.colId),
-      dom.forEach(this.choices, (choice) => dom('option', String(choice[1] ?? ''), {value: String(choice[0])})),
+      dom.forEach(this.withEmpty, (choice) => dom('option', String(choice[1] ?? ''), {value: String(choice[0])})),
     );
   }
 }

app/client/components/Forms/FormView.ts

@@ -3,7 +3,7 @@ import * as commands from 'app/client/components/commands';
 import {Cursor} from 'app/client/components/Cursor';
 import * as components from 'app/client/components/Forms/elements';
 import {NewBox} from 'app/client/components/Forms/Menu';
-import {Box, BoxModel, BoxType, LayoutModel, parseBox, Place} from 'app/client/components/Forms/Model';
+import {BoxModel, LayoutModel, parseBox, Place} from 'app/client/components/Forms/Model';
 import * as style from 'app/client/components/Forms/styles';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {copyToClipboard} from 'app/client/lib/clipboardUtils';
@@ -20,7 +20,7 @@ import {showTransientTooltip} from 'app/client/ui/tooltips';
 import {cssButton} from 'app/client/ui2018/buttons';
 import {icon} from 'app/client/ui2018/icons';
 import {confirmModal} from 'app/client/ui2018/modals';
-import {INITIAL_FIELDS_COUNT} from "app/common/Forms";
+import {Box, BoxType, INITIAL_FIELDS_COUNT} from "app/common/Forms";
 import {Events as BackboneEvents} from 'backbone';
 import {Computed, dom, Holder, IDomArgs, Observable} from 'grainjs';
 import defaults from 'lodash/defaults';

app/client/components/Forms/Menu.ts

@@ -1,12 +1,13 @@
 import {allCommands} from 'app/client/components/commands';
+import * as components from 'app/client/components/Forms/elements';
 import {FormView} from 'app/client/components/Forms/FormView';
-import {BoxModel, BoxType, Place} from 'app/client/components/Forms/Model';
+import {BoxModel, Place} from 'app/client/components/Forms/Model';
 import {makeTestId, stopEvent} from 'app/client/lib/domUtils';
 import {FocusLayer} from 'app/client/lib/FocusLayer';
 import {makeT} from 'app/client/lib/localization';
 import {getColumnTypes as getNewColumnTypes} from 'app/client/ui/GridViewMenus';
 import * as menus from 'app/client/ui2018/menus';
-import * as components from 'app/client/components/Forms/elements';
+import {BoxType} from 'app/common/Forms';
 import {Computed, dom, IDomArgs, MultiHolder} from 'grainjs';
 
 const t = makeT('FormView');
@@ -140,8 +141,9 @@ export function buildMenu(props: Props, ...args: IDomArgs<HTMLElement>): IDomArg
           ]),
           menus.menuDivider(),
           menus.menuSubHeader(t('Building blocks')),
-          menus.menuItem(where(struct('Columns')), menus.menuIcon('Columns'), t("Columns")),
+          menus.menuItem(where(struct('Header')), menus.menuIcon('Headband'), t("Header")),
           menus.menuItem(where(struct('Paragraph')), menus.menuIcon('Paragraph'), t("Paragraph")),
+          menus.menuItem(where(struct('Columns')), menus.menuIcon('Columns'), t("Columns")),
           menus.menuItem(where(struct('Separator')), menus.menuIcon('Separator'), t("Separator")),
         ];
       };

app/client/components/Forms/Model.ts

@@ -1,22 +1,10 @@
 import * as elements from 'app/client/components/Forms/elements';
 import {FormView} from 'app/client/components/Forms/FormView';
+import {Box, BoxType} from 'app/common/Forms';
 import {bundleChanges, Computed, Disposable, dom, IDomArgs, MutableObsArray, obsArray, Observable} from 'grainjs';
 import {v4 as uuidv4} from 'uuid';
 
 type Callback = () => Promise<void>;
-export type BoxType =   'Paragraph' | 'Section' | 'Columns' | 'Submit'
-                      | 'Placeholder' | 'Layout' | 'Field' | 'Label'
-                      | 'Separator'
-                      ;
-
-/**
- * Box model is a JSON that represents a form element. Every element can be converted to this element and every
- * ViewModel should be able to read it and built itself from it.
- */
-export interface Box extends Record<string, any> {
-  type: BoxType,
-  children?: Array<Box>,
-}
 
 /**
  * A place where to insert a box.
@@ -59,10 +47,6 @@ export abstract class BoxModel extends Disposable {
    * List of children boxes.
    */
   public children: MutableObsArray<BoxModel>;
-  /**
-   * Any other dynamically added properties (that are not concrete fields in the derived classes)
-   */
-  public props: Record<string, Observable<any>> = {};
   /**
    * Publicly exposed state if the element was just cut.
    * TODO: this should be moved to FormView, as this model doesn't care about that.
@@ -70,6 +54,11 @@ export abstract class BoxModel extends Disposable {
   public cut = Observable.create(this, false);
 
   public selected: Observable<boolean>;
+
+  /**
+   * Any other dynamically added properties (that are not concrete fields in the derived classes)
+   */
+  private _props: Record<string, Observable<any>> = {};
   /**
    * Don't use it directly, use the BoxModel.new factory method instead.
    */
@@ -163,7 +152,7 @@ export abstract class BoxModel extends Disposable {
     }
     // We need to remove it from the parent, so find it first.
     const droppedId = dropped.id;
-    const droppedRef = this.root().get(droppedId);
+    const droppedRef = droppedId ? this.root().get(droppedId) : null;
     if (droppedRef) {
       droppedRef.removeSelf();
     }
@@ -171,14 +160,14 @@ export abstract class BoxModel extends Disposable {
   }
 
   public prop(name: string, defaultValue?: any) {
-    if (!this.props[name]) {
-      this.props[name] = Observable.create(this, defaultValue ?? null);
+    if (!this._props[name]) {
+      this._props[name] = Observable.create(this, defaultValue ?? null);
     }
-    return this.props[name];
+    return this._props[name];
   }
 
   public hasProp(name: string) {
-    return this.props.hasOwnProperty(name);
+    return this._props.hasOwnProperty(name);
   }
 
   public async save(before?: () => Promise<void>): Promise<void> {
@@ -306,7 +295,8 @@ export abstract class BoxModel extends Disposable {
     }
 
     // Update all properties of self.
-    for (const key in boxDef) {
+    for (const someKey in boxDef) {
+      const key = someKey as keyof Box;
       // Skip some keys.
       if (key === 'id' || key === 'type' || key === 'children') { continue; }
       // Skip any inherited properties.
@@ -347,7 +337,7 @@ export abstract class BoxModel extends Disposable {
       id: this.id,
       type: this.type,
       children: this.children.get().map(child => child?.toJSON() || null),
-      ...(Object.fromEntries(Object.entries(this.props).map(([key, val]) => [key, val.get()]))),
+      ...(Object.fromEntries(Object.entries(this._props).map(([key, val]) => [key, val.get()]))),
     };
   }
 

app/client/components/Forms/Section.ts

@@ -1,9 +1,10 @@
 import * as style from './styles';
 import {buildEditor} from 'app/client/components/Forms/Editor';
 import {buildMenu} from 'app/client/components/Forms/Menu';
-import {Box, BoxModel} from 'app/client/components/Forms/Model';
-import {dom, styled} from 'grainjs';
+import {BoxModel} from 'app/client/components/Forms/Model';
 import {makeTestId} from 'app/client/lib/domUtils';
+import {Box} from 'app/common/Forms';
+import {dom, styled} from 'grainjs';
 
 const testId = makeTestId('test-forms-');
 
@@ -53,8 +54,7 @@ export class SectionModel extends BoxModel {
       return null;
     }
     // We need to remove it from the parent, so find it first.
-    const droppedId = dropped.id;
-    const droppedRef = this.root().get(droppedId);
+    const droppedRef = dropped.id ? this.root().get(dropped.id) : null;
     if (droppedRef) {
       droppedRef.removeSelf();
     }

app/client/components/Forms/elements.ts

@@ -1,5 +1,5 @@
-import {Columns, Placeholder} from 'app/client/components/Forms/Columns';
-import {Box, BoxType} from 'app/client/components/Forms/Model';
+import {Columns, Paragraph, Placeholder} from 'app/client/components/Forms/Columns';
+import {Box, BoxType} from 'app/common/Forms';
 /**
  * Add any other element you whish to use in the form here.
  * FormView will look for any exported BoxModel derived class in format `type` + `Model`, and use It
@@ -16,10 +16,8 @@ export function defaultElement(type: BoxType): Box {
   switch(type) {
     case 'Columns': return Columns();
     case 'Placeholder': return Placeholder();
-    case 'Separator': return {
-      type: 'Paragraph',
-      text: '---',
-    };
+    case 'Separator': return Paragraph('---');
+    case 'Header': return Paragraph('## **Header**', 'center');
     default: return {type};
   }
 }