(core) Fix calendar and card tip bug on mobile

Summary:
On mobile, tips for calendar and card list aren't currently
shown, but the creator panel was still automatically being
opened in preparation for showing the tip.

Test Plan: Manual and existing tests.

Reviewers: jarek

Reviewed By: jarek

Subscribers: jarek

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

app/client/components/BehavioralPromptsManager.ts

@@ -8,26 +8,32 @@ import {getGristConfig} from 'app/common/urlUtils';
 import {Computed, Disposable, dom, Observable} from 'grainjs';
 import {IPopupOptions} from 'popweasel';
 
-export interface AttachOptions {
-  /** Defaults to false. */
-  forceShow?: boolean;
-  /** Defaults to false. */
+/**
+ * Options for showing a tip.
+ */
+export interface ShowTipOptions {
+  /** Defaults to `false`. */
   hideArrow?: boolean;
-  /** Defaults to false. */
-  hideDontShowTips?: boolean;
-  /** Defaults to true. */
-  markAsSeen?: boolean;
-  /** Defaults to false. */
-  showOnMobile?: boolean;
   popupOptions?: IPopupOptions;
   onDispose?(): void;
-  shouldShow?(): boolean;
+}
+
+/**
+ * Options for attaching a tip to a DOM element.
+ */
+export interface AttachTipOptions extends ShowTipOptions {
+  /**
+   * Optional callback that should return true if the tip should be disabled.
+   *
+   * If omitted, the tip is enabled.
+   */
+  isDisabled?(): boolean;
 }
 
 interface QueuedTip {
   prompt: BehavioralPrompt;
   refElement: Element;
-  options: AttachOptions;
+  options: ShowTipOptions;
 }
 
 /**
@@ -52,12 +58,14 @@ export class BehavioralPromptsManager extends Disposable {
     super();
   }
 
-  public showTip(refElement: Element, prompt: BehavioralPrompt, options: AttachOptions = {}) {
+  public showTip(refElement: Element, prompt: BehavioralPrompt, options: ShowTipOptions = {}) {
     this._queueTip(refElement, prompt, options);
   }
 
-  public attachTip(prompt: BehavioralPrompt, options: AttachOptions = {}) {
+  public attachTip(prompt: BehavioralPrompt, options: AttachTipOptions = {}) {
     return (element: Element) => {
+      if (options.isDisabled?.()) { return; }
+
       this._queueTip(element, prompt, options);
     };
   }
@@ -70,6 +78,29 @@ export class BehavioralPromptsManager extends Disposable {
     return !this._prefs.get().dontShowTips;
   }
 
+  public shouldShowTip(prompt: BehavioralPrompt): boolean {
+    if (this._isDisabled) { return false; }
+
+    const {
+      showContext = 'desktop',
+      showDeploymentTypes,
+      forceShow = false,
+    } = GristBehavioralPrompts[prompt];
+
+    const {deploymentType} = getGristConfig();
+    if (
+      showDeploymentTypes !== '*' &&
+      (!deploymentType || !showDeploymentTypes.includes(deploymentType))
+    ) {
+      return false;
+    }
+
+    const context = isNarrowScreen() ? 'mobile' : 'desktop';
+    if (showContext !== '*' && showContext !== context) { return false; }
+
+    return forceShow || (!this._prefs.get().dontShowTips && !this.hasSeenTip(prompt));
+  }
+
   public enable() {
     this._isDisabled = false;
   }
@@ -83,8 +114,8 @@ export class BehavioralPromptsManager extends Disposable {
     this.enable();
   }
 
-  private _queueTip(refElement: Element, prompt: BehavioralPrompt, options: AttachOptions) {
-    if (!this._shouldQueueTip(prompt, options)) { return; }
+  private _queueTip(refElement: Element, prompt: BehavioralPrompt, options: ShowTipOptions) {
+    if (!this.shouldShowTip(prompt)) { return; }
 
     this._queuedTips.push({prompt, refElement, options});
     if (this._queuedTips.length > 1) {
@@ -96,15 +127,15 @@ export class BehavioralPromptsManager extends Disposable {
     this._showTip(refElement, prompt, options);
   }
 
-  private _showTip(refElement: Element, prompt: BehavioralPrompt, options: AttachOptions) {
+  private _showTip(refElement: Element, prompt: BehavioralPrompt, options: ShowTipOptions) {
     const close = () => {
       if (!ctl.isDisposed()) {
         ctl.close();
       }
     };
 
-    const {hideArrow, hideDontShowTips, markAsSeen = true, onDispose, popupOptions} = options;
-    const {title, content} = GristBehavioralPrompts[prompt];
+    const {hideArrow, onDispose, popupOptions} = options;
+    const {title, content, hideDontShowTips = false, markAsSeen = true} = GristBehavioralPrompts[prompt];
     const ctl = showBehavioralPrompt(refElement, title(), content(), {
       onClose: (dontShowTips) => {
         if (dontShowTips) { this._dontShowTips(); }
@@ -143,27 +174,4 @@ export class BehavioralPromptsManager extends Disposable {
     this._prefs.set({...this._prefs.get(), dontShowTips: true});
     this._queuedTips = [];
   }
-
-  private _shouldQueueTip(prompt: BehavioralPrompt, options: AttachOptions) {
-    if (
-      this._isDisabled ||
-      options.shouldShow?.() === false ||
-      (isNarrowScreen() && !options.showOnMobile) ||
-      (this._prefs.get().dontShowTips && !options.forceShow) ||
-      this.hasSeenTip(prompt)
-    ) {
-      return false;
-    }
-
-    const {deploymentType} = getGristConfig();
-    const {deploymentTypes} = GristBehavioralPrompts[prompt];
-    if (
-      deploymentTypes !== '*' &&
-      (!deploymentType || !deploymentTypes.includes(deploymentType))
-    ) {
-      return false;
-    }
-
-    return true;
-  }
 }

app/client/components/GristDoc.ts

@@ -336,10 +336,6 @@ export class GristDoc extends DisposableWithEvents {
               }
 
               this.behavioralPromptsManager.showTip(cursor, 'rickRow', {
-                forceShow: true,
-                hideDontShowTips: true,
-                markAsSeen: false,
-                showOnMobile: true,
                 onDispose: () => this.playRickRollVideo(),
               });
             })
@@ -1422,8 +1418,8 @@ export class GristDoc extends DisposableWithEvents {
     if (
       // Don't show the tip if a non-card widget was selected.
       !['single', 'detail'].includes(selectedWidgetType) ||
-      // Or if we've already seen it.
-      this.behavioralPromptsManager.hasSeenTip('editCardLayout')
+      // Or if we shouldn't see the tip.
+      !this.behavioralPromptsManager.shouldShowTip('editCardLayout')
     ) {
       return;
     }
@@ -1449,11 +1445,13 @@ export class GristDoc extends DisposableWithEvents {
   private async _handleNewAttachedCustomWidget(widget: IAttachedCustomWidget) {
     switch (widget) {
       case 'custom.calendar': {
+        if (this.behavioralPromptsManager.shouldShowTip('calendarConfig')) {
           // Open the right panel to the calendar subtab.
           commands.allCommands.viewTabOpen.run();
 
           // Wait for the right panel to finish animation if it was collapsed before.
           await commands.allCommands.rightPanelOpen.run();
+        }
         break;
       }
     }

app/client/ui/CustomSectionConfig.ts

@@ -481,9 +481,9 @@ export class CustomSectionConfig extends Disposable {
             popupOptions: {
               placement: 'left-start',
             },
-            shouldShow: () => {
-              // Only show tip if a custom widget isn't already selected.
-              return !this._selectedId.get() || (isCustom.get() && this._url.get().trim() === '');
+            isDisabled: () => {
+              // Disable tip if a custom widget is already selected.
+              return Boolean(this._selectedId.get() && !(isCustom.get() && this._url.get().trim() === ''));
             },
           })
         ),

app/client/ui/GristTooltips.ts

@@ -103,7 +103,15 @@ see or edit which parts of your document.')
 export interface BehavioralPromptContent {
   title: () => string;
   content: (...domArgs: DomElementArg[]) => DomContents;
-  deploymentTypes: GristDeploymentType[] | '*';
+  showDeploymentTypes: GristDeploymentType[] | '*';
+  /** Defaults to `desktop`. */
+  showContext?: 'mobile' | 'desktop' | '*';
+  /** Defaults to `false`. */
+  hideDontShowTips?: boolean;
+  /** Defaults to `false`. */
+  forceShow?: boolean;
+  /** Defaults to `true`. */
+  markAsSeen?: boolean;
 }
 
 export const GristBehavioralPrompts: Record<BehavioralPrompt, BehavioralPromptContent> = {
@@ -119,7 +127,7 @@ export const GristBehavioralPrompts: Record<BehavioralPrompt, BehavioralPromptCo
       ),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   referenceColumnsConfig: {
     title: () => t('Reference Columns'),
@@ -134,7 +142,7 @@ record in that table, but you may select which column from that record to show.'
       ),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   rawDataPage: {
     title: () => t('Raw Data page'),
@@ -144,7 +152,7 @@ including summary tables and tables not included in page layouts.')),
       dom('div', cssLink({href: commonUrls.helpRawData, target: '_blank'}, t('Learn more.'))),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   accessRules: {
     title: () => t('Access Rules'),
@@ -154,7 +162,7 @@ to determine who can see or edit which parts of your document.')),
       dom('div', cssLink({href: commonUrls.helpAccessRules, target: '_blank'}, t('Learn more.'))),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   filterButtons: {
     title: () => t('Pinning Filters'),
@@ -164,7 +172,7 @@ to determine who can see or edit which parts of your document.')),
       dom('div', cssLink({href: commonUrls.helpFilterButtons, target: '_blank'}, t('Learn more.'))),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   nestedFiltering: {
     title: () => t('Nested Filtering'),
@@ -173,7 +181,7 @@ to determine who can see or edit which parts of your document.')),
       dom('div', t('Only those rows will appear which match all of the filters.')),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   pageWidgetPicker: {
     title: () => t('Selecting Data'),
@@ -182,7 +190,7 @@ to determine who can see or edit which parts of your document.')),
       dom('div', t('Use the 𝚺 icon to create summary (or pivot) tables, for totals or subtotals.')),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   pageWidgetPickerSelectBy: {
     title: () => t('Linking Widgets'),
@@ -192,7 +200,7 @@ to determine who can see or edit which parts of your document.')),
       dom('div', cssLink({href: commonUrls.helpLinkingWidgets, target: '_blank'}, t('Learn more.'))),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   editCardLayout: {
     title: () => t('Editing Card Layout'),
@@ -203,7 +211,7 @@ to determine who can see or edit which parts of your document.')),
       })),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   addNew: {
     title: () => t('Add New'),
@@ -211,7 +219,7 @@ to determine who can see or edit which parts of your document.')),
       dom('div', t('Click the Add New button to create new documents or workspaces, or import data.')),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   rickRow: {
     title: () => t('Anchor Links'),
@@ -225,7 +233,11 @@ to determine who can see or edit which parts of your document.')),
       ),
       ...args,
     ),
-    deploymentTypes: '*',
+    showDeploymentTypes: '*',
+    showContext: '*',
+    hideDontShowTips: true,
+    forceShow: true,
+    markAsSeen: false,
   },
   customURL: {
     title: () => t('Custom Widgets'),
@@ -238,7 +250,7 @@ to determine who can see or edit which parts of your document.')),
       dom('div', cssLink({href: commonUrls.helpCustomWidgets, target: '_blank'}, t('Learn more.'))),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
   calendarConfig: {
     title: () => t('Calendar'),
@@ -250,6 +262,6 @@ data.")),
       dom('div', cssLink({href: commonUrls.helpCalendarWidget, target: '_blank'}, t('Learn more.'))),
       ...args,
     ),
-    deploymentTypes: ['saas'],
+    showDeploymentTypes: ['saas'],
   },
 };

test/nbrowser/AttachedCustomWidget.ts

@@ -84,7 +84,7 @@ describe('AttachedCustomWidget', function () {
   it('should not ask for permission', async () => {
     await gu.addNewSection(/Calendar/, /Table1/, {selectBy: /TABLE1/});
     await gu.getSection('TABLE1 Calendar').click();
-    await gu.waitForSidePanel();
+    await gu.toggleSidePanel('right', 'open');
     await driver.find('.test-right-tab-pagewidget').click();
 
     await gu.waitForServer();