From 7aa83858bd11c871664762756b3d663c1f3b9b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 4 Apr 2023 09:51:34 +0200 Subject: [PATCH] (core) Fixing bug with the import preview being not responsive Summary: Fixing the bug by setting viewInstance on the temp viewsection record Test Plan: Added Reviewers: dsagal Reviewed By: dsagal Subscribers: dsagal Differential Revision: https://phab.getgrist.com/D3848 --- app/client/components/GristDoc.ts | 21 ++++++++++++++++++++- test/nbrowser/gristUtils.ts | 5 +++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 03176482..3b344100 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -393,7 +393,16 @@ export class GristDoc extends DisposableWithEvents { })); // Importer takes a function for creating previews. - const createPreview = (vs: ViewSectionRec) => GridView.create(this, vs, true); + const createPreview = (vs: ViewSectionRec) => { + const preview = GridView.create(this, vs, true); + // We need to set the instance to the newly created section. This is important, as + // GristDoc is responsible for changing the cursor position not the cursor itself. Final + // cursor position is determined by finding active (or visible) section and passing this + // command (setCursor) to its instance. + vs.viewInstance(preview); + preview.autoDisposeCallback(() => vs.viewInstance(null)); + return preview; + }; const importSourceElems = ImportSourceElement.fromArray(this.docPluginManager.pluginsList); const importMenuItems = [ @@ -593,6 +602,16 @@ export class GristDoc extends DisposableWithEvents { public async setCursorPos(cursorPos: CursorPos) { if (cursorPos.sectionId && cursorPos.sectionId !== this.externalSectionId.get()) { const desiredSection: ViewSectionRec = this.docModel.viewSections.getRowModel(cursorPos.sectionId); + // If this is completely unknown section (without a parent), it is probably an import preview. + if (!desiredSection.parentId.peek() && !desiredSection.isRaw.peek()) { + const view = desiredSection.viewInstance.peek(); + // Make sure we have a view instance here - it will prove our assumption that this is + // an import preview. Section might also be disconnected during undo/redo. + if (view && !view.isDisposed()) { + view.setCursorPos(cursorPos); + return; + } + } if (desiredSection.view.peek().getRowId() !== this.activeViewId.get()) { // This may be asynchronous. In other cases, the change is synchronous, and some code // relies on it (doesn't wait for this function to resolve). diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 8078542a..56009f92 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -248,6 +248,7 @@ export async function expandSection(title?: string) { ? driver.findContent(`.test-viewsection-title`, exactMatch(title)).findClosest(".viewsection_title") : driver.find(".active_section"); await select.find(".test-section-menu-expandSection").click(); + await driver.findWait('.test-viewLayout-overlay .test-close-button', 500); } export async function getSectionId() { @@ -541,9 +542,9 @@ export async function rightClick(cell: WebElement) { * section. RowNum is a 1-based number as in the row headers, and col is a 0-based index for * grid view or field name for detail view. */ -export async function getCursorPosition() { +export async function getCursorPosition(section?: WebElement) { return await retryOnStale(async () => { - const section = await driver.findWait('.active_section', 4000); + section = section ?? await driver.findWait('.active_section', 4000); const cursor = await section.findWait('.active_cursor', 1000); // Query assuming the cursor is in a GridView and a DetailView, then use whichever query data // works out.