From 08881d9663ca16a7d50b2f2e8bcf1dc911ad777b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Wed, 5 Jan 2022 21:14:44 +0100 Subject: [PATCH] (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 | 11 ++++-- app/client/components/DetailView.js | 8 ++++- app/client/components/GridView.js | 11 +++--- app/client/declarations.d.ts | 2 +- app/client/lib/koDom.js | 56 +++++++++++++++++++++-------- app/client/models/SearchModel.ts | 2 +- test/nbrowser/gristUtils.ts | 12 +++++++ 7 files changed, 78 insertions(+), 24 deletions(-) diff --git a/app/client/components/BaseView.js b/app/client/components/BaseView.js index 76c307c0..c9124697 100644 --- a/app/client/components/BaseView.js +++ b/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(); }; diff --git a/app/client/components/DetailView.js b/app/client/components/DetailView.js index 0d0f11f6..f617223d 100644 --- a/app/client/components/DetailView.js +++ b/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; diff --git a/app/client/components/GridView.js b/app/client/components/GridView.js index 2e60ad19..be72065a 100644 --- a/app/client/components/GridView.js +++ b/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. diff --git a/app/client/declarations.d.ts b/app/client/declarations.d.ts index 6aa36f9a..228a349d 100644 --- a/app/client/declarations.d.ts +++ b/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; + public scrollToCursor(sync: boolean): Promise; } export = BaseView; } diff --git a/app/client/lib/koDom.js b/app/client/lib/koDom.js index fc3c46a4..7815bb2a 100644 --- a/app/client/lib/koDom.js +++ b/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; diff --git a/app/client/models/SearchModel.ts b/app/client/models/SearchModel.ts index be23d70b..bc9cd6a2 100644 --- a/app/client/models/SearchModel.ts +++ b/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) { diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 16cd1a6a..1e07ecc1 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -1891,6 +1891,18 @@ export async function onNewTab(action: () => Promise) { 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);