(core) Showing a raw data section on a popup

Summary:
Show raw data will now open a popup with
raw section instead of redirecting to raw data page.

Adding new anchor link type "a2" that is able to open
any section in a popup on a current view.

Not related:
Fixing highlightMatches function, after merging core PR.

Test Plan: Updated tests

Reviewers: alexmojaki, georgegevoian

Reviewed By: alexmojaki, georgegevoian

Subscribers: georgegevoian, alexmojaki

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

app/client/components/BaseView.js

@@ -307,7 +307,7 @@ BaseView.prototype.getAnchorLinkForSection = function(sectionId) {
       // Note that this case only happens in combination with the widget linking mentioned.
       // If the table is empty but the 'new record' row is selected, the `viewData.getRowId` line above works.
       || 'new';
-  const colRef = this.viewSection.viewFields().peek()[this.cursor.fieldIndex()].colRef();
+  const colRef = this.viewSection.viewFields().peek()[this.cursor.fieldIndex.peek()].colRef.peek();
   return {hash: {sectionId, rowId, colRef}};
 }
 
@@ -328,7 +328,8 @@ BaseView.prototype.copyLink = async function() {
 BaseView.prototype.showRawData = async function() {
   const sectionId = this.schemaModel.rawViewSectionRef.peek();
   const anchorUrlState = this.getAnchorLinkForSection(sectionId);
-  await urlState().pushUrl({...anchorUrlState, docPage: 'data'});
+  anchorUrlState.hash.popup = true;
+  await urlState().pushUrl(anchorUrlState, {replace: true, avoidReload: true});
 }
 
 BaseView.prototype.filterByThisCellValue = function() {

app/client/components/GristDoc.ts

@@ -17,7 +17,7 @@ import {Drafts} from "app/client/components/Drafts";
 import {EditorMonitor} from "app/client/components/EditorMonitor";
 import * as GridView from 'app/client/components/GridView';
 import {Importer} from 'app/client/components/Importer';
-import {RawData} from 'app/client/components/RawData';
+import {RawDataPage, RawDataPopup} from 'app/client/components/RawDataPage';
 import {ActionGroupWithCursorPos, UndoStack} from 'app/client/components/UndoStack';
 import {ViewLayout} from 'app/client/components/ViewLayout';
 import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals';
@@ -67,6 +67,7 @@ import {
   Computed,
   dom,
   Emitter,
+  fromKo,
   Holder,
   IDisposable,
   IDomComponent,
@@ -108,6 +109,12 @@ export interface IExtraTool {
   content: TabContent[]|IDomComponent;
 }
 
+interface PopupOptions {
+  viewSection: ViewSectionRec;
+  hash: HashLink;
+  close: () => void;
+}
+
 export class GristDoc extends DisposableWithEvents {
   public docModel: DocModel;
   public viewModel: ViewRec;
@@ -159,6 +166,9 @@ export class GristDoc extends DisposableWithEvents {
   private _viewLayout: ViewLayout|null = null;
   private _showGristTour = getUserOrgPrefObs(this.userOrgPrefs, 'showGristTour');
   private _seenDocTours = getUserOrgPrefObs(this.userOrgPrefs, 'seenDocTours');
+  private _popupOptions: Observable<PopupOptions|null> = Observable.create(this, null);
+  private _activeContent: Computed<IDocPage|PopupOptions>;
+
 
   constructor(
     public readonly app: App,
@@ -204,6 +214,8 @@ export class GristDoc extends DisposableWithEvents {
       return viewId || use(defaultViewId);
     });
 
+    this._activeContent = Computed.create(this, use => use(this._popupOptions) ?? use(this.activeViewId));
+
     // This viewModel reflects the currently active view, relying on the fact that
     // createFloatingRowModel() supports an observable rowId for its argument.
     // Although typings don't reflect it, createFloatingRowModel() accepts non-numeric values,
@@ -224,12 +236,17 @@ export class GristDoc extends DisposableWithEvents {
       }
     }));
 
-    // Navigate to an anchor if one is present in the url hash.
+    // Subscribe to URL state, and navigate to anchor or open a popup if necessary.
     this.autoDispose(subscribe(urlState().state, async (use, state) => {
       if (state.hash) {
         try {
-          const cursorPos = this._getCursorPosFromHash(state.hash);
+          if (state.hash.popup) {
-          await this.recursiveMoveToCursorPos(cursorPos, true);
+            await this.openPopup(state.hash);
+          } else {
+            // Navigate to an anchor if one is present in the url hash.
+            const cursorPos = this._getCursorPosFromHash(state.hash);
+            await this.recursiveMoveToCursorPos(cursorPos, true);
+          }
         } catch (e) {
           reportError(e);
         } finally {
@@ -332,22 +349,23 @@ export class GristDoc extends DisposableWithEvents {
 
     // create computed observable for viewInstance - if it is loaded or not
 
-    // Add an artificial intermediary computed only to delay the evaluation of currentView, so
+    // GrainJS will not recalculate section.viewInstance correctly because it will be
-    // that it happens after section.viewInstance is set. If it happens before, then
+    // modified (updated from null to a correct instance) in the same tick. We need to
-    // section.viewInstance is seen as null, and as it gets updated, GrainJS refuses to
+    // switch for a moment to knockout to fix this.
-    // recalculate this computed since it was already calculated in the same tick.
+    const viewInstance = fromKo(this.autoDispose(ko.pureComputed(() => {
-    const activeViewId = Computed.create(this, (use) => use(this.activeViewId));
+      const viewId = toKo(ko, this.activeViewId)();
-    const viewInstance = Computed.create(this, (use) => {
+      if (!isViewDocPage(viewId)) { return null; }
-      const section = use(this.viewModel.activeSection);
+      const section = this.viewModel.activeSection();
-      const viewId = use(activeViewId);
+      const view = section.viewInstance();
-      const view = use(section.viewInstance);
+      return view;
-      return isViewDocPage(viewId) ? view : null;
+    })));
-    });
+
     // then listen if the view is present, because we still need to wait for it load properly
     this.autoDispose(viewInstance.addListener(async (view) => {
       if (view) {
         await view.getLoadingDonePromise();
       }
+      if (view?.isDisposed()) { return; }
       // finally set the current view as fully loaded
       this.currentView.set(view);
     }));
@@ -355,9 +373,8 @@ export class GristDoc extends DisposableWithEvents {
     // create observable for current cursor position
     this.cursorPosition = Computed.create<ViewCursorPos | undefined>(this, use => {
       // get the BaseView
-      const view = use(viewInstance);
+      const view = use(this.currentView);
       if (!view) { return undefined; }
-      // get current viewId
       const viewId = use(this.activeViewId);
       if (!isViewDocPage(viewId)) { return undefined; }
       // read latest position
@@ -393,15 +410,25 @@ export class GristDoc extends DisposableWithEvents {
    * Builds the DOM for this GristDoc.
    */
   public buildDom() {
-    return cssViewContentPane(testId('gristdoc'),
+    return cssViewContentPane(
-      cssViewContentPane.cls("-contents", use => use(this.activeViewId) === 'data'),
+      testId('gristdoc'),
-      dom.domComputed<IDocPage>(this.activeViewId, (viewId) => (
+      cssViewContentPane.cls("-contents", use => use(this.activeViewId) === 'data' || use(this._popupOptions) !== null),
-        viewId === 'code' ? dom.create(CodeEditorPanel, this) :
+      dom.domComputed(this._activeContent, (content) => {
-        viewId === 'acl' ? dom.create(AccessRules, this) :
+        return  (
-        viewId === 'data' ? dom.create(RawData, this) :
+          content === 'code' ? dom.create(CodeEditorPanel, this) :
-        viewId === 'GristDocTour' ? null :
+          content === 'acl' ? dom.create(AccessRules, this) :
-        dom.create((owner) => (this._viewLayout = ViewLayout.create(owner, this, viewId)))
+          content === 'data' ? dom.create(RawDataPage, this) :
-      )),
+          content === 'GristDocTour' ? null :
+          typeof content === 'object' ? dom.create(owner => {
+            // In case user changes a page, close the popup.
+            owner.autoDispose(this.activeViewId.addListener(content.close));
+            // In case the section is removed, close the popup.
+            content.viewSection.autoDispose({dispose: content.close});
+            return dom.create(RawDataPopup, this, content.viewSection, content.close);
+          }) :
+          dom.create((owner) => (this._viewLayout = ViewLayout.create(owner, this, content)))
+        );
+      }),
     );
   }
 
@@ -934,17 +961,82 @@ export class GristDoc extends DisposableWithEvents {
     await tableRec.tableName.saveOnly(newTableName);
   }
 
+  /**
+   * Opens popup with a section data (used by Raw Data view).
+   */
+  public async openPopup(hash: HashLink) {
+    // We can only open a popup for a section.
+    if (!hash.sectionId) { return; }
+    // 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();
+    // We need to make it active, so that cursor on this section will be the
+    // active one. This will change activeViewSectionId on a parent view of this section,
+    // which might be a diffrent view from what we currently have. If the section is
+    // a raw data section it will use `EmptyRowModel` as raw sections don't have parents.
+    popupSection.hasFocus(true);
+    this._popupOptions.set({
+      hash,
+      viewSection: popupSection,
+      close: () => {
+        // In case we are already close, do nothing.
+        if (!this._popupOptions.get()) { return; }
+        if (popupSection !== prevSection) {
+          // We need to blur raw view section. Otherwise it will automatically be opened
+          // on raw data view. Note: raw data section doesn't have its own view, it uses
+          // 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); }
+        }
+        // Clearing popup data will close this popup.
+        this._popupOptions.set(null);
+      }
+    });
+    // If the anchor link is valid, set the cursor.
+    if (hash.colRef && hash.rowId) {
+      const fieldIndex = popupSection.viewFields.peek().all().findIndex(f => f.colRef.peek() === hash.colRef);
+      if (fieldIndex >= 0) {
+        const view = await this._waitForView(popupSection);
+        view?.setCursorPos({ sectionId: hash.sectionId, rowId: hash.rowId, fieldIndex });
+      }
+    }
+  }
+
   /**
    * Waits for a view to be ready
    */
-  private async _waitForView() {
+  private async _waitForView(popupSection?: ViewSectionRec) {
+    const sectionToCheck = popupSection ?? this.viewModel.activeSection.peek();
     // For pages like ACL's, there isn't a view instance to wait for.
-    if (!this.viewModel.activeSection.peek().getRowId()) {
+    if (!sectionToCheck.getRowId()) {
       return null;
     }
-    const view = await waitObs(this.viewModel.activeSection.peek().viewInstance);
+    async function singleWait(s: ViewSectionRec): Promise<BaseView> {
-    if (!view) {
+      const view = await waitObs(
-      return null;
+        sectionToCheck.viewInstance,
+        vsi => Boolean(vsi && !vsi.isDisposed())
+      );
+      return view!;
+    }
+    let view = await singleWait(sectionToCheck);
+    if (view.isDisposed()) {
+      // If the view is disposed (it can happen, as wait is not reliable enough, because it uses
+      // subscription for testing the predicate, which might dispose object before we have a chance to test it).
+      // This can happen when section is recreating itself on a popup.
+      if (popupSection) {
+        view = await singleWait(popupSection);
+      }
+      if (view.isDisposed()) {
+        return null;
+      }
     }
     await view.getLoadingDonePromise();
     // Wait extra bit for scroll to happen.

app/client/components/RawDataPage.ts

@@ -8,15 +8,15 @@ import {colors, mediaSmall, vars} from 'app/client/ui2018/cssVars';
 import {icon} from 'app/client/ui2018/icons';
 import {Computed, Disposable, dom, fromKo, makeTestId, Observable, styled} from 'grainjs';
 import {reportError} from 'app/client/models/errors';
+import {ViewSectionRec} from 'app/client/models/DocModel';
 
 const testId = makeTestId('test-raw-data-');
 
-export class RawData extends Disposable {
+export class RawDataPage extends Disposable {
   private _lightboxVisible: Observable<boolean>;
   constructor(private _gristDoc: GristDoc) {
     super();
     const commandGroup = {
-      cancel: () => { this._close(); },
       printSection: () => { printViewSection(null, this._gristDoc.viewModel.activeSection()).catch(reportError); },
     };
     this.autoDispose(commands.createGroup(commandGroup, this, true));
@@ -41,9 +41,6 @@ export class RawData extends Disposable {
   }
 
   public buildDom() {
-    // Handler to close the lightbox.
-    const close = this._close.bind(this);
-
     return cssContainer(
       dom('div',
         dom.create(DataTables, this._gristDoc),
@@ -52,30 +49,12 @@ export class RawData extends Disposable {
         dom.hide(this._lightboxVisible)
       ),
       /***************  Lightbox section **********/
-      dom.domComputedOwned(fromKo(this._gristDoc.viewModel.activeSection), (owner, viewSection) => {
+      dom.domComputed(fromKo(this._gristDoc.viewModel.activeSection), (viewSection) => {
         const sectionId = viewSection.getRowId();
         if (!sectionId || !viewSection.isRaw.peek()) {
           return null;
         }
-        ViewSectionHelper.create(owner, this._gristDoc, viewSection);
+        return dom.create(RawDataPopup, this._gristDoc, viewSection, () => this._close());
-        return cssOverlay(
-          testId('overlay'),
-          cssSectionWrapper(
-            buildViewSectionDom({
-              gristDoc: this._gristDoc,
-              sectionRowId: viewSection.getRowId(),
-              draggable: false,
-              focusable: false,
-              widgetNameHidden: true
-            })
-          ),
-          cssCloseButton('CrossBig',
-            testId('close-button'),
-            dom.on('click', close)
-          ),
-          // Close the lightbox when user clicks exactly on the overlay.
-          dom.on('click', (ev, elem) => void (ev.target === elem ? close() : null))
-        );
       }),
     );
   }
@@ -85,6 +64,41 @@ export class RawData extends Disposable {
   }
 }
 
+export class RawDataPopup extends Disposable {
+  constructor(
+    private _gristDoc: GristDoc,
+    private _viewSection: ViewSectionRec,
+    private _onClose: () => void,
+    ) {
+    super();
+    const commandGroup = {
+      cancel: () => { this._onClose(); },
+    };
+    this.autoDispose(commands.createGroup(commandGroup, this, true));
+  }
+  public buildDom() {
+    ViewSectionHelper.create(this, this._gristDoc, this._viewSection);
+    return cssOverlay(
+      testId('overlay'),
+      cssSectionWrapper(
+        buildViewSectionDom({
+          gristDoc: this._gristDoc,
+          sectionRowId: this._viewSection.getRowId(),
+          draggable: false,
+          focusable: false,
+          widgetNameHidden: true
+        })
+      ),
+      cssCloseButton('CrossBig',
+        testId('close-button'),
+        dom.on('click', () => this._onClose())
+      ),
+      // Close the lightbox when user clicks exactly on the overlay.
+      dom.on('click', (ev, elem) => void (ev.target === elem ? this._onClose() : null))
+    );
+  }
+}
+
 const cssContainer = styled('div', `
   overflow-y: auto;
   position: relative;

app/client/components/RecordLayout.js

@@ -69,6 +69,7 @@ function RecordLayout(options) {
 
   // Update the stored layoutSpecObj with any missing fields that are present in viewFields.
   this.layoutSpec = this.autoDispose(ko.computed(function() {
+    if (this.viewSection.isDisposed()) { return null; }
     return RecordLayout.updateLayoutSpecWithFields(
       this.viewSection.layoutSpecObj(), this.viewSection.viewFields().all());
   }, this).extend({rateLimit: 0})); // layoutSpecObj and viewFields should be updated together.

app/client/models/entities/ViewSectionRec.ts

@@ -474,7 +474,7 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
   this.hasFocus = ko.pureComputed({
     // Read may occur for recently disposed sections, must check condition first.
     read: () => !this.isDisposed() && this.view().activeSectionId() === this.id(),
-    write: (val) => { if (val) { this.view().activeSectionId(this.id()); } }
+    write: (val) => { this.view().activeSectionId(val ? this.id() : 0); }
   });
 
   this.activeLinkSrcSectionRef = modelUtil.customValue(this.linkSrcSectionRef);
@@ -595,8 +595,8 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
   this.allowSelectBy = Observable.create(this, false);
   this.selectedRows = Observable.create(this, []);
 
-  this.tableId = ko.pureComputed(() => this.table().tableId());
+  this.tableId = this.autoDispose(ko.pureComputed(() => this.table().tableId()));
-  const rawSection = ko.pureComputed(() => this.table().rawViewSection());
+  const rawSection = this.autoDispose(ko.pureComputed(() => this.table().rawViewSection()));
   this.rulesCols = refListRecords(docModel.columns, ko.pureComputed(() => rawSection().rules()));
   this.rulesColsIds = ko.pureComputed(() => this.rulesCols().map(c => c.colId()));
   this.rulesStyles = modelUtil.savingComputed({

app/client/widgets/FieldBuilder.ts

@@ -436,7 +436,7 @@ export class FieldBuilder extends Disposable {
    * Builds the cell and editor DOM for the chosen UserType. Calls the buildDom and
    *  buildEditorDom functions of its widgetImpl.
    */
-  public buildDomWithCursor(row: DataRowModel, isActive: boolean, isSelected: boolean) {
+  public buildDomWithCursor(row: DataRowModel, isActive: ko.Computed<boolean>, isSelected: ko.Computed<boolean>) {
     const computedFlags = koUtil.withKoUtils(ko.pureComputed(() => {
       return this.field.rulesColsIds().map(colRef => row.cells[colRef]?.() ?? false);
     }, this).extend({ deferred: true }));

app/common/gristUrls.ts

@@ -250,7 +250,7 @@ export function encodeUrl(gristConfig: Partial<GristLoadConfig>,
   const hashParts: string[] = [];
   if (state.hash && state.hash.rowId) {
     const hash = state.hash;
-    hashParts.push(`a1`);
+    hashParts.push(state.hash?.popup ? 'a2' : `a1`);
     for (const key of ['sectionId', 'rowId', 'colRef'] as Array<keyof HashLink>) {
       if (hash[key]) { hashParts.push(`${key[0]}${hash[key]}`); }
     }
@@ -372,9 +372,9 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
     for (const part of hashParts) {
       hashMap.set(part.slice(0, 1), part.slice(1));
     }
-    if (hashMap.has('#') && hashMap.get('#') === 'a1') {
+    if (hashMap.has('#') && ['a1', 'a2'].includes(hashMap.get('#') || '')) {
       const link: HashLink = {};
-      for (const key of ['sectionId', 'rowId', 'colRef'] as Array<keyof HashLink>) {
+      for (const key of ['sectionId', 'rowId', 'colRef'] as Array<Exclude<keyof HashLink, 'popup'>>) {
         const ch = key.substr(0, 1);
         if (!hashMap.has(ch)) { continue; }
         const value = hashMap.get(ch);
@@ -384,6 +384,9 @@ export function decodeUrl(gristConfig: Partial<GristLoadConfig>, location: Locat
           link[key] = parseInt(value!, 10);
         }
       }
+      if (hashMap.get('#') === 'a2') {
+        link.popup =  true;
+      }
       state.hash = link;
     }
     state.welcomeTour = hashMap.get('#') === 'repeat-welcome-tour';
@@ -773,6 +776,7 @@ export interface HashLink {
   sectionId?: number;
   rowId?: UIRowId;
   colRef?: number;
+  popup?: boolean;
 }
 
 // Check whether a urlId is a prefix of the docId, and adequately long to be

test/nbrowser/gristUtils.ts

@@ -536,16 +536,18 @@ export async function getFormulaText() {
 /**
  * Check that formula editor is shown and its value matches the given regexp.
  */
-export async function checkFormulaEditor(valueRe: RegExp) {
+export async function checkFormulaEditor(value: RegExp|string) {
   assert.equal(await driver.findWait('.test-formula-editor', 500).isDisplayed(), true);
+  const valueRe = typeof value === 'string' ? exactMatch(value) : value;
   assert.match(await driver.find('.code_editor_container').getText(), valueRe);
 }
 
 /**
  * Check that plain text editor is shown and its value matches the given regexp.
  */
-export async function checkTextEditor(valueRe: RegExp) {
+export async function checkTextEditor(value: RegExp|string) {
   assert.equal(await driver.findWait('.test-widget-text-editor', 500).isDisplayed(), true);
+  const valueRe = typeof value === 'string' ? exactMatch(value) : value;
   assert.match(await driver.find('.celleditor_text_editor').value(), valueRe);
 }
 
@@ -2384,6 +2386,11 @@ export async function waitForAnchor() {
   await driver.wait(async () => (await getTestState()).anchorApplied, 2000);
 }
 
+export async function getAnchor() {
+  await driver.find('body').sendKeys(Key.chord(Key.SHIFT, await modKey(), 'a'));
+  return (await getTestState()).clipboard || '';
+}
+
 export async function getActiveSectionTitle(timeout?: number) {
   return await driver.findWait('.active_section .test-viewsection-title', timeout ?? 0).getText();
 }