From a5099fc5987edbe0656843a8e407ad40ad403e5b Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Mon, 4 Mar 2024 17:48:40 -0500 Subject: [PATCH] (core) Skip showing empty doc tours Summary: Empty doc tours are now skipped. Before, an error was shown instead. Test Plan: Browser test. Reviewers: jarek Reviewed By: jarek Differential Revision: https://phab.getgrist.com/D4204 --- app/client/components/GristDoc.ts | 23 +++++++++++++++-------- test/nbrowser/DuplicateDocument.ts | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index 5f6893fa..34e1da0c 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -358,9 +358,9 @@ export class GristDoc extends DisposableWithEvents { urlState().state, isTourActiveObs(), fromKo(this.docModel.isTutorial), - (_use, state, hasActiveTour, isTutorial) => { + async (_use, state, hasActiveTour, isTutorial) => { // Tours and tutorials can interfere with in-product tips and announcements. - const hasPendingDocTour = state.docTour || this._shouldAutoStartDocTour(); + const hasPendingDocTour = state.docTour || await this._shouldAutoStartDocTour(); const hasPendingWelcomeTour = state.welcomeTour || this._shouldAutoStartWelcomeTour(); const isPopupManagerDisabled = this.behavioralPromptsManager.isDisabled(); if ( @@ -401,7 +401,7 @@ export class GristDoc extends DisposableWithEvents { } const shouldStartTutorial = isTutorial; - const shouldStartDocTour = state.docTour || this._shouldAutoStartDocTour(); + const shouldStartDocTour = state.docTour || await this._shouldAutoStartDocTour(); const shouldStartWelcomeTour = state.welcomeTour || this._shouldAutoStartWelcomeTour(); if (shouldStartTutorial || shouldStartDocTour || shouldStartWelcomeTour) { isStartingTourOrTutorial = true; @@ -1823,15 +1823,22 @@ export class GristDoc extends DisposableWithEvents { /** * Returns whether a doc tour should automatically be started. * - * Currently, tours are started if a GristDocTour table exists and the user hasn't - * seen the tour before. + * Currently, tours are started if a non-empty GristDocTour table exists and the + * user hasn't seen the tour before. */ - private _shouldAutoStartDocTour(): boolean { - if (this._disableAutoStartingTours || this.docModel.isTutorial()) { + private async _shouldAutoStartDocTour(): Promise { + if ( + this._disableAutoStartingTours || + this.docModel.isTutorial() || + !this.docModel.hasDocTour() || + this._seenDocTours.get()?.includes(this.docId()) + ) { return false; } - return this.docModel.hasDocTour() && !this._seenDocTours.get()?.includes(this.docId()); + const tableData = this.docData.getTable('GristDocTour')!; + await this.docData.fetchTable('GristDocTour'); + return tableData.numRecords() > 0; } /** diff --git a/test/nbrowser/DuplicateDocument.ts b/test/nbrowser/DuplicateDocument.ts index 84e277c7..437d23c8 100644 --- a/test/nbrowser/DuplicateDocument.ts +++ b/test/nbrowser/DuplicateDocument.ts @@ -218,4 +218,23 @@ describe("DuplicateDocument", function() { await driver.find(".test-bc-workspace").click(); await gu.removeDoc(`DuplicateTest2 ${name} Copy`); }); + + it("should not auto-start tour if a document with a tour is copied as a template", async function() { + const session = await gu.session().teamSite.login(); + await session.tempDoc(cleanup, 'doctour.grist'); + await session.tempWorkspace(cleanup, 'Test Workspace'); + assert.isTrue(await driver.findWait('.test-onboarding-popup', 1000).isPresent()); + await driver.find('.test-onboarding-close').click(); + await gu.waitForServer(); + + await driver.find('.test-tb-share').click(); + await driver.find('.test-save-copy').click(); + await driver.findWait('.test-modal-dialog', 1000); + await driver.find('.test-save-as-template').click(); + await gu.completeCopy({destName: 'DuplicateTest3', destWorkspace: 'Test Workspace'}); + + // Give it a second, just to be sure the tour doesn't appear. + await driver.sleep(1000); + assert.isFalse(await driver.find('.test-onboarding-popup').isPresent()); + }); });