(core) Scrolling to the active record on search

Summary:
Two bugs fixed:
1. On search, when the first result is in the active record, GridView wasn't scrolling to the active record.
2. When an active record was not visible, GridView wasn't scrolling to the active record when the column index was changed.

The problem was that the scrolling behavior was based only on rowIndex which isn't changed (and doesn't notify subscribers) when a column index changes or when the search highlights a cell.
This diff makes the computed depend also on the fieldIndex, and is introducing a new method that can scroll to the active record on demand (which is used by the search).

Test Plan: Updated tests.

Reviewers: georgegevoian

Reviewed By: georgegevoian

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

app/client/components/BaseView.js

@@ -693,4 +693,12 @@ BaseView.prototype.isFiltered = function() {
   return this._filteredRowSource.getNumRows() < this.tableModel.tableData.numRecords();
 };
 
+/**
+ * Makes sure that active record is in the view.
+ */
+BaseView.prototype.revealActiveRecord = function() {
+  // to override
+  return Promise.resolve();
+};
+
 module.exports = BaseView;

app/client/components/GridView.js

@@ -27,7 +27,7 @@ const {reportError} = require('app/client/models/AppModel');
 const {onDblClickMatchElem} = require('app/client/lib/dblclick');
 
 // Grist UI Components
-const {Holder} = require('grainjs');
+const {Holder, Computed} = require('grainjs');
 const {menu} = require('../ui2018/menus');
 const {calcFieldsCondition} = require('../ui/GridViewMenus');
 const {ColumnAddMenu, ColumnContextMenu, MultiColumnMenu, freezeAction} = require('../ui/GridViewMenus');
@@ -86,6 +86,24 @@ function GridView(gristDoc, viewSectionModel, isPreview = false) {
     return tree;
   }));
 
+  // Create observable holding current rowIndex that the view should be scrolled to.
+  // We will always notify, because we want to scroll to the row even when only the
+  // column is changed (in situation when the row is not visible).
+  this.visibleRowIndex = ko.observable(this.cursor.rowIndex()).extend({notify: 'always'});
+  // Create grain's Computed with current cursor position (we need it to examine position
+  // before the change and after).
+  this.currentPosition = Computed.create(this, (use) => ({
+    rowIndex : use(this.cursor.rowIndex),
+    fieldIndex : use(this.cursor.fieldIndex)
+  }));
+  // Add listener, and check if the cursor is indeed changed, if so, update the row
+  // and scroll it into view (using kd.scrollChildIntoView in buildDom function).
+  this.autoDispose(this.currentPosition.addListener((cur, prev) => {
+    if (cur.rowIndex !== prev.rowIndex || cur.fieldIndex !== prev.fieldIndex) {
+      this.visibleRowIndex(cur.rowIndex);
+    }
+  }));
+
   this.autoDispose(this.cursor.fieldIndex.subscribe(idx => {
     const offset = this.colRightOffsets.peek().getSumTo(idx);
 
@@ -745,7 +763,7 @@ GridView.prototype._getColStyle = function(colIndex) {
 };
 
 
-// TODO: for now lets just assume youre clicking on a .field, .row, or .column
+// TODO: for now lets just assume you are clicking on a .field, .row, or .column
 GridView.prototype.domToRowModel = function(elem, elemType) {
   switch (elemType) {
     case selector.COL:
@@ -859,7 +877,7 @@ GridView.prototype.buildDom = function() {
 
     self.scrollPane =
     dom('div.grid_view_data.gridview_data_scroll.show_scrollbar',
-      kd.scrollChildIntoView(self.cursor.rowIndex),
+      kd.scrollChildIntoView(self.visibleRowIndex),
       dom.onDispose(() => {
         // Save the previous scroll values to the section.
         self.viewSection.lastScrollPos = _.extend({
@@ -1410,6 +1428,10 @@ GridView.prototype.maybeSelectRow = function(elem, rowId) {
   }
 };
 
+GridView.prototype.revealActiveRecord = function() {
+  return kd.doScrollChildIntoView(this.scrollPane, this.cursor.rowIndex());
+}
+
 // End Context Menus
 
 module.exports = GridView;

app/client/declarations.d.ts

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

app/client/lib/koDom.js

@@ -276,34 +276,47 @@ exports.cssClass = cssClass;
  *    whose value is the index of the child element to keep scrolled into view.
  */
 function scrollChildIntoView(valueOrFunc) {
-  return makeBinding(valueOrFunc, function(elem, index) {
-    if (index === null) {
-      return;
-    }
-    var scrolly = ko.utils.domData.get(elem, "scrolly");
-    if (scrolly) {
-      // Delay this in case it's triggered while other changes are processed (e.g. splices).
-      setTimeout(() => scrolly.isDisposed() || scrolly.scrollRowIntoView(index), 0);
-    } else {
-      var child = elem.children[index];
-      if (!child) {
-        return;
-      }
+  return makeBinding(valueOrFunc, doScrollChildIntoView);
+}
+function doScrollChildIntoView(elem, index) {
+  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);
+          }
+          resolve();
+        } catch(err) {
+          reject(err);
+        }
+      }, 0);
+    });
+  } else {
+    const child = elem.children[index];
+    if (child) {
       if (index === 0) {
         // Scroll the container all the way if showing the first child.
         elem.scrollTop = 0;
       }
-      var childRect = child.getBoundingClientRect();
-      var parentRect = elem.getBoundingClientRect();
+      const childRect = child.getBoundingClientRect();
+      const parentRect = elem.getBoundingClientRect();
       if (childRect.top < parentRect.top) {
         child.scrollIntoView(true);             // Align with top if scrolling up..
       } else if (childRect.bottom > parentRect.bottom) {
         child.scrollIntoView(false);              // ..bottom if scrolling down.
       }
     }
-  });
+    return Promise.resolve();
+  }
 }
 exports.scrollChildIntoView = scrollChildIntoView;
+exports.doScrollChildIntoView = doScrollChildIntoView;
 
 
 /**

app/client/models/SearchModel.ts

@@ -309,9 +309,12 @@ class FinderImpl implements IFinder {
     // this ad-hoc way rather than use observables, to avoid the overhead of *every* cell
     // depending on an additional observable.
     await delay(0);
-    const viewInstance = await waitObs<any>(section.viewInstance);
+    const viewInstance = (await waitObs(section.viewInstance))!;
     await viewInstance.getLoadingDonePromise();
     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();
 
     const cursor = viewInstance.viewPane.querySelector('.selected_cursor');
     if (cursor) {