(core) Enable Record Cards

Summary:
Adds remaining functionality, fixes, and polish to Record Cards and
removes their feature flag, enabling them by default.

Test Plan: Tests deferred; will be included in a follow-up diff.

Reviewers: jarek, paulfitz

Reviewed By: jarek

Subscribers: paulfitz, jarek

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

app/client/components/BaseView.js

@@ -274,7 +274,9 @@ BaseView.prototype.deleteRecords = function(source) {
     buildConfirmDelete(selectedCell, onSave, rowIds.length <= 1);
   } else {
     onSave().then(() => {
-      reportUndo(this.gristDoc, `You deleted ${rowIds.length} row${rowIds.length > 1 ? 's' : ''}.`);
+      if (!this.isDisposed()) {
+        reportUndo(this.gristDoc, `You deleted ${rowIds.length} row${rowIds.length > 1 ? 's' : ''}.`);
+      }
       return true;
     });
   }

app/client/components/DataTables.ts

@@ -3,7 +3,6 @@ import {GristDoc} from 'app/client/components/GristDoc';
 import {copyToClipboard} from 'app/client/lib/clipboardUtils';
 import {setTestState} from 'app/client/lib/testState';
 import {TableRec} from 'app/client/models/DocModel';
-import {RECORD_CARDS} from 'app/client/models/features';
 import {docListHeader, docMenuTrigger} from 'app/client/ui/DocMenuCss';
 import {duplicateTable, DuplicateTableResponse} from 'app/client/ui/DuplicateTable';
 import {hoverTooltip, showTransientTooltip} from 'app/client/ui/tooltips';
@@ -99,13 +98,14 @@ export class DataTables extends Disposable {
                   hoverTooltip(
                     dom.domComputed(use => use(use(tableRec.recordCardViewSection).disabled)
                       ? t('Record Card Disabled')
-                      : t('Record Card')),
+                      : t('Edit Record Card')),
                     {key: DATA_TABLES_TOOLTIP_KEY, closeOnClick: false}
                   ),
-                  dom.hide(!RECORD_CARDS()),
+                  dom.hide(this._gristDoc.isReadonly),
                   // Make the button invisible to maintain consistent alignment with non-summary tables.
                   dom.style('visibility', u => u(tableRec.summarySourceTable) === 0 ? 'visible' : 'hidden'),
                   cssRecordCardButton.cls('-disabled', use => use(use(tableRec.recordCardViewSection).disabled)),
+                  testId('table-record-card'),
                 ),
                 cssDotsButton(
                   testId('table-menu'),
@@ -120,7 +120,8 @@ export class DataTables extends Disposable {
                   throw new Error(`Table ${tableRec.tableId.peek()} doesn't have a raw view section.`);
                 }
                 this._gristDoc.viewModel.activeSectionId(sectionId);
-              })
+              }),
+              cssTable.cls('-readonly', this._gristDoc.isReadonly),
             );
           })
         ),
@@ -132,7 +133,8 @@ export class DataTables extends Disposable {
     return dom.domComputed((use) => {
       const rawViewSectionRef = use(fromKo(table.rawViewSectionRef));
       const isSummaryTable = use(table.summarySourceTable) !== 0;
-      if (!rawViewSectionRef || isSummaryTable) {
+      const isReadonly = use(this._gristDoc.isReadonly);
+      if (!rawViewSectionRef || isSummaryTable || isReadonly) {
         // Some very old documents might not have a rawViewSection, and raw summary
         // tables can't currently be renamed.
         const tableName = [
@@ -185,7 +187,7 @@ export class DataTables extends Disposable {
         )),
         testId('menu-remove-table'),
       ),
-      dom.maybe(use => RECORD_CARDS() && use(table.summarySourceTable) === 0, () => [
+      dom.maybe(use => use(table.summarySourceTable) === 0, () => [
         menuDivider(),
         menuItem(
           () => this._editRecordCard(table),
@@ -308,6 +310,10 @@ const cssTable = styled('div', `
   &:hover {
     border-color: ${css.theme.rawDataTableBorderHover};
   }
+  &-readonly {
+    /* Row count column is hidden when document is read-only. */
+    grid-template-columns: 16px auto 56px;
+  }
 `);
 
 const cssTableIcon = styled('div', `

app/client/components/DetailView.js

@@ -33,6 +33,7 @@ function DetailView(gristDoc, viewSectionModel) {
 
   this.viewFields = gristDoc.docModel.viewFields;
   this._isSingle = (this.viewSection.parentKey.peek() === 'single');
+  this._isExternalSectionPopup = gristDoc.externalSectionId.get() === this.viewSection.id();
 
   //--------------------------------------------------
   // Create and attach the DOM for the view.
@@ -191,7 +192,9 @@ DetailView.prototype.deleteRows = async function(rowIds) {
   try {
     await BaseView.prototype.deleteRows.call(this, rowIds);
   } finally {
-    this.cursor.rowIndex(index);
+    if (!this.isDisposed()) {
+      this.cursor.rowIndex(index);
+    }
   }
 };
 
@@ -365,7 +368,13 @@ DetailView.prototype.buildTitleControls = function() {
   // the controls can be confusing in this case.
   // Note that the controls should still be visible with a filter link.
   const showControls = ko.computed(() => {
-    if (!this._isSingle || this.recordLayout.layoutEditor()) { return false; }
+    if (
+      !this._isSingle||
+      this.recordLayout.layoutEditor() ||
+      this._isExternalSectionPopup
+    ) {
+      return false;
+    }
     const linkingState = this.viewSection.linkingState();
     return !(linkingState && Boolean(linkingState.cursorPos));
   });

app/client/components/GridView.js

@@ -56,7 +56,6 @@ const {CombinedStyle} = require("app/client/models/Styles");
 const {buildRenameColumn} = require('app/client/ui/ColumnTitle');
 const {makeT} = require('app/client/lib/localization');
 const {reportError} = require('app/client/models/AppModel');
-const {RECORD_CARDS} = require('app/client/models/features');
 const {urlState} = require('app/client/models/gristUrlState');
 
 const t = makeT('GridView');
@@ -375,13 +374,15 @@ GridView.gridCommands = {
     this.viewSection.rawNumFrozen.setAndSave(action.numFrozen);
   },
   viewAsCard() {
-    if (!RECORD_CARDS()) { return; }
     if (this._isRecordCardDisabled()) { return; }
 
     const selectedRows = this.selectedRows();
+    if (selectedRows.length !== 1) { return; }
+
+    const colRef = this.viewSection.viewFields().at(this.cursor.fieldIndex()).column().id();
     const rowId = selectedRows[0];
     const sectionId = this.viewSection.tableRecordCard().id();
-    const anchorUrlState = {hash: {rowId, sectionId, recordCard: true}};
+    const anchorUrlState = {hash: {colRef, rowId, sectionId, recordCard: true}};
     urlState().pushUrl(anchorUrlState, {replace: true}).catch(reportError);
   },
 };

app/client/components/GristDoc.ts

@@ -209,6 +209,8 @@ export class GristDoc extends DisposableWithEvents {
   private _showBackgroundVideoPlayer: Observable<boolean> = Observable.create(this, false);
   private _backgroundVideoPlayerHolder: Holder<YouTubePlayer> = Holder.create(this);
   private _disableAutoStartingTours: boolean = false;
+  private _isShowingPopupSection = false;
+  private _prevSectionId: number | null = null;
 
   constructor(
     public readonly app: App,
@@ -565,6 +567,13 @@ export class GristDoc extends DisposableWithEvents {
         commands.allCommands.viewTabFocus.run();
       }
     }));
+
+    this.autoDispose(this._popupSectionOptions.addListener((popupOptions) => {
+      if (!popupOptions) {
+        this._isShowingPopupSection = false;
+        this._prevSectionId = null;
+      }
+    }));
   }
 
   /**
@@ -616,10 +625,16 @@ export class GristDoc extends DisposableWithEvents {
             // In case the section is removed, close the popup.
             content.viewSection.autoDispose({dispose: content.close});
 
-            const {recordCard} = content.hash;
+            const {recordCard, rowId} = content.hash;
             if (recordCard) {
+              if (!rowId || rowId === 'new') {
+                // Should be unreachable, but just to be sure (and to satisfy type checking)...
+                throw new Error('Unable to open Record Card: undefined row id');
+              }
+
               return dom.create(RecordCardPopup, {
                 gristDoc: this,
+                rowId,
                 viewSection: content.viewSection,
                 onClose: content.close,
               });
@@ -629,7 +644,15 @@ export class GristDoc extends DisposableWithEvents {
           }) :
           dom.create((owner) => {
             this.viewLayout = ViewLayout.create(owner, this, content);
-            this.viewLayout.maximized.addListener(n => this.maximizedSectionId.set(n));
+            this.viewLayout.maximized.addListener(sectionId => {
+              this.maximizedSectionId.set(sectionId);
+
+              if (sectionId === null && !this._isShowingPopupSection) {
+                // If we didn't navigate to another section in the popup, move focus
+                // back to the previous section.
+                this._focusPreviousSection();
+              }
+            });
             owner.onDispose(() => this.viewLayout = null);
             return this.viewLayout;
           })
@@ -1290,11 +1313,11 @@ export class GristDoc extends DisposableWithEvents {
     if (!hash.sectionId) {
       return;
     }
+    if (!this._prevSectionId) {
+      this._prevSectionId = this.viewModel.activeSection.peek().id();
+    }
     // We might open popup either for a section in this view or some other section (like Raw Data Page).
     if (this.viewModel.viewSections.peek().peek().some(s => s.id.peek() === hash.sectionId)) {
-      if (this.viewLayout) {
-        this.viewLayout.previousSectionId = this.viewModel.activeSectionId.peek();
-      }
       this.viewModel.activeSectionId(hash.sectionId);
       // If the anchor link is valid, set the cursor.
       if (hash.colRef && hash.rowId) {
@@ -1308,10 +1331,10 @@ export class GristDoc extends DisposableWithEvents {
       this.viewLayout?.maximized.set(hash.sectionId);
       return;
     }
+    this._isShowingPopupSection = true;
     // We will borrow active viewModel and will trick him into believing that
     // the section from the link is his viewSection and it is active. Fortunately
     // he doesn't care. After popup is closed, we will restore the original.
-    const prevSection = this.viewModel.activeSection.peek();
     this.viewModel.activeSectionId(hash.sectionId);
     // Now we have view section we want to show in the popup.
     const popupSection = this.viewModel.activeSection.peek();
@@ -1329,20 +1352,17 @@ export class GristDoc extends DisposableWithEvents {
         if (!this._popupSectionOptions.get()) {
           return;
         }
-        if (popupSection !== prevSection) {
+        if (popupSection.id() !== this._prevSectionId) {
           // We need to blur the popup section. Otherwise it will automatically be opened
           // on raw data view. Note: raw data and record card sections don't have parent views;
           // they use the empty row model as a parent (which feels like a hack).
           if (!popupSection.isDisposed()) {
             popupSection.hasFocus(false);
           }
-          // We need to restore active viewSection for a view that we borrowed.
           // When this popup was opened we tricked active view by setting its activeViewSection
-          // to our viewSection (which might be a completely diffrent section or a raw data section) not
-          // connected to this view.
-          if (!prevSection.isDisposed()) {
-            prevSection.hasFocus(true);
-          }
+          // to our viewSection (which might be a completely different section or a raw data section) not
+          // connected to this view. We need to return focus back to the previous section.
+          this._focusPreviousSection();
         }
         // Clearing popup section data will close this popup.
         this._popupSectionOptions.set(null);
@@ -1401,6 +1421,19 @@ export class GristDoc extends DisposableWithEvents {
     this._showBackgroundVideoPlayer.set(false);
   }
 
+  private _focusPreviousSection() {
+    const prevSectionId = this._prevSectionId;
+    if (!prevSectionId) { return; }
+
+    if (
+      this.viewModel.viewSections.peek().all().some(s =>
+        !s.isDisposed() && s.id.peek() === prevSectionId)
+    ) {
+      this.viewModel.activeSectionId(prevSectionId);
+    }
+    this._prevSectionId = null;
+  }
+
   /**
    * Waits for a view to be ready
    */

app/client/components/RecordCardPopup.ts

@@ -2,22 +2,27 @@ import {buildViewSectionDom} from 'app/client/components/buildViewSectionDom';
 import * as commands from 'app/client/components/commands';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {cssCloseButton, cssOverlay} from 'app/client/components/RawDataPage';
-import {ViewSectionRec} from 'app/client/models/DocModel';
 import {ViewSectionHelper} from 'app/client/components/ViewLayout';
+import {ViewSectionRec} from 'app/client/models/DocModel';
+import {ChangeType, RowList} from 'app/client/models/rowset';
 import {theme} from 'app/client/ui2018/cssVars';
-import {Disposable, dom, makeTestId, styled} from 'grainjs';
+import {DisposableWithEvents} from 'app/common/DisposableWithEvents';
+import {dom, makeTestId, styled} from 'grainjs';
 
 const testId = makeTestId('test-record-card-popup-');
 
 interface RecordCardPopupOptions {
   gristDoc: GristDoc;
+  rowId: number;
   viewSection: ViewSectionRec;
   onClose(): void;
 }
 
-export class RecordCardPopup extends Disposable {
+export class RecordCardPopup extends DisposableWithEvents {
   private _gristDoc = this._options.gristDoc;
+  private _rowId = this._options.rowId;
   private _viewSection = this._options.viewSection;
+  private _tableModel = this._gristDoc.getTableModel(this._viewSection.table().tableId());
   private _handleClose = this._options.onClose;
 
   constructor(private _options: RecordCardPopupOptions) {
@@ -26,6 +31,11 @@ export class RecordCardPopup extends Disposable {
       cancel: () => { this._handleClose(); },
     };
     this.autoDispose(commands.createGroup(commandGroup, this, true));
+
+    // Close the popup if the underlying row is removed.
+    const onRowChange = this._onRowChange.bind(this);
+    this._tableModel.on('rowChange', onRowChange);
+    this.onDispose(() => this._tableModel.off('rowChange', onRowChange));
   }
 
   public buildDom() {
@@ -39,7 +49,6 @@ export class RecordCardPopup extends Disposable {
           draggable: false,
           focusable: false,
           renamable: false,
-          hideTitleControls: true,
         }),
       ),
       cssCloseButton('CrossBig',
@@ -49,6 +58,12 @@ export class RecordCardPopup extends Disposable {
       dom.on('click', (ev, elem) => void (ev.target === elem ? this._handleClose() : null)),
     );
   }
+
+  private _onRowChange(type: ChangeType, rows: RowList) {
+    if (type === 'remove' && [...rows].includes(this._rowId)) {
+      this._handleClose();
+    }
+  }
 }
 
 const cssSectionWrapper = styled('div', `

app/client/components/ViewLayout.ts

@@ -84,7 +84,6 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
   public viewModel: ViewRec;
   public layoutSpec: ko.Computed<BoxSpec>;
   public maximized: Observable<number|null>;
-  public previousSectionId = 0; // Used to restore focus after a maximized section is closed.
   public isResizing = Observable.create(this, false);
   public layout: Layout;
   public layoutEditor: LayoutEditor;
@@ -203,16 +202,6 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
       // If we are closing popup, resize all sections.
       if (!sectionId) {
         this._onResize();
-        // Reset active section to the first one if the section is popup is collapsed.
-        if (prev
-          && this.viewModel.activeCollapsedSections.peek().includes(prev)
-          && this.previousSectionId) {
-        // Make sure that previous section exists still.
-        if (this.viewModel.viewSections.peek().all()
-                .some(s => !s.isDisposed() && s.id.peek() === this.previousSectionId)) {
-          this.viewModel.activeSectionId(this.previousSectionId);
-        }
-      }
       } else {
         // Otherwise resize only active one (the one in popup).
         const section = this.viewModel.activeSection.peek();

app/client/components/buildViewSectionDom.ts

@@ -78,7 +78,6 @@ export function buildViewSectionDom(options: {
     tableNameHidden,
     widgetNameHidden,
     renamable = true,
-    hideTitleControls = false,
   } = options;
 
   // Creating normal section dom
@@ -110,7 +109,7 @@ export function buildViewSectionDom(options: {
         testId('viewsection-title'),
         cssTestClick(testId("viewsection-blank")),
       ),
-      hideTitleControls ? null : viewInstance.buildTitleControls(),
+      viewInstance.buildTitleControls(),
       dom('div.viewsection_buttons',
         dom.create(viewSectionMenu, gristDoc, vs)
       )