(core) Required fields in forms and bug fixes

Summary:
- Adding little green asterisk at the end of field title.
- Fixing bug on columns component. Adding paragraph as a column and then selecting it was throwing error in the RightPanel
- Fixing boolean column bug in the editor
- Adding (--Choose--) placeholder for dropdowns
- Fixing columns logic: Dragging and dropping columns can unexpectedly add more columns.
- Added favicon and default page title
- Added svg to sync file for electron.

Test Plan: Updated

Reviewers: paulfitz

Reviewed By: paulfitz

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

app/client/components/Forms/Columns.ts

@@ -26,22 +26,29 @@ export class ColumnsModel extends BoxModel {
     this.replace(box, Placeholder());
   }
 
-  // Dropping a box on a column will replace it.
+  // Dropping a box on this component (Columns) directly will add it as a new column.
   public accept(dropped: Box): BoxModel {
     if (!this.parent) { throw new Error('No parent'); }
 
     // We need to remove it from the parent, so find it first.
-    const droppedRef = dropped.id ? this.root().get(dropped.id) : null;
+    const droppedRef = dropped.id ? this.root().find(dropped.id) : null;
+
+    // If this is already my child, don't do anything.
+    if (droppedRef && droppedRef.parent === this) {
+      return droppedRef;
+    }
 
-    // Now we simply insert it after this box.
     droppedRef?.removeSelf();
 
-    return this.parent.replace(this, dropped);
+    return this.append(dropped);
   }
 
   public render(...args: IDomArgs<HTMLElement>): HTMLElement {
-    // Now render the dom.
+    const dragHover = Observable.create(null, false);
+
     const content: HTMLElement = style.cssColumns(
+      dom.autoDispose(dragHover),
+
       // Pass column count as a css variable (to style the grid).
       inlineStyle(`--css-columns-count`, this._columnCount),
 
@@ -52,11 +59,27 @@ export class ColumnsModel extends BoxModel {
       }),
 
       // Append + button at the end.
-      dom('div',
+      cssPlaceholder(
         testId('add'),
         icon('Plus'),
         dom.on('click', () => this.placeAfterListChild()(Placeholder())),
-        style.cssColumn.cls('-add-button')
+        style.cssColumn.cls('-add-button'),
+        style.cssColumn.cls('-drag-over', dragHover),
+
+        dom.on('dragleave', (ev) => {
+          ev.stopPropagation();
+          ev.preventDefault();
+          // Just remove the style and stop propagation.
+          dragHover.set(false);
+        }),
+        dom.on('dragover', (ev) => {
+          // As usual, prevent propagation.
+          ev.stopPropagation();
+          ev.preventDefault();
+          // Here we just change the style of the element.
+          ev.dataTransfer!.dropEffect = "move";
+          dragHover.set(true);
+        }),
       ),
 
       ...args,
@@ -91,6 +114,8 @@ export class PlaceholderModel extends BoxModel {
     return cssPlaceholder(
       style.cssDrop(),
       testId('Placeholder'),
+      testId('element'),
+      dom.attr('data-box-model', String(box.type)),
       dom.autoDispose(scope),
 
       style.cssColumn.cls('-drag-over', dragHover),
@@ -133,12 +158,20 @@ export class PlaceholderModel extends BoxModel {
 
         // We need to remove it from the parent, so find it first.
         const droppedId = dropped.id;
-        const droppedRef = box.root().get(droppedId);
+        const droppedRef = box.root().find(droppedId);
-        if (!droppedRef) { return; }
+
+        // Make sure that the dropped stuff is not our parent.
+        if (droppedRef) {
+          for(const child of droppedRef.traverse()) {
+            if (this === child) {
+              return;
+            }
+          }
+        }
 
         // Now we simply insert it after this box.
         bundleChanges(() => {
-          droppedRef.removeSelf();
+          droppedRef?.removeSelf();
           const parent = box.parent!;
           parent.replace(box, dropped);
           parent.save().catch(reportError);
@@ -179,4 +212,8 @@ export function Columns(): Box {
 
 const cssPlaceholder = styled('div', `
   position: relative;
+  & * {
+    /* Otherwise it will emit drag events that we want to ignore to avoid flickering */
+    pointer-events: none;
+  }
 `);

app/client/components/Forms/Editor.ts

@@ -137,7 +137,13 @@ export function buildEditor(props: Props, ...args: IDomArgs<HTMLElement>) {
       ev.dataTransfer!.dropEffect = "move";
       dragHover.set(true);
 
-      if (dragging.get() || props.box.type === 'Section') { return; }
+      // If we are being dragged, don't animate anything.
+      if (dragging.get()) { return; }
+
+      // We only animate if the box will add dropped element as sibling.
+      if (box.willAccept() !== 'sibling') {
+        return;
+      }
 
       const myHeight = element.offsetHeight;
       const percentHeight = Math.round((ev.offsetY / myHeight) * 100);
@@ -180,14 +186,27 @@ export function buildEditor(props: Props, ...args: IDomArgs<HTMLElement>) {
       dragBelow.set(false);
 
       const dropped = parseBox(ev.dataTransfer!.getData('text/plain'));
+      if (!dropped) { return; }
       // We need to remove it from the parent, so find it first.
       const droppedId = dropped.id;
       if (droppedId === box.id) { return; }
-      const droppedModel = box.root().get(droppedId);
+      const droppedModel = box.root().find(droppedId);
       // It might happen that parent is dropped into child, so we need to check for that.
-      if (droppedModel?.get(box.id)) { return; }
+      if (droppedModel?.find(box.id)) { return; }
+
+      if (!box.willAccept(droppedModel)) {
+        return;
+      }
+
+      // TODO: accept should do the swapping.
+      if (box.willAccept(droppedModel) === 'swap') {
+        await box.save(async () => {
+          box.parent!.swap(box, droppedModel!);
+        });
+        return;
+      }
+
       await box.save(async () => {
-        droppedModel?.removeSelf();
         await box.accept(dropped, wasBelow ? 'below' : 'above')?.afterDrop();
       });
     }),
@@ -199,6 +218,7 @@ export function buildEditor(props: Props, ...args: IDomArgs<HTMLElement>) {
     ),
     testId(box.type),
     testId('element'),
+    dom.attr('data-box-model', String(box.type)),
     dom.maybe(overlay, () => style.cssSelectedOverlay()),
     // Custom icons for removing.
     props.removeIcon === null || props.removeButton ? null :

app/client/components/Forms/Field.ts

@@ -7,8 +7,8 @@ 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 {Box, CHOOSE_TEXT} from 'app/common/Forms';
-import {Constructor} from 'app/common/gutil';
+import {Constructor, not} from 'app/common/gutil';
 import {
   BindableValue,
   Computed,
@@ -41,6 +41,7 @@ export class FieldModel extends BoxModel {
   public field = refRecord(this.view.gristDoc.docModel.viewFields, this.fieldRef);
   public colId = Computed.create(this, (use) => use(use(this.field).colId));
   public column = Computed.create(this, (use) => use(use(this.field).column));
+  public required: Computed<boolean>;
   public question = Computed.create(this, (use) => {
     const field = use(this.field);
     if (field.isDisposed() || use(field.id) === 0) { return ''; }
@@ -67,10 +68,6 @@ export class FieldModel extends BoxModel {
     return this.prop('leaf') as Observable<number>;
   }
 
-  public get required() {
-    return this.prop('formRequired', false) as Observable<boolean|undefined>;
-  }
-
   /**
    * A renderer of question instance.
    */
@@ -84,6 +81,14 @@ export class FieldModel extends BoxModel {
   constructor(box: Box, parent: BoxModel | null, view: FormView) {
     super(box, parent, view);
 
+    this.required = Computed.create(this, (use) => {
+      const field = use(this.field);
+      return Boolean(use(field.widgetOptionsJson.prop('formRequired')));
+    });
+    this.required.onWrite(value => {
+      this.field.peek().widgetOptionsJson.prop('formRequired').setAndSave(value).catch(reportError);
+    });
+
     this.question.onWrite(value => {
       this.field.peek().question.setAndSave(value).catch(reportError);
     });
@@ -125,7 +130,7 @@ export class FieldModel extends BoxModel {
       edit: this.edit,
       overlay,
       onSave: save,
-    }, ...args));
+    }));
 
     return buildEditor({
         box: this,
@@ -136,6 +141,7 @@ export class FieldModel extends BoxModel {
         content,
       },
       dom.on('dblclick', () => this.selected.get() && this.edit.set(true)),
+      ...args
     );
   }
 
@@ -166,11 +172,12 @@ export abstract class Question extends Disposable {
     overlay: Observable<boolean>,
     onSave: (value: string) => void,
   }, ...args: IDomArgs<HTMLElement>) {
-    return css.cssPadding(
+    return css.cssQuestion(
       testId('question'),
       testType(this.model.colType),
       this.renderLabel(props, dom.style('margin-bottom', '5px')),
       this.renderInput(),
+      css.cssQuestion.cls('-required', this.model.required),
       ...args
     );
   }
@@ -224,19 +231,32 @@ export abstract class Question extends Disposable {
 
     return [
       dom.autoDispose(scope),
-      element = css.cssEditableLabel(
+      css.cssRequiredWrapper(
-        controller,
-        {onInput: true},
-        // Attach common Enter,Escape, blur handlers.
-        css.saveControls(edit, saveDraft),
-        // Autoselect whole text when mounted.
-        // Auto grow for textarea.
-        autoGrow(controller),
-        // Enable normal menu.
-        dom.on('contextmenu', stopEvent),
-        dom.style('resize', 'none'),
         testId('label'),
-        css.cssEditableLabel.cls('-edit', props.edit),
+        // When in edit - hide * and change display from grid to display
+        css.cssRequiredWrapper.cls('-required', use => Boolean(use(this.model.required) && !use(this.model.edit))),
+        dom.maybe(props.edit, () => [
+          element = css.cssEditableLabel(
+            controller,
+            {onInput: true},
+            // Attach common Enter,Escape, blur handlers.
+            css.saveControls(edit, saveDraft),
+            // Autoselect whole text when mounted.
+            // Auto grow for textarea.
+            autoGrow(controller),
+            // Enable normal menu.
+            dom.on('contextmenu', stopEvent),
+            dom.style('resize', 'none'),
+            css.cssEditableLabel.cls('-edit'),
+            testId('label-editor'),
+          ),
+        ]),
+        dom.maybe(not(props.edit), () => [
+          css.cssRenderedLabel(
+            dom.text(controller),
+            testId('label-rendered'),
+          ),
+        ]),
         // When selected, we want to be able to edit the label by clicking it
         // so we need to make it relative and z-indexed.
         dom.style('position', u => u(this.model.selected) ? 'relative' : 'static'),
@@ -277,11 +297,9 @@ class ChoiceModel extends Question {
   });
 
   protected choicesWithEmpty = Computed.create(this, use => {
-    const list = Array.from(use(this.choices));
+    const list: Array<string|null> = Array.from(use(this.choices));
     // Add empty choice if not present.
-    if (list.length === 0 || list[0] !== '') {
+    list.unshift(null);
-      list.unshift('');
-    }
     return list;
   });
 
@@ -291,7 +309,7 @@ class ChoiceModel extends Question {
       {tabIndex: "-1"},
       ignoreClick,
       dom.prop('name', use => use(use(field).colId)),
-      dom.forEach(this.choicesWithEmpty, (choice) => dom('option', choice, {value: choice})),
+      dom.forEach(this.choicesWithEmpty, (choice) => dom('option', choice ?? CHOOSE_TEXT, {value: choice ?? ''})),
     );
   }
 }
@@ -319,12 +337,12 @@ class BoolModel extends Question {
     question: Observable<string>,
     onSave: () => void,
   }) {
-    return css.cssPadding(
+    return css.cssQuestion(
       testId('question'),
       testType(this.model.colType),
       cssToggle(
         this.renderInput(),
-        this.renderLabel(props, css.cssEditableLabel.cls('-normal')),
+        this.renderLabel(props, css.cssLabelInline.cls('')),
       ),
     );
   }
@@ -404,7 +422,7 @@ 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, '']);
+    list.unshift(['', CHOOSE_TEXT]);
     return list;
   });
 
@@ -450,8 +468,10 @@ function testType(value: BindableValue<string>) {
 }
 
 const cssToggle = styled('div', `
-  display: flex;
+  display: grid;
   align-items: center;
+  grid-template-columns: auto 1fr;
   gap: 8px;
+  padding: 4px 0px;
   --grist-actual-cell-color: ${colors.lightGreen};
 `);

app/client/components/Forms/FormView.ts

@@ -23,7 +23,7 @@ import {icon} from 'app/client/ui2018/icons';
 import {confirmModal} from 'app/client/ui2018/modals';
 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 {Computed, dom, Holder, IDomArgs, MultiHolder, Observable} from 'grainjs';
 import defaults from 'lodash/defaults';
 import isEqual from 'lodash/isEqual';
 import {v4 as uuidv4} from 'uuid';
@@ -37,7 +37,7 @@ export class FormView extends Disposable {
   public viewPane: HTMLElement;
   public gristDoc: GristDoc;
   public viewSection: ViewSectionRec;
-  public selectedBox: Observable<BoxModel | null>;
+  public selectedBox: Computed<BoxModel | null>;
   public selectedColumns: ko.Computed<ViewFieldRec[]>|null;
 
   protected sortedRows: SortedRowSet;
@@ -60,7 +60,38 @@ export class FormView extends Disposable {
   public create(gristDoc: GristDoc, viewSectionModel: ViewSectionRec) {
     BaseView.call(this as any, gristDoc, viewSectionModel, {'addNewRow': false});
     this.menuHolder = Holder.create(this);
-    this.selectedBox = Observable.create(this, null);
+
+    // We will store selected box here.
+    const selectedBox = Observable.create(this, null as BoxModel|null);
+
+    // But we will guard it with a computed, so that if box is disposed we will clear it.
+    this.selectedBox = Computed.create(this, use => use(selectedBox));
+
+    // Prepare scope for the method calls.
+    const holder = Holder.create(this);
+
+    this.selectedBox.onWrite((box) => {
+      // Create new scope and dispose the previous one (using holder).
+      const scope = MultiHolder.create(holder);
+      if (!box) {
+        selectedBox.set(null);
+        return;
+      }
+      if (box.isDisposed()) {
+        throw new Error('Box is disposed');
+      }
+      selectedBox.set(box);
+
+      // Now subscribe to the new box, if it is disposed, remove it from the selected box.
+      // Note that the dispose listener itself is disposed when the box is switched as we don't
+      // care anymore for this event if the box is switched.
+      scope.autoDispose(box.onDispose(() => {
+        if (selectedBox.get() === box) {
+          selectedBox.set(null);
+        }
+      }));
+    });
+
     this.bundle = (clb) => this.gristDoc.docData.bundleActions('Saving form layout', clb, {nestInActiveBundle: true});
 
 
@@ -153,7 +184,7 @@ export class FormView extends Disposable {
             this.selectedBox.set(this.selectedBox.get()!.insertBefore(boxInClipboard));
           }
           // Remove the original box from the clipboard.
-          const cut = this._root.get(boxInClipboard.id);
+          const cut = this._root.find(boxInClipboard.id);
           cut?.removeSelf();
           await this._root.save();
           await navigator.clipboard.writeText('');
@@ -162,7 +193,7 @@ export class FormView extends Disposable {
       },
       nextField: () => {
         const current = this.selectedBox.get();
-        const all = [...this._root.iterate()];
+        const all = [...this._root.traverse()];
         if (!all.length) { return; }
         if (!current) {
           this.selectedBox.set(all[0]);
@@ -177,7 +208,7 @@ export class FormView extends Disposable {
       },
       prevField: () => {
         const current = this.selectedBox.get();
-        const all = [...this._root.iterate()];
+        const all = [...this._root.traverse()];
         if (!all.length) { return; }
         if (!current) {
           this.selectedBox.set(all[all.length - 1]);
@@ -191,12 +222,12 @@ export class FormView extends Disposable {
         }
       },
       lastField: () => {
-        const all = [...this._root.iterate()];
+        const all = [...this._root.traverse()];
         if (!all.length) { return; }
         this.selectedBox.set(all[all.length - 1]);
       },
       firstField: () => {
-        const all = [...this._root.iterate()];
+        const all = [...this._root.traverse()];
         if (!all.length) { return; }
         this.selectedBox.set(all[0]);
       },

app/client/components/Forms/Menu.ts

@@ -17,11 +17,26 @@ const testId = makeTestId('test-forms-menu-');
 export type NewBox = {add: string} | {show: string} | {structure: BoxType};
 
 interface Props {
+  /**
+   * If this menu was shown as a result of clicking on a box. This box will be selected.
+   */
   box?: BoxModel;
+  /**
+   * Parent view (to access GristDoc/selectedBox and others, TODO: this should be turned into events)
+   */
   view?: FormView;
+  /**
+   * Whether this is context menu, so move `Copy` etc in front, and nest new items in its own menu.
+   */
   context?: boolean;
+  /**
+   * Custom menu items to be added at the bottom (below additional separator).
+   */
   customItems?: Element[],
-  insertBox?: Place
+  /**
+   * Custom logic of finding right spot to insert the new box.
+   */
+  insertBox?: Place,
 }
 
 export function buildMenu(props: Props, ...args: IDomArgs<HTMLElement>): IDomArgs<HTMLElement> {
@@ -53,6 +68,9 @@ export function buildMenu(props: Props, ...args: IDomArgs<HTMLElement>): IDomArg
   const oneTo5 = Computed.create(owner, (use) => use(unmapped).length > 0 && use(unmapped).length <= 5);
   const moreThan5 = Computed.create(owner, (use) => use(unmapped).length > 5);
 
+  // If we are in a column, then we can't insert a new column.
+  const disableInsert = box?.parent?.type === 'Columns' && box.type !== 'Placeholder';
+
   return [
     dom.autoDispose(owner),
     menus.menu((ctl) => {
@@ -145,17 +163,22 @@ export function buildMenu(props: Props, ...args: IDomArgs<HTMLElement>): IDomArg
           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")),
+
+          props.customItems ? menus.menuDivider() : null,
+          ...(props.customItems ?? []),
         ];
       };
 
-      if (!props.context) {
+      if (!props.context && !disableInsert) {
         return insertMenu(custom ?? atEnd)();
       }
 
       return [
-        menus.menuItemSubmenu(insertMenu(above), {action: above({add: 'Text'})}, t("Insert question above")),
+        disableInsert ? null : [
-        menus.menuItemSubmenu(insertMenu(below), {action: below({add: 'Text'})}, t("Insert question below")),
+          menus.menuItemSubmenu(insertMenu(above), {action: above({add: 'Text'})}, t("Insert question above")),
-        menus.menuDivider(),
+          menus.menuItemSubmenu(insertMenu(below), {action: below({add: 'Text'})}, t("Insert question below")),
+          menus.menuDivider(),
+        ],
         menus.menuItemCmd(allCommands.contextMenuCopy, t("Copy")),
         menus.menuItemCmd(allCommands.contextMenuCut, t("Cut")),
         menus.menuItemCmd(allCommands.contextMenuPaste, t("Paste")),

app/client/components/Forms/Model.ts

@@ -53,7 +53,10 @@ export abstract class BoxModel extends Disposable {
    */
   public cut = Observable.create(this, false);
 
-  public selected: Observable<boolean>;
+  /**
+   * Computed if this box is selected or not.
+   */
+  public selected: Computed<boolean>;
 
   /**
    * Any other dynamically added properties (that are not concrete fields in the derived classes)
@@ -134,12 +137,31 @@ export abstract class BoxModel extends Disposable {
    * Cuts self and puts it into clipboard.
    */
   public async cutSelf() {
-    [...this.root().iterate()].forEach(box => box?.cut.set(false));
+    [...this.root().traverse()].forEach(box => box?.cut.set(false));
     // Add this box as a json to clipboard.
     await navigator.clipboard.writeText(JSON.stringify(this.toJSON()));
     this.cut.set(true);
   }
 
+  /**
+   * The way this box will accept dropped content.
+   * - sibling: it will add it as a sibling
+   * - child: it will add it as a child.
+   * - swap: swaps with the box
+   */
+  public willAccept(box?: Box|BoxModel|null): 'sibling' | 'child' | 'swap' | null {
+    // If myself and the dropped element share the same parent, and the parent is a column
+    // element, just swap us.
+    if (this.parent && box instanceof BoxModel && this.parent === box?.parent && box.parent?.type === 'Columns') {
+      return 'swap';
+    }
+
+    // If we are in column, we won't accept anything.
+    if (this.parent?.type === 'Columns') { return null; }
+
+    return 'sibling';
+  }
+
   /**
    * Accepts box from clipboard and inserts it before this box or if this is a container box, then
    * as a first child. Default implementation is to insert before self.
@@ -152,7 +174,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 = droppedId ? this.root().get(droppedId) : null;
+    const droppedRef = droppedId ? this.root().find(droppedId) : null;
     if (droppedRef) {
       droppedRef.removeSelf();
     }
@@ -184,6 +206,16 @@ export abstract class BoxModel extends Disposable {
     return newOne;
   }
 
+  public swap(box1: BoxModel, box2: BoxModel) {
+    const index1 = this.children.get().indexOf(box1);
+    const index2 = this.children.get().indexOf(box2);
+    if (index1 < 0 || index2 < 0) { throw new Error('Cannot swap boxes that are not in parent'); }
+    const box1JSON = box1.toJSON();
+    const box2JSON = box2.toJSON();
+    this.replace(box1, box2JSON);
+    this.replace(box2, box1JSON);
+  }
+
   public append(box: Box) {
     const newOne = BoxModel.new(box, this);
     this.children.push(newOne);
@@ -255,10 +287,11 @@ export abstract class BoxModel extends Disposable {
   /**
    * Finds a box with a given id in the tree.
    */
-  public get(droppedId: string): BoxModel | null {
+  public find(droppedId: string|undefined|null): BoxModel | null {
+    if (!droppedId) { return null; }
     for (const child of this.kids()) {
       if (child.id === droppedId) { return child; }
-      const found = child.get(droppedId);
+      const found = child.find(droppedId);
       if (found) { return found; }
     }
     return null;
@@ -341,10 +374,10 @@ export abstract class BoxModel extends Disposable {
     };
   }
 
-  public * iterate(): IterableIterator<BoxModel> {
+  public * traverse(): IterableIterator<BoxModel> {
     for (const child of this.kids()) {
       yield child;
-      yield* child.iterate();
+      yield* child.traverse();
     }
   }
 
@@ -387,7 +420,7 @@ export function unwrap<T>(val: T | Computed<T>): T {
   return val instanceof Computed ? val.get() : val;
 }
 
-export function parseBox(text: string) {
+export function parseBox(text: string): Box|null {
   try {
     const json = JSON.parse(text);
     return json && typeof json === 'object' && json.type ? json : null;

app/client/components/Forms/Section.ts

@@ -42,6 +42,9 @@ export class SectionModel extends BoxModel {
     );
   }
 
+  public override willAccept(): 'sibling' | 'child' | null {
+    return 'child';
+  }
 
   /**
    * Accepts box from clipboard and inserts it before this box or if this is a container box, then
@@ -54,7 +57,7 @@ export class SectionModel extends BoxModel {
       return null;
     }
     // We need to remove it from the parent, so find it first.
-    const droppedRef = dropped.id ? this.root().get(dropped.id) : null;
+    const droppedRef = dropped.id ? this.root().find(dropped.id) : null;
     if (droppedRef) {
       droppedRef.removeSelf();
     }

app/client/components/Forms/styles.ts

@@ -117,6 +117,46 @@ export function textbox(obs: Observable<string|undefined>, ...args: DomElementAr
   );
 }
 
+export const cssQuestion = styled('div', `
+
+`);
+
+export const cssRequiredWrapper = styled('div', `
+  margin-bottom: 8px;
+  min-height: 16px;
+  &-required {
+    display: grid;
+    grid-template-columns: auto 1fr;
+    gap: 4px;
+  }
+  &-required:after {
+    content: "*";
+    color: ${colors.lightGreen};
+    font-size: 11px;
+    font-weight: 700;
+  }
+`);
+
+export const cssRenderedLabel = styled('div', `
+  font-weight: normal;
+  padding: 0px;
+  border: 0px;
+  width: 100%;
+  margin: 0px;
+  background: transparent;
+  cursor: pointer;
+  min-height: 16px;
+
+  color: ${colors.darkText};
+  font-size: 11px;
+  line-height: 16px;
+  font-weight: 700;
+  white-space: pre-wrap;
+  &-placeholder {
+    font-style: italic
+  }
+`);
+
 export const cssEditableLabel = styled(textarea, `
   font-weight: normal;
   outline: none;
@@ -143,11 +183,20 @@ export const cssEditableLabel = styled(textarea, `
     outline-offset: 1px;
     border-radius: 2px;
   }
-  &-normal {
+`);
+
+export const cssLabelInline = styled('div', `
+  margin-bottom: 0px;
+  & .${cssRenderedLabel.className} {
     color: ${theme.mediumText};
     font-size: 15px;
     font-weight: normal;
   }
+  & .${cssEditableLabel.className} {
+    color: ${colors.darkText};
+    font-size: 15px;
+    font-weight: normal;
+  }
 `);
 
 export const cssDesc = styled('div', `
@@ -192,6 +241,10 @@ export const cssSelect = styled('select', `
   &-invalid {
     color: ${theme.inputInvalid};
   }
+  &:has(option[value='']:checked) {
+    font-style: italic;
+    color: ${colors.slate};
+  }
 `);
 
 export const cssFieldEditorContent = styled('div', `
@@ -239,15 +292,13 @@ export const cssPlusIcon = styled(icon, `
  --icon-color: ${theme.controlPrimaryFg};
 `);
 
-export const cssPadding = styled('div', `
-`);
 
 export const cssColumns = styled('div', `
   --css-columns-count: 2;
   display: grid;
   grid-template-columns: repeat(var(--css-columns-count), 1fr) 32px;
   gap: 8px;
-  padding: 12px 4px;
+  padding: 8px 4px;
 
   .${cssFormView.className}-preview & {
     background: transparent;
@@ -293,7 +344,6 @@ export const cssColumn = styled('div', `
   }
 
   &-add-button {
-    align-self: flex-end;
   }
 
   .${cssFormView.className}-preview &-add-button {

app/common/Forms.ts

@@ -23,6 +23,8 @@ export type BoxType =   'Paragraph' | 'Section' | 'Columns' | 'Submit'
  */
 export const INITIAL_FIELDS_COUNT = 9;
 
+export const CHOOSE_TEXT = '— Choose —';
+
 /**
  * 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.
@@ -217,8 +219,9 @@ abstract class BaseQuestion implements Question {
     // This might be HTML.
     const label = field.question;
     const name = this.name(field);
+    const required = field.options.formRequired ? 'grist-label-required' : '';
     return `
-      <label class='grist-label' for='${name}'>${label}</label>
+      <label class='grist-label ${required}' for='${name}'>${label}</label>
     `;
   }
 
@@ -255,12 +258,12 @@ class DateTime extends BaseQuestion {
 class Choice extends BaseQuestion  {
   public input(field: FieldModel, context: RenderContext): string {
     const required = field.options.formRequired ? 'required' : '';
-    const choices: string[] = field.options.choices || [];
+    const choices: Array<string|null> = field.options.choices || [];
     // Insert empty option.
-    choices.unshift('');
+    choices.unshift(null);
     return `
       <select name='${this.name(field)}' ${required} >
-        ${choices.map((choice) => `<option value='${choice}'>${choice}</option>`).join('')}
+        ${choices.map((choice) => `<option value='${choice ?? ''}'>${choice ?? CHOOSE_TEXT}</option>`).join('')}
       </select>
     `;
   }
@@ -278,11 +281,12 @@ class Bool extends BaseQuestion {
   }
 
   public input(field: FieldModel, context: RenderContext): string {
+    const requiredLabel = field.options.formRequired ? 'grist-label-required' : '';
     const required = field.options.formRequired ? 'required' : '';
     const label = field.question ? field.question : field.colId;
     return `
-      <label class='grist-switch'>
+      <label class='grist-switch ${requiredLabel}'>
-        <input type='checkbox' name='${this.name(field)}' value="1" ${required} />
+        <input type='checkbox' name='${this.name(field)}' value="1" ${required}  />
         <div class="grist-widget_switch grist-switch_transition">
           <div class="grist-switch_slider"></div>
           <div class="grist-switch_circle"></div>
@@ -346,7 +350,7 @@ class Ref extends BaseQuestion {
     // Support for 1000 choices, TODO: make it dynamic.
     choices.splice(1000);
     // Insert empty option.
-    choices.unshift(['', '']);
+    choices.unshift(['', CHOOSE_TEXT]);
     // <option type='number' is not standard, we parse it ourselves.
     const required = field.options.formRequired ? 'required' : '';
     return `

app/server/lib/DocApi.ts

@@ -1416,6 +1416,7 @@ export class DocWorkerApi {
           throw new ApiError('Form not found', 404);
         }
         const Tables = activeDoc.docData!.getMetaTable('_grist_Tables');
+        const tableRecord = Tables.getRecord(section.tableRef);
         const Views_section_field = activeDoc.docData!.getMetaTable('_grist_Views_section_field');
         const fields = Views_section_field.filterRecords({parentId: sectionId});
         const Tables_column = activeDoc.docData!.getMetaTable('_grist_Tables_column');
@@ -1521,6 +1522,11 @@ export class DocWorkerApi {
         const doc = await this._dbManager.getDoc(req);
         const tableId = await getRealTableId(String(section.tableRef), {activeDoc, req});
 
+        const rawSectionRef = tableRecord?.rawViewSectionRef;
+        const rawSection = !rawSectionRef ? null :
+                           activeDoc.docData!.getMetaTable('_grist_Views_section').getRecord(rawSectionRef);
+        const tableName = rawSection?.title;
+
         const template = handlebars.compile(form);
         const renderedHtml = template({
           // Trusted content generated by us.
@@ -1532,6 +1538,7 @@ export class DocWorkerApi {
           CONTENT: html,
           SUCCESS_TEXT: box.successText || 'Thank you! Your response has been recorded.',
           SUCCESS_URL: redirectUrl,
+          TITLE: `${section.title || tableName || tableId || 'Form'} - Grist`
         });
         res.status(200).send(renderedHtml);
       })

static/forms/form.css

@@ -296,6 +296,20 @@ body {
   display: block;
 }
 
+.grist-label-required::after {
+  content: "*";
+  color: var(--primary, #16b378);
+  margin-left: 4px;
+}
+
+/**
+ * When an empty value is selected, show the placeholder in italic gray.
+ * The css is: every select that has an empty option selected, and is not active (so not open).
+ */
+.grist-form select:has(option[value='']:checked):not(:active) {
+  font-style: italic;
+  color: var(--light-gray, #bfbfbf);
+}
 
 /* Markdown reset */
 

static/forms/form.html

@@ -5,8 +5,8 @@
   {{#if BASE}}
   <base href="{{ BASE }}">
   {{/if}}
-  <style>
+  <title>{{ TITLE }}</title>
-  </style>
+  <link rel="icon" type="image/x-icon" href="icons/favicon.png" />
   <script src="forms/grist-form-submit.js"></script>
   <script src="forms/purify.min.js"></script>
   <script>

test/nbrowser/FormView.ts

@@ -1,4 +1,6 @@
+import {CHOOSE_TEXT} from 'app/common/Forms';
 import {UserAPI} from 'app/common/UserAPI';
+import {escapeRegExp} from 'lodash';
 import {addToRepl, assert, driver, Key, WebElement, WebElementPromise} from 'mocha-webdriver';
 import * as gu from 'test/nbrowser/gristUtils';
 import {setupTestSuite} from 'test/nbrowser/testUtils';
@@ -12,10 +14,6 @@ describe('FormView', function() {
 
   const cleanup = setupTestSuite();
 
-  gu.withEnvironmentSnapshot({
-    'GRIST_EXPERIMENTAL_PLUGINS': '1'
-  });
-
   addToRepl('question', question);
   addToRepl('labels', readLabels);
   addToRepl('questionType', questionType);
@@ -245,7 +243,9 @@ describe('FormView', function() {
     await gu.onNewTab(async () => {
       await driver.get(formUrl);
       // Make sure options are there.
-      assert.deepEqual(await driver.findAll('select[name="D"] option', e => e.getText()), ['', 'Foo', 'Bar', 'Baz']);
+      assert.deepEqual(
+        await driver.findAll('select[name="D"] option', e => e.getText()), [CHOOSE_TEXT, 'Foo', 'Bar', 'Baz']
+      );
       await driver.findWait('select[name="D"]', 1000).click();
       await driver.find("option[value='Bar']").click();
       await driver.find('input[type="submit"]').click();
@@ -335,7 +335,7 @@ describe('FormView', function() {
       await driver.get(formUrl);
       assert.deepEqual(
         await driver.findAll('select[name="D"] option', e => e.getText()),
-        ['', ...['Bar', 'Baz', 'Foo']]
+        [CHOOSE_TEXT, ...['Bar', 'Baz', 'Foo']]
       );
       assert.deepEqual(
         await driver.findAll('select[name="D"] option', e => e.value()),
@@ -877,7 +877,7 @@ describe('FormView', function() {
     assert.deepEqual(await readLabels(), ['A', 'B', 'C', 'D']);
 
     // And the question is inside a section.
-    assert.equal(await element('Section', 1).element('label', 4).value(), 'D');
+    assert.equal(await element('Section', 1).element('label', 4).getText(), 'D');
 
     // Make sure we can move that question around.
     await driver.withActions(a =>
@@ -895,7 +895,7 @@ describe('FormView', function() {
 
     await gu.undo();
     assert.deepEqual(await readLabels(), ['A', 'B', 'C', 'D']);
-    assert.equal(await element('Section', 1).element('label', 4).value(), 'D');
+    assert.equal(await element('Section', 1).element('label', 4).getText(), 'D');
 
     await revert();
     assert.deepEqual(await readLabels(), ['A', 'B', 'C']);
@@ -903,6 +903,10 @@ describe('FormView', function() {
 
   it('basic columns work', async function() {
     const revert = await gu.begin();
+
+    // Open the creator panel to make sure it works.
+    await gu.openColumnPanel();
+
     await plusButton().click();
     await clickMenu('Columns');
     await gu.waitForServer();
@@ -928,7 +932,7 @@ describe('FormView', function() {
     assert.deepEqual(await readLabels(), ['A', 'B', 'C', 'D']);
 
     // The question D is in the columns.
-    assert.equal(await element('Columns').element('label').value(), 'D');
+    assert.equal(await element('Columns').element('label').getText(), 'D');
 
     // We can move it around.
     await driver.withActions(a =>
@@ -950,13 +954,12 @@ describe('FormView', function() {
     await gu.waitForServer();
     assert.deepEqual(await readLabels(), ['A', 'B', 'C', 'D']);
 
-    let allColumns = await driver.findAll('.test-forms-column');
 
-    assert.lengthOf(allColumns, 3);
+    assert.equal(await elementCount('column'), 3);
-    assert.isTrue(await allColumns[0].matches('.test-forms-Placeholder'));
+    assert.equal(await element('column', 1).type(), 'Placeholder');
-    assert.isTrue(await allColumns[1].matches('.test-forms-question'));
+    assert.equal(await element('column', 2).type(), 'Field');
-    assert.equal(await allColumns[1].find('.test-forms-label').value(), 'D');
+    assert.equal(await element('column', 2).element('label').getText(), 'D');
-    assert.isTrue(await allColumns[2].matches('.test-forms-Placeholder'));
+    assert.equal(await element('column', 3).type(), 'Placeholder');
 
     // Check that we can remove the question.
     await question('D').rightClick();
@@ -971,18 +974,185 @@ describe('FormView', function() {
     await gu.undo();
     assert.deepEqual(await readLabels(), ['A', 'B', 'C', 'D']);
 
-    allColumns = await driver.findAll('.test-forms-column');
+    assert.equal(await elementCount('column'), 3);
-    assert.lengthOf(allColumns, 3);
+    assert.equal(await element('column', 1).type(), 'Placeholder');
-    assert.isTrue(await allColumns[0].matches('.test-forms-Placeholder'));
+    assert.equal(await element('column', 2).type(), 'Field');
-    assert.isTrue(await allColumns[1].matches('.test-forms-question'));
+    assert.equal(await element('column', 2).element('label').getText(), 'D');
-    assert.equal(await allColumns[1].find('.test-forms-label').value(), 'D');
+    assert.equal(await element('column', 3).type(), 'Placeholder');
-    assert.isTrue(await allColumns[2].matches('.test-forms-Placeholder'));
+
+    // There was a bug with paragraph and columns.
+    // Add a paragraph to first placeholder.
+    await element('Columns').element(`Placeholder`, 1).click();
+    await clickMenu('Paragraph');
+    await gu.waitForServer();
+
+    // Now click this paragraph.
+    await element('Columns').element(`Paragraph`, 1).click();
+    // And make sure there aren't any errors.
+    await gu.checkForErrors();
 
     await revert();
     assert.lengthOf(await driver.findAll('.test-forms-column'), 0);
     assert.deepEqual(await readLabels(), ['A', 'B', 'C']);
   });
 
+  it('drags and drops on columns properly', async function() {
+    const revert = await gu.begin();
+    // Open the creator panel to make sure it works.
+    await gu.openColumnPanel();
+
+    await plusButton().click();
+    await clickMenu('Columns');
+    await gu.waitForServer();
+
+    // Make sure that dragging columns on its placeholder doesn't do anything.
+    await driver.withActions(a =>
+      a.move({origin: element('Columns').element(`Placeholder`, 1).find(`.test-forms-drag`)})
+        .press()
+        .move({origin: element('Columns').element(`Placeholder`, 2).find(`.test-forms-drag`)})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+
+    // Make sure we see form correctly.
+    const testNothingIsMoved = async () => {
+      assert.deepEqual(await readLabels(), ['A', 'B', 'C']);
+      assert.deepEqual(await elements(), [
+        'Paragraph',
+        'Paragraph',
+        'Section',
+        'Paragraph',
+        'Paragraph',
+        'Field',
+        'Field',
+        'Field',
+        'Columns',
+        'Placeholder',
+        'Placeholder'
+      ]);
+    };
+
+    await testNothingIsMoved();
+
+    // Now do the same but move atop the + placeholder.
+    await driver.withActions(a =>
+      a.move({origin: element('Columns').element(`Placeholder`, 1).find(`.test-forms-drag`)})
+        .press()
+        .move({origin: driver.find('.test-forms-Columns .test-forms-add')})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+    await testNothingIsMoved();
+
+    // Now move C column into first column.
+    await driver.withActions(a =>
+      a.move({origin: questionDrag('C')})
+        .press()
+        .move({origin: element('Columns').element(`Placeholder`, 1).find(`.test-forms-drag`)})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+
+    // Check that it worked.
+    assert.equal(await element('column', 1).type(), 'Field');
+    assert.equal(await element('column', 1).element('label').getText(), 'C');
+    assert.equal(await element('column', 2).type(), 'Placeholder');
+
+    // Try to move B over C.
+    await driver.withActions(a =>
+      a.move({origin: questionDrag('B')})
+        .press()
+        .move({origin: questionDrag('C')})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+
+    // Make sure it didn't work.
+    assert.equal(await element('column', 1).type(), 'Field');
+    assert.equal(await element('column', 1).element('label').getText(), 'C');
+
+    // And B is still there.
+    assert.equal(await element('Field', 2).element('label').getText(), 'B');
+
+    // Now move B on the empty placholder.
+    await driver.withActions(a =>
+      a.move({origin: questionDrag('B')})
+        .press()
+        .move({origin: element('column', 2).drag()})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+
+    // Make sure it worked.
+    assert.equal(await element('column', 1).type(), 'Field');
+    assert.equal(await element('column', 1).element('label').getText(), 'C');
+    assert.equal(await element('column', 2).type(), 'Field');
+    assert.equal(await element('column', 2).element('label').getText(), 'B');
+
+    // Now swap them moving C over B.
+    await driver.withActions(a =>
+      a.move({origin: questionDrag('C')})
+        .press()
+        .move({origin: questionDrag('B')})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+    assert.equal(await element('column', 1).element('label').getText(), 'B');
+    assert.equal(await element('column', 2).element('label').getText(), 'C');
+
+    // And swap them back.
+    await driver.withActions(a =>
+      a.move({origin: questionDrag('B')})
+        .press()
+        .move({origin: questionDrag('C')})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+    assert.equal(await element('column', 1).element('label').getText(), 'C');
+    assert.equal(await element('column', 2).element('label').getText(), 'B');
+
+    // Make sure we still have two columns only.
+    assert.lengthOf(await driver.findAll('.test-forms-column'), 2);
+
+    // Make sure draggin column on the add button doesn't add column.
+    await driver.withActions(a =>
+      a.move({origin: questionDrag('B')})
+        .press()
+        .move({origin: driver.find('.test-forms-Columns .test-forms-add')})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+
+    // Make sure we still have two columns only.
+    assert.lengthOf(await driver.findAll('.test-forms-column'), 2);
+    assert.equal(await element('column', 1).element('label').getText(), 'C');
+    assert.equal(await element('column', 2).element('label').getText(), 'B');
+
+    // Now move A over the + button to add a new column.
+    await driver.withActions(a =>
+      a.move({origin: questionDrag('A')})
+        .press()
+        .move({origin: driver.find('.test-forms-Columns .test-forms-add')})
+        .release()
+    );
+    await gu.waitForServer();
+    await gu.checkForErrors();
+    assert.lengthOf(await driver.findAll('.test-forms-column'), 3);
+    assert.equal(await element('column', 1).element('label').getText(), 'C');
+    assert.equal(await element('column', 2).element('label').getText(), 'B');
+    assert.equal(await element('column', 3).element('label').getText(), 'A');
+
+    await revert();
+  });
+
   it('changes type of a question', async function() {
     // Add text question as D column.
     await plusButton().click();
@@ -1043,11 +1213,12 @@ async function elementCount(type: string, parent?: WebElement) {
 }
 
 async function readLabels() {
-  return await driver.findAll('.test-forms-question .test-forms-label', el => el.value());
+  return await driver.findAll('.test-forms-question .test-forms-label', el => el.getText());
 }
 
 function question(label: string) {
-  return extra(gu.findValue(`.test-forms-label`, label).findClosest('.test-forms-editor'));
+  return extra(driver.findContent(`.test-forms-label`, new RegExp('^' + escapeRegExp(label) + '\\*?$'))
+                     .findClosest('.test-forms-editor'));
 }
 
 function questionDrag(label: string) {
@@ -1083,7 +1254,7 @@ function selected() {
 }
 
 function selectedLabel() {
-  return selected().find('.test-forms-label').value();
+  return selected().find('.test-forms-label-rendered').getText();
 }
 
 function hiddenColumns() {
@@ -1101,21 +1272,25 @@ type ExtraElement = WebElementPromise & {
    * A draggable element inside. This is 2x2px div to help with drag and drop.
    */
   drag: () => WebElementPromise,
+  type: () => Promise<string>,
 };
 
 function extra(el: WebElementPromise): ExtraElement {
   const webElement: any = el;
 
   webElement.rightClick = async function() {
-    await driver.withActions(a => a.contextClick(webElement));
+    await driver.withActions(a => a.contextClick(el));
   };
 
   webElement.element = function(type: string, index?: number) {
-    return element(type, index ?? 1, webElement);
+    return element(type, index ?? 1, el);
   };
 
   webElement.drag = function() {
-    return webElement.find('.test-forms-drag');
+    return el.find('.test-forms-drag');
+  };
+  webElement.type = async function() {
+    return await el.getAttribute('data-box-model');
   };
 
   return webElement;
@@ -1126,3 +1301,7 @@ async function arrow(key: string, times: number = 1) {
     await gu.sendKeys(key);
   }
 }
+
+async function elements() {
+  return await driver.findAll('.test-forms-element', el => el.getAttribute('data-box-model'));
+}