(core) Updating RawData views

Summary:
- Better focus on the widget title
- Adding columns only to the current view section
- New popup with options when user wants to delete a page
- New dialog to enter table name
- New table as a widget doesn't create a separate page
- Removing a table doesn't remove the primary view

Test Plan: Updated and new tests

Reviewers: georgegevoian

Reviewed By: georgegevoian

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

app/client/components/GridView.js

@@ -653,15 +653,22 @@ GridView.prototype.addNewColumn = function() {
  .then(() => this.scrollPaneRight());
 };
 
-GridView.prototype.insertColumn = function(index) {
-  var pos = tableUtil.fieldInsertPositions(this.viewSection.viewFields(), index)[0];
+GridView.prototype.insertColumn = async function(index) {
+  const pos = tableUtil.fieldInsertPositions(this.viewSection.viewFields(), index)[0];
   var action = ['AddColumn', null, {"_position": pos}];
-  return this.tableModel.sendTableAction(action)
-  .bind(this).then(function() {
-    this.selectColumn(index);
-    this.currentEditingColumnIndex(index);
-    // this.columnConfigTab.show();
+  await this.gristDoc.docData.bundleActions('Insert column', async () => {
+    const colInfo = await this.tableModel.sendTableAction(action);
+    if (!this.viewSection.isRaw.peek()){
+      const fieldInfo = {
+        colRef: colInfo.colRef,
+        parentPos: pos,
+        parentId: this.viewSection.id.peek()
+      };
+      await this.gristDoc.docModel.viewFields.sendTableAction(['AddRecord', null, fieldInfo]);
+    }
   });
+  this.selectColumn(index);
+  this.currentEditingColumnIndex(index);
 };
 
 GridView.prototype.scrollPaneRight = function() {

app/client/components/GristDoc.ts

@@ -45,6 +45,7 @@ import {IPageWidget, toPageWidget} from 'app/client/ui/PageWidgetPicker';
 import {IPageWidgetLink, linkFromId, selectBy} from 'app/client/ui/selectBy';
 import {startWelcomeTour} from 'app/client/ui/welcomeTour';
 import {isNarrowScreen, mediaSmall, testId} from 'app/client/ui2018/cssVars';
+import {invokePrompt} from 'app/client/ui2018/modals';
 import {IconName} from 'app/client/ui2018/IconList';
 import {FieldEditor} from "app/client/widgets/FieldEditor";
 import {MinimalActionGroup} from 'app/common/ActionGroup';
@@ -500,7 +501,9 @@ export class GristDoc extends DisposableWithEvents {
    * Sends an action to create a new empty table and switches to that table's primary view.
    */
   public async addEmptyTable(): Promise<void> {
-    const tableInfo = await this.docData.sendAction(['AddEmptyTable']);
+    const name = await this._promptForName();
+    if (name === undefined) { return; }
+    const tableInfo = await this.docData.sendAction(['AddEmptyTable', name || null]);
     await this.openDocPage(this.docModel.tables.getRowModel(tableInfo.id).primaryViewId());
   }
 
@@ -510,10 +513,16 @@ export class GristDoc extends DisposableWithEvents {
   public async addWidgetToPage(val: IPageWidget) {
     const docData = this.docModel.docData;
     const viewName = this.viewModel.name.peek();
-
+    let tableId: string|null|undefined;
+    if (val.table === 'New Table') {
+      tableId = await this._promptForName();
+      if (tableId === undefined) {
+        return;
+      }
+    }
     const res = await docData.bundleActions(
       `Added new linked section to view ${viewName}`,
-      () => this.addWidgetToPageImpl(val)
+      () => this.addWidgetToPageImpl(val, tableId ?? null)
     );
 
     // The newly-added section should be given focus.
@@ -523,13 +532,12 @@ export class GristDoc extends DisposableWithEvents {
   /**
    * The actual implementation of addWidgetToPage
    */
-  public async addWidgetToPageImpl(val: IPageWidget) {
+  public async addWidgetToPageImpl(val: IPageWidget, tableId: string|null = null) {
     const viewRef = this.activeViewId.get();
-    const tableRef = val.table === 'New Table' ? 0 : val.table;
     const link = linkFromId(val.link);
-
+    const tableRef = val.table === 'New Table' ? 0 : val.table;
     const result = await this.docData.sendAction(
-      ['CreateViewSection', tableRef, viewRef, val.type, val.summarize ? val.columns : null]
+      ['CreateViewSection', tableRef, viewRef, val.type, val.summarize ? val.columns : null, tableId]
     );
     if (val.type === 'chart') {
       await this._ensureOneNumericSeries(result.sectionRef);
@@ -549,13 +557,15 @@ export class GristDoc extends DisposableWithEvents {
    */
   public async addNewPage(val: IPageWidget) {
     if (val.table === 'New Table') {
-      const result = await this.docData.sendAction(['AddEmptyTable']);
+      const name = await this._promptForName();
+      if (name === undefined) { return; }
+      const result = await this.docData.sendAction(['AddEmptyTable', name]);
       await this.openDocPage(result.views[0].id);
     } else {
       let result: any;
       await this.docData.bundleActions(`Add new page`, async () => {
         result = await this.docData.sendAction(
-          ['CreateViewSection', val.table, 0, val.type, val.summarize ? val.columns : null]
+          ['CreateViewSection', val.table, 0, val.type, val.summarize ? val.columns : null, null]
         );
         if (val.type === 'chart') {
           await this._ensureOneNumericSeries(result.sectionRef);
@@ -917,6 +927,10 @@ export class GristDoc extends DisposableWithEvents {
     }
   }
 
+  private async _promptForName() {
+    return await invokePrompt("Table name", "Create", '', "Default table name");
+  }
+
   private async _replaceViewSection(section: ViewSectionRec, newVal: IPageWidget) {
 
     const docModel = this.docModel;

app/client/components/duplicatePage.ts

@@ -186,7 +186,7 @@ async function createNewViewSections(docData: GristDoc['docData'], viewSections:
 
 // Helper to create an action that add widget to the view with viewId.
 function newViewSectionAction(widget: IPageWidget, viewId: number) {
-  return ['CreateViewSection', widget.table, viewId, widget.type, widget.summarize ? widget.columns : null];
+  return ['CreateViewSection', widget.table, viewId, widget.type, widget.summarize ? widget.columns : null, null];
 }
 
 /**

app/client/ui/PageWidgetPicker.ts

@@ -167,11 +167,16 @@ export function buildPageWidgetPicker(
       link: value.link.get(),
       section: value.section.get(),
     });
-    // If savePromise throws an error, before or after timeout, we let the error propagate as it
-    // should be handle by the caller.
-    if (await isLongerThan(savePromise, DELAY_BEFORE_SPINNER_MS)) {
-      const label = getWidgetTypes(type).label;
-      await spinnerModal(`Building ${label} widget`, savePromise);
+    if (value.table.get() === 'New Table') {
+      // Adding empty table will show a prompt, so we don't want to wait for it.
+      await savePromise;
+    } else {
+      // If savePromise throws an error, before or after timeout, we let the error propagate as it
+      // should be handle by the caller.
+      if (await isLongerThan(savePromise, DELAY_BEFORE_SPINNER_MS)) {
+        const label = getWidgetTypes(type).label;
+        await spinnerModal(`Building ${label} widget`, savePromise);
+      }
     }
   }
 

app/client/ui/Pages.ts

@@ -1,15 +1,19 @@
-import { createGroup } from "app/client/components/commands";
-import { duplicatePage } from "app/client/components/duplicatePage";
-import { GristDoc } from "app/client/components/GristDoc";
-import { PageRec } from "app/client/models/DocModel";
-import { urlState } from "app/client/models/gristUrlState";
-import * as MetaTableModel from "app/client/models/MetaTableModel";
-import { find as findInTree, fromTableData, TreeItemRecord, TreeRecord,
-         TreeTableData} from "app/client/models/TreeModel";
-import { TreeViewComponent } from "app/client/ui/TreeViewComponent";
-import { buildPageDom, PageActions } from "app/client/ui2018/pages";
-import { mod } from 'app/common/gutil';
-import { Computed, Disposable, dom, fromKo, observable, Observable } from "grainjs";
+import {createGroup} from 'app/client/components/commands';
+import {duplicatePage} from 'app/client/components/duplicatePage';
+import {GristDoc} from 'app/client/components/GristDoc';
+import {PageRec} from 'app/client/models/DocModel';
+import {urlState} from 'app/client/models/gristUrlState';
+import * as MetaTableModel from 'app/client/models/MetaTableModel';
+import {find as findInTree, fromTableData, TreeItemRecord, TreeRecord,
+        TreeTableData} from 'app/client/models/TreeModel';
+import {TreeViewComponent} from 'app/client/ui/TreeViewComponent';
+import {labeledCircleCheckbox} from 'app/client/ui2018/checkbox';
+import {colors} from 'app/client/ui2018/cssVars';
+import {cssLink} from 'app/client/ui2018/links';
+import {ISaveModalOptions, saveModal} from 'app/client/ui2018/modals';
+import {buildPageDom, PageActions} from 'app/client/ui2018/pages';
+import {mod} from 'app/common/gutil';
+import {Computed, Disposable, dom, DomContents, fromKo, makeTestId, observable, Observable, styled} from 'grainjs';
 
 // build dom for the tree view of pages
 export function buildPagesDom(owner: Disposable, activeDoc: GristDoc, isOpen: Observable<boolean>) {
@@ -49,16 +53,18 @@ export function buildPagesDom(owner: Disposable, activeDoc: GristDoc, isOpen: Ob
   return dom('div', dom.create(TreeViewComponent, model, {isOpen, selected, isReadonly: activeDoc.isReadonly}));
 }
 
-function buildDomFromTable(pagesTable: MetaTableModel<PageRec>, activeDoc: GristDoc, id: number) {
+const testId = makeTestId('test-removepage-');
+
+function buildDomFromTable(pagesTable: MetaTableModel<PageRec>, activeDoc: GristDoc, pageId: number) {
   const {isReadonly} = activeDoc;
-  const pageName = pagesTable.rowModels[id].view.peek().name;
-  const viewId = pagesTable.rowModels[id].view.peek().id.peek();
-  const docData = pagesTable.tableData.docData;
+  const pageName = pagesTable.rowModels[pageId].view.peek().name;
+  const viewId = pagesTable.rowModels[pageId].view.peek().id.peek();
+
   const actions: PageActions = {
     onRename: (newName: string) => newName.length && pageName.saveOnly(newName),
-    onRemove: () => docData.sendAction(['RemoveRecord', '_grist_Views', viewId]),
+    onRemove: () => removeView(activeDoc, viewId, pageName.peek()),
     // TODO: duplicate should prompt user for confirmation
-    onDuplicate: () => duplicatePage(activeDoc, id),
+    onDuplicate: () => duplicatePage(activeDoc, pageId),
     // Can't remove last visible page
     isRemoveDisabled: () => activeDoc.docModel.visibleDocPages.peek().length <= 1,
     isReadonly
@@ -67,6 +73,46 @@ function buildDomFromTable(pagesTable: MetaTableModel<PageRec>, activeDoc: Grist
   return buildPageDom(fromKo(pageName), actions, urlState().setLinkUrl({docPage: viewId}));
 }
 
+function removeView(activeDoc: GristDoc, viewId: number, pageName: string) {
+  const docData = activeDoc.docData;
+  // Create a set with tables on other pages (but not on this one).
+  const tablesOnOtherViews = new Set(activeDoc.docModel.viewSections.rowModels
+    .filter(vs => !vs.isRaw.peek() && vs.parentId.peek() !== viewId)
+    .map(vs => vs.tableRef.peek()));
+
+  // Check if this page is a last page for some tables.
+  const notVisibleTables = [...new Set(activeDoc.docModel.viewSections.rowModels
+    .filter(vs => vs.parentId.peek() === viewId) // Get all sections on this view
+    .filter(vs => !vs.table.peek().summarySourceTable.peek()) // Sections that have normal tables
+    .filter(vs => !tablesOnOtherViews.has(vs.tableRef.peek())) // That aren't on other views
+    .filter(vs => vs.table.peek().tableId.peek()) // Which we can access (has tableId)
+    .map(vs => vs.table.peek()))]; // Return tableRec object, and remove duplicates.
+
+  const removePage = () => [['RemoveRecord', '_grist_Views', viewId]];
+  const removeAll = () => [
+    ...removePage(),
+    ...notVisibleTables.map(t => ['RemoveTable', t.tableId.peek()])
+  ];
+
+  if (notVisibleTables.length) {
+    const tableNames = notVisibleTables.map(t => t.tableNameDef.peek());
+    buildPrompt(tableNames, async (option) => {
+      // Errors are handled in the dialog.
+      if (option === 'data') {
+        await docData.sendActions(removeAll(), `Remove page ${pageName} with tables ${tableNames}`);
+      } else if (option === 'page') {
+        await docData.sendActions(removePage(), `Remove only page ${pageName}`);
+      } else {
+        // This should not happen, as save should be disabled when no option is selected.
+      }
+    });
+  } else {
+    return docData.sendActions(removePage(), `Remove only page ${pageName}`);
+  }
+}
+
+type RemoveOption = '' | 'data' | 'page';
+
 // Select another page in cyclic ordering of pages. Order is downard if given a positive `delta`,
 // upward otherwise.
 function otherPage(currentPage: TreeItemRecord, delta: number) {
@@ -75,3 +121,94 @@ function otherPage(currentPage: TreeItemRecord, delta: number) {
   const docPage = records[index].viewRef;
   return urlState().pushUrl({docPage});
 }
+
+function buildPrompt(tableNames: string[], onSave: (option: RemoveOption) => Promise<any>) {
+  saveModal((ctl, owner): ISaveModalOptions => {
+    const selected = Observable.create<RemoveOption>(owner, '');
+    const saveDisabled = Computed.create(owner, use => use(selected) === '');
+    const saveFunc = () => onSave(selected.get());
+    return {
+      title: `The following table${tableNames.length > 1 ? 's' : ''} will no longer be visible`,
+      body: dom('div',
+        testId('popup'),
+        buildWarning(tableNames),
+        cssOptions(
+          buildOption(selected, 'data', `Delete data and this page.`),
+          buildOption(selected, 'page',
+            [
+              `Delete this page, but do not delete data. `,
+              `Table will remain available in `,
+              cssLink(urlState().setHref({docPage: 'data'}), 'raw data page', { target: '_blank'}),
+              `.`
+            ]),
+        )
+      ),
+      saveDisabled,
+      saveLabel: 'Delete',
+      saveFunc,
+      width: 'fixed-wide',
+      extraButtons: [],
+    };
+  });
+}
+
+function buildOption(value: Observable<RemoveOption>, id: RemoveOption, content: DomContents) {
+  const selected = Computed.create(null, use => use(value) === id)
+    .onWrite(val => val ? value.set(id) : void 0);
+  return dom.update(
+    labeledCircleCheckbox(selected, content, dom.autoDispose(selected)),
+    testId(`option-${id}`),
+    cssBlockCheckbox.cls(''),
+    cssBlockCheckbox.cls('-block', selected),
+  );
+}
+
+function buildWarning(tables: string[]) {
+  return cssWarning(
+    dom.forEach(tables, (t) => cssTableName(t, testId('table')))
+  );
+}
+
+const cssOptions = styled('div', `
+  display: flex;
+  flex-direction: column;
+  gap: 10px;
+`);
+
+// We need to reset top and left of ::before element, as it is wrongly set
+// on the inline checkbox.
+// To simulate radio button behavior, we will block user input after option is selected, because
+// checkbox doesn't support two-way binding.
+const cssBlockCheckbox = styled('div', `
+  display: flex;
+  padding: 10px 8px;
+  border: 1px solid ${colors.mediumGrey};
+  border-radius: 3px;
+  cursor: pointer;
+  & input::before, & input::after  {
+    top: unset;
+    left: unset;
+  }
+  &:hover {
+    border-color: ${colors.lightGreen};
+  }
+  &-block {
+    pointer-events: none;
+  }
+  &-block a {
+    pointer-events: all;
+  }
+`);
+
+const cssWarning = styled('div', `
+  display: flex;
+  flex-wrap: wrap;
+  gap: 8px;
+  margin-bottom: 16px;
+`);
+
+const cssTableName = styled('div', `
+  background: #eee;
+  padding: 3px 6px;
+  border-radius: 4px;
+`);

app/client/ui/WidgetTitle.ts

@@ -60,16 +60,18 @@ function buildWidgetRenamePopup(ctrl: IOpenController, vs: ViewSectionRec, optio
   // User input for table name.
   const inputTableName = Observable.create(ctrl, tableName);
   // User input for widget title.
-  const inputWidgetTitle = Observable.create(ctrl, vs.title.peek());
+  const inputWidgetTitle = Observable.create(ctrl, vs.title.peek() ?? '');
   // Placeholder for widget title:
   // - when widget title is empty shows a default widget title (what would be shown when title is empty)
   // - when widget title is set, shows just a text to override it.
   const inputWidgetPlaceholder = !vs.title.peek() ? 'Override widget title' : vs.defaultWidgetTitle.peek();
 
-  const disableSave = Computed.create(ctrl, (use) =>
-    (use(inputTableName) === tableName || use(inputTableName).trim() === '') &&
-    use(inputWidgetTitle) === vs.title.peek()
-  );
+  const disableSave = Computed.create(ctrl, (use) => {
+    const newTableName = use(inputTableName)?.trim() ?? '';
+    const newWidgetTitle = use(inputWidgetTitle)?.trim() ?? '';
+    // Can't save when table name is empty or there wasn't any change.
+    return !newTableName || (newTableName === tableName && newWidgetTitle === use(vs.title));
+  });
 
   const modalCtl = ModalControl.create(ctrl, () => ctrl.close());
 
@@ -88,9 +90,10 @@ function buildWidgetRenamePopup(ctrl: IOpenController, vs: ViewSectionRec, optio
   };
 
   const saveWidgetTitle = async () => {
+    const newTitle = inputWidgetTitle.get()?.trim() ?? '';
     // If value was changed.
-    if (inputWidgetTitle.get() !== vs.title.peek()) {
-      await vs.title.saveOnly(inputWidgetTitle.get());
+    if (newTitle !== vs.title.peek()) {
+      await vs.title.saveOnly(newTitle);
     }
   };
   const doSave = modalCtl.doWork(() => Promise.all([
@@ -99,23 +102,20 @@ function buildWidgetRenamePopup(ctrl: IOpenController, vs: ViewSectionRec, optio
   ]), {close: true});
 
   function initialFocus() {
-    // Set focus on a thing user is likely to change.
-    // Initial focus is set on tableName unless:
-    // - if this is a summary table - as it is not editable,
-    // - if widgetTitle is not empty - so user wants to change it further,
-    // - if widgetTitle is empty but the default widget name will have type suffix (like Table1 (Card)), so it won't
-    //   be a table name - so user doesn't want the default value.
-    if (
-       !widgetInput ||
-       isSummary ||
-       vs.title.peek() ||
-       (
-          !vs.title.peek() &&
-          vs.defaultWidgetTitle.peek().toUpperCase() !== tableRec.tableName.peek().toUpperCase()
-       )) {
-      widgetInput?.focus();
-    } else if (!isSummary) {
-      tableInput?.focus();
+    const isRawView = !widgetInput;
+    const isWidgetTitleEmpty = !vs.title.peek();
+    function focus(inputEl?: HTMLInputElement) {
+      inputEl?.focus();
+      inputEl?.select();
+    }
+    if (isSummary) {
+      focus(widgetInput);
+    } else if (isRawView) {
+      focus(tableInput);
+    } else if (isWidgetTitleEmpty) {
+      focus(tableInput);
+    } else {
+      focus(widgetInput);
     }
   }
 

app/client/ui2018/checkbox.ts

@@ -34,6 +34,8 @@ export const cssLabel = styled('label', `
   }
 `);
 
+
+
 // TODO: the !important markings are to trump bootstrap, and should be removed when it's gone.
 export const cssCheckboxSquare = styled('input', `
   -webkit-appearance: none;

app/client/ui2018/links.ts

@@ -7,7 +7,6 @@ import { dom, IDomArgs, Observable, styled } from 'grainjs';
  * Styling for a simple green <A HREF> link.
  */
 
-// Match the font-weight of buttons.
 export const cssLink = styled('a', `
   color: ${colors.lightGreen};
   --icon-color: ${colors.lightGreen};

app/client/ui2018/modals.ts

@@ -1,10 +1,11 @@
 import {FocusLayer} from 'app/client/lib/FocusLayer';
 import {reportError} from 'app/client/models/errors';
+import {cssInput} from 'app/client/ui/MakeCopyMenu';
 import {bigBasicButton, bigPrimaryButton, cssButton} from 'app/client/ui2018/buttons';
 import {colors, mediaSmall, testId, vars} from 'app/client/ui2018/cssVars';
 import {loadingSpinner} from 'app/client/ui2018/loaders';
 import {waitGrainObs} from 'app/common/gutil';
-import {Computed, Disposable, dom, DomContents, DomElementArg, MultiHolder, Observable, styled} from 'grainjs';
+import {Computed, Disposable, dom, DomContents, DomElementArg, input, MultiHolder, Observable, styled} from 'grainjs';
 
 // IModalControl is passed into the function creating the body of the modal.
 export interface IModalControl {
@@ -287,6 +288,93 @@ export function confirmModal(
   }));
 }
 
+
+/**
+ * Creates a simple prompt modal (replacement for the native one).
+ * Closed via clicking anywhere outside the modal or Cancel button.
+ *
+ * Example usage:
+ *  promptModal(
+ *    "Enter your name",
+ *    (name: string) => alert(`Hello ${name}`),
+ *    "Ok" // Confirm button name,
+ *    "John doe", // Initial text (can be empty or undefined)
+ *    "Enter your name", // input placeholder
+ *    () => console.log('User cancelled') // Called when user cancels, or clicks outside.
+ *  )
+ *
+ * @param title: Prompt text.
+ * @param onConfirm: Handler for Confirm button.
+ * @param btnText: Text of the confirm button.
+ * @param initial: Initial value in the input element.
+ * @param placeholder: Placeholder for the input element.
+ * @param onCancel: Optional cancel handler.
+ */
+export function promptModal(
+  title: string,
+  onConfirm: (text: string) => Promise<unknown>,
+  btnText: string,
+  initial?: string,
+  placeholder?: string,
+  onCancel?: () => void
+): void {
+  saveModal((ctl, owner): ISaveModalOptions => {
+    let confirmed = false;
+    const text = Observable.create(owner, initial ?? '');
+    const txtInput = input(text, { onInput : true }, { placeholder }, cssInput.cls(''), testId('modal-prompt'));
+    const options: ISaveModalOptions = {
+      title,
+      body: txtInput,
+      saveLabel: btnText,
+      saveFunc: () => {
+        // Mark that confirm was invoked.
+        confirmed = true;
+        return onConfirm(text.get() || '');
+      },
+      width: 'normal'
+    };
+    owner.onDispose(() => {
+      if (confirmed) { return; }
+      onCancel?.();
+    });
+    setTimeout(() => txtInput.focus(), 10);
+    return options;
+  });
+}
+
+/**
+ * Wraps prompt modal in a promise that is resolved either when user confirms or cancels.
+ * When user cancels the returned value is always undefined.
+ *
+ * Example usage:
+ *  async handler() {
+ *    const name = await invokePrompt("Please enter your name");
+ *    if (name !== undefined) alert(`Hello ${name}`);
+ *  }
+ *
+ * @param title: Prompt text.
+ * @param btnText: Text of the confirm button, default is "Ok".
+ * @param initial: Initial value in the input element.
+ * @param placeholder: Placeholder for the input element.
+ */
+export function invokePrompt(
+  title: string,
+  btnText?: string,
+  initial?: string,
+  placeholder?: string
+): Promise<string|undefined> {
+  let onResolve: (text: string|undefined) => any;
+  const prom = new Promise<string|undefined>((resolve) => {
+    onResolve = resolve;
+  });
+  promptModal(title, onResolve!, btnText ?? 'Ok', initial, placeholder, () => {
+    if (onResolve) {
+      onResolve(undefined);
+    }
+  });
+  return prom;
+}
+
 /**
  * Builds a simple spinner modal. The modal gets removed when `promise` resolves.
  */

app/server/lib/ActiveDoc.ts

@@ -611,7 +611,7 @@ export class ActiveDoc extends EventEmitter {
   public addInitialTable(docSession: OptDocSession) {
     // Use a non-client-specific session, so that this action is not part of anyone's undo history.
     const newDocSession = makeExceptionalDocSession('nascent');
-    return this.applyUserActions(newDocSession, [["AddEmptyTable"]]);
+    return this.applyUserActions(newDocSession, [["AddEmptyTable", null]]);
   }
 
   /**