(core) Scrolling to the active record when editor is activated

Summary: When an editor is activated by typing, the active view should be scrolled to the active record.

Test Plan: new tests

Reviewers: georgegevoian

Reviewed By: georgegevoian

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

app/client/components/BaseView.js

@@ -246,8 +246,11 @@ _.extend(Base.prototype, BackboneEvents);
  * These commands are common to GridView and DetailView.
  */
 BaseView.commonCommands = {
-  input: function(input) { this.activateEditorAtCursor({init: input}); },
-  editField: function() { this.activateEditorAtCursor(); },
+  input: function(init) {
+    this.scrollToCursor(true).catch(reportError);
+    this.activateEditorAtCursor({init});
+  },
+  editField: function() { this.scrollToCursor(true); this.activateEditorAtCursor(); },
 
   insertRecordBefore: function() { this.insertRow(this.cursor.rowIndex()); },
   insertRecordAfter: function() { this.insertRow(this.cursor.rowIndex() + 1); },
@@ -695,8 +698,10 @@ BaseView.prototype.isFiltered = function() {
 
 /**
  * Makes sure that active record is in the view.
+ * @param {Boolean} sync If the scroll should be performed synchronously. For typing we should scroll synchronously,
+ * for other cases asynchronously as there might be some other operations pending (see doScrollChildIntoView in koDom).
  */
-BaseView.prototype.revealActiveRecord = function() {
+BaseView.prototype.scrollToCursor = function() {
   // to override
   return Promise.resolve();
 };

app/client/components/DetailView.js

@@ -68,6 +68,7 @@ function DetailView(gristDoc, viewSectionModel) {
 
   //--------------------------------------------------
   // Construct DOM
+  this.scrollPane = null;
   this.viewPane = this.autoDispose(this.buildDom());
 
   //--------------------------------------------------
@@ -286,7 +287,7 @@ DetailView.prototype.buildDom = function() {
     }),
     kd.maybe(() => !this.recordLayout.isEditingLayout(), () => {
       if (!this._isSingle) {
-        return dom('div.detailview_scroll_pane.flexitem',
+        return this.scrollPane = dom('div.detailview_scroll_pane.flexitem',
           kd.scrollChildIntoView(this.cursor.rowIndex),
           dom.onDispose(() => {
             // Save the previous scroll values to the section.
@@ -414,4 +415,9 @@ DetailView.prototype._isAddRow = function(index = this.cursor.rowIndex()) {
   return this.viewData.getRowId(index) === 'new';
 };
 
+DetailView.prototype.scrollToCursor = function(sync = true) {
+  if (!this.scrollPane) { return Promise.resolve(); }
+  return kd.doScrollChildIntoView(this.scrollPane, this.cursor.rowIndex(), sync);
+}
+
 module.exports = DetailView;

app/client/components/GridView.js

@@ -267,7 +267,7 @@ GridView.gridCommands = {
 
   fieldEditSave: function() { this.cursor.rowIndex(this.cursor.rowIndex() + 1); },
   // Re-define editField after fieldEditSave to make it take precedence for the Enter key.
-  editField: function() { this.activateEditorAtCursor(); },
+  editField: function() { this.scrollToCursor(true); this.activateEditorAtCursor(); },
 
   deleteRecords: function() {
     const saved = this.cursor.getCursorPos();
@@ -292,7 +292,10 @@ GridView.gridCommands = {
   convertFormulasToData: function() { this._convertFormulasToData(this.getSelection()); },
   copy: function() { return this.copy(this.getSelection()); },
   cut: function() { return this.cut(this.getSelection()); },
-  paste: function(pasteObj, cutCallback) { return this.paste(pasteObj, cutCallback); },
+  paste: async function(pasteObj, cutCallback) {
+    await this.paste(pasteObj, cutCallback);
+    await this.scrollToCursor(false);
+  },
   cancel: function() { this.clearSelection(); },
   sortAsc: function() {
     sortBy(this.viewSection.activeSortSpec, this.currentColumn().getRowId(), Sort.ASC);
@@ -1478,8 +1481,8 @@ GridView.prototype.maybeSelectRow = function(elem, rowId) {
 
 // End Context Menus
 
-GridView.prototype.revealActiveRecord = function() {
-  return kd.doScrollChildIntoView(this.scrollPane, this.cursor.rowIndex());
+GridView.prototype.scrollToCursor = function(sync = true) {
+  return kd.doScrollChildIntoView(this.scrollPane, this.cursor.rowIndex(), sync);
 }
 
 // Helper to show tooltip over column selection in the full edit mode.

app/client/declarations.d.ts

@@ -72,7 +72,7 @@ declare module "app/client/components/BaseView" {
     public onResize(): void;
     public prepareToPrint(onOff: boolean): void;
     public moveEditRowToCursor(): DataRowModel;
-    public revealActiveRecord(): Promise<void>;
+    public scrollToCursor(sync: boolean): Promise<void>;
   }
   export = BaseView;
 }

app/client/lib/koDom.js

@@ -278,25 +278,53 @@ exports.cssClass = cssClass;
 function scrollChildIntoView(valueOrFunc) {
   return makeBinding(valueOrFunc, doScrollChildIntoView);
 }
-function doScrollChildIntoView(elem, index) {
+// Key at which we will store the index to scroll for async scrolling.
+const indexKey = Symbol();
+function doScrollChildIntoView(elem, index, sync) {
   if (index === null) {
     return Promise.resolve();
   }
   const scrolly = ko.utils.domData.get(elem, "scrolly");
   if (scrolly) {
-    // Delay this in case it's triggered while other changes are processed (e.g. splices).
-    return new Promise((resolve, reject) => {
-      setTimeout(() => {
-        try {
-          if (!scrolly.isDisposed()) {
-            scrolly.scrollRowIntoView(index);
+    if (sync) {
+      scrolly.scrollRowIntoView(index);
+      // Clear async index for scrolling.
+      elem[indexKey] = null;
+      return Promise.resolve();
+    } else {
+      // Delay this in case it's triggered while other changes are processed (e.g. splices).
+
+      // Scrolling is asynchronous, so in case there is already
+      // active scroll queued, we will change the target index.
+      // For example:
+      // doScrollChildIntoView(el, 10, false) # sets the index to 10 and queues a Promise1
+      // doScrollChildIntoView(el, 20, false) # updates index to 20 and queues a Promise2
+      // ....
+      // Promise1 moves to 20, and clears the index.
+      // Promise2 checks the index is null and just returns.
+      elem[indexKey] = index;
+      return new Promise((resolve, reject) => {
+        setTimeout(() => {
+          try {
+            // If scroll was cancelled (there was another call after, that finished
+            // and cleared the index) return.
+            if (elem[indexKey] === null) {
+              resolve();
+              return;
+            }
+            if (!scrolly.isDisposed()) {
+              scrolly.scrollRowIntoView(elem[indexKey]);
+            }
+            resolve();
+          } catch(err) {
+            reject(err);
+          } finally {
+            // Clear the index, any subsequent async scrolls will be cancelled (on the if test above).
+            elem[indexKey] = null;
           }
-          resolve();
-        } catch(err) {
-          reject(err);
-        }
-      }, 0);
-    });
+        }, 0);
+      });
+    }
   } else {
     const child = elem.children[index];
     if (child) {
@@ -312,8 +340,8 @@ function doScrollChildIntoView(elem, index) {
         child.scrollIntoView(false);              // ..bottom if scrolling down.
       }
     }
-    return Promise.resolve();
   }
+  return Promise.resolve();
 }
 exports.scrollChildIntoView = scrollChildIntoView;
 exports.doScrollChildIntoView = doScrollChildIntoView;

app/client/models/SearchModel.ts

@@ -314,7 +314,7 @@ class FinderImpl implements IFinder {
     if (this._aborted) { return; }
     // Make sure we are at good place. This is important when the cursor
     // was already in a matched record, but the record was scrolled away.
-    await viewInstance.revealActiveRecord();
+    viewInstance.scrollToCursor(true).catch(reportError);
 
     const cursor = viewInstance.viewPane.querySelector('.selected_cursor');
     if (cursor) {

test/nbrowser/gristUtils.ts

@@ -1891,6 +1891,18 @@ export async function onNewTab(action: () => Promise<void>) {
   await driver.switchTo().window(tabs[tabs.length - 2]);
 }
 
+/**
+ * Scrolls active Grid or Card list view.
+ */
+export async function scrollActiveView(x: number, y: number) {
+  await driver.executeScript(function(x1: number, y1: number) {
+    const view = document.querySelector(".active_section .grid_view_data") ||
+                 document.querySelector(".active_section .detailview_scroll_pane");
+    view!.scrollBy(x1, y1);
+  }, x, y);
+  await driver.sleep(10); // wait a bit for the scroll to happen (this is async operation in Grist).
+}
+
 } // end of namespace gristUtils
 
 stackWrapOwnMethods(gristUtils);