From 524e505353930f317f1dfd52dcf34110e247df83 Mon Sep 17 00:00:00 2001
From: Johannes Millan <johannes.millan@gmail.com>
Date: Wed, 3 Jun 2026 19:04:36 +0200
Subject: [PATCH] fix(onboarding): harden sync setup dismissal

Mark sync-based onboarding dismissal as fully complete so the hint tour does not restart on the next boot. Guard the sync setup CTA while the dialog is loading or open, cover the async races in tests, and move the onboarding z-index to a design token.
---
 docs/styling-guide.md                         | 23 +++++-----
 ...onboarding-preset-selection.component.html |  2 +-
 ...onboarding-preset-selection.component.scss |  2 +-
 ...oarding-preset-selection.component.spec.ts | 42 +++++++++++++++++--
 .../onboarding-preset-selection.component.ts  | 26 ++++++++++--
 src/assets/i18n/en.json                       |  2 +-
 src/styles/_css-variables.scss                |  1 +
 7 files changed, 77 insertions(+), 21 deletions(-)

diff --git a/docs/styling-guide.md b/docs/styling-guide.md
index fa4972b1ae..4586bd0c55 100644
--- a/docs/styling-guide.md
+++ b/docs/styling-guide.md
@@ -120,17 +120,18 @@ Quickest adoption — add the `.focus-ring` utility class from `util.scss`, whic
 
 ## Z-Index Layers
 
-| Variable                | Value | Purpose                  |
-| ----------------------- | ----- | ------------------------ |
-| `--z-check-done`        | 11    | Task done checkbox       |
-| `--z-main-header`       | 12    | Main header              |
-| `--z-task-title-focus`  | 32    | Focused task title       |
-| `--z-mobile-bottom-nav` | 50    | Mobile bottom navigation |
-| `--z-side-nav`          | 60    | Side navigation          |
-| `--z-backdrop`          | 222   | Backdrop overlay         |
-| `--z-add-task-bar`      | 999   | Add task bar             |
-| `--z-search-bar`        | 999   | Search bar               |
-| `--z-tour`              | 1001  | Tour overlay             |
+| Variable                 | Value | Purpose                  |
+| ------------------------ | ----- | ------------------------ |
+| `--z-check-done`         | 11    | Task done checkbox       |
+| `--z-main-header`        | 12    | Main header              |
+| `--z-task-title-focus`   | 32    | Focused task title       |
+| `--z-mobile-bottom-nav`  | 50    | Mobile bottom navigation |
+| `--z-side-nav`           | 60    | Side navigation          |
+| `--z-backdrop`           | 222   | Backdrop overlay         |
+| `--z-add-task-bar`       | 999   | Add task bar             |
+| `--z-search-bar`         | 999   | Search bar               |
+| `--z-onboarding-presets` | 999   | Onboarding preset screen |
+| `--z-tour`               | 1001  | Tour overlay             |
 
 ## Layout Variables
 
diff --git a/src/app/features/onboarding/onboarding-preset-selection.component.html b/src/app/features/onboarding/onboarding-preset-selection.component.html
index b2c76775ef..af31617427 100644
--- a/src/app/features/onboarding/onboarding-preset-selection.component.html
+++ b/src/app/features/onboarding/onboarding-preset-selection.component.html
@@ -33,7 +33,7 @@
       <button
         type="button"
         class="sync-cta-link"
-        [disabled]="!!selectedPreset()"
+        [disabled]="!!selectedPreset() || isSyncSetupInProgress()"
         (click)="setupSync()"
       >
         {{ 'ONBOARDING.SYNC_CTA_LINK' | translate }}
diff --git a/src/app/features/onboarding/onboarding-preset-selection.component.scss b/src/app/features/onboarding/onboarding-preset-selection.component.scss
index ea38dccd2e..cbd16c8cbe 100644
--- a/src/app/features/onboarding/onboarding-preset-selection.component.scss
+++ b/src/app/features/onboarding/onboarding-preset-selection.component.scss
@@ -4,7 +4,7 @@
   // 999, not 1000: stay just below the CDK overlay container (z-index 1000) so
   // dialogs opened from here (e.g. the sync setup dialog) render above this
   // opaque backdrop rather than relying on a same-z-index DOM-order tiebreak
-  z-index: 999;
+  z-index: var(--z-onboarding-presets);
 }
 
 .onboarding-backdrop {
diff --git a/src/app/features/onboarding/onboarding-preset-selection.component.spec.ts b/src/app/features/onboarding/onboarding-preset-selection.component.spec.ts
index 3968abfb9a..191ab9e7da 100644
--- a/src/app/features/onboarding/onboarding-preset-selection.component.spec.ts
+++ b/src/app/features/onboarding/onboarding-preset-selection.component.spec.ts
@@ -5,7 +5,7 @@ import {
   signal,
   WritableSignal,
 } from '@angular/core';
-import { of } from 'rxjs';
+import { Subject } from 'rxjs';
 
 import { OnboardingPresetSelectionComponent } from './onboarding-preset-selection.component';
 import { ONBOARDING_PRESETS } from './onboarding-presets.const';
@@ -17,13 +17,15 @@ describe('OnboardingPresetSelectionComponent', () => {
   let component: OnboardingPresetSelectionComponent;
   let mockDialog: jasmine.SpyObj<MatDialog>;
   let cfgSignal: WritableSignal<{ sync: { isEnabled: boolean } }>;
+  let afterClosed$: Subject<void>;
 
   const setup = (): void => {
     cfgSignal = signal({ sync: { isEnabled: false } });
+    afterClosed$ = new Subject<void>();
 
     mockDialog = jasmine.createSpyObj('MatDialog', ['open']);
     mockDialog.open.and.returnValue({
-      afterClosed: () => of(undefined),
+      afterClosed: () => afterClosed$.asObservable(),
     } as unknown as MatDialogRef<unknown>);
 
     const mockGlobalConfig = jasmine.createSpyObj('GlobalConfigService', [], {
@@ -44,11 +46,13 @@ describe('OnboardingPresetSelectionComponent', () => {
 
   beforeEach(() => {
     localStorage.removeItem(LS.ONBOARDING_PRESET_DONE);
+    localStorage.removeItem(LS.ONBOARDING_HINTS_DONE);
     setup();
   });
 
   afterEach(() => {
     localStorage.removeItem(LS.ONBOARDING_PRESET_DONE);
+    localStorage.removeItem(LS.ONBOARDING_HINTS_DONE);
   });
 
   describe('setupSync', () => {
@@ -64,25 +68,55 @@ describe('OnboardingPresetSelectionComponent', () => {
     });
 
     it('dismisses onboarding when sync was enabled in the dialog', async () => {
-      cfgSignal.set({ sync: { isEnabled: true } });
       let dismissedCount = 0;
       component.dismissed.subscribe(() => dismissedCount++);
 
       await component.setupSync();
+      expect(dismissedCount).toBe(0);
+      expect(localStorage.getItem(LS.ONBOARDING_PRESET_DONE)).toBeNull();
+
+      cfgSignal.set({ sync: { isEnabled: true } });
+      afterClosed$.next();
 
       expect(dismissedCount).toBe(1);
       expect(localStorage.getItem(LS.ONBOARDING_PRESET_DONE)).toBe('true');
+      expect(localStorage.getItem(LS.ONBOARDING_HINTS_DONE)).toBe('true');
     });
 
     it('keeps onboarding open when sync was not enabled (dialog cancelled)', async () => {
-      cfgSignal.set({ sync: { isEnabled: false } });
       let dismissedCount = 0;
       component.dismissed.subscribe(() => dismissedCount++);
 
       await component.setupSync();
+      afterClosed$.next();
 
       expect(dismissedCount).toBe(0);
       expect(localStorage.getItem(LS.ONBOARDING_PRESET_DONE)).toBeNull();
+      expect(localStorage.getItem(LS.ONBOARDING_HINTS_DONE)).toBeNull();
+    });
+
+    it('does not open duplicate dialogs while sync setup is already active', async () => {
+      const firstSetup = component.setupSync();
+      const secondSetup = component.setupSync();
+
+      await Promise.all([firstSetup, secondSetup]);
+
+      expect(mockDialog.open).toHaveBeenCalledTimes(1);
+
+      afterClosed$.next();
+      await component.setupSync();
+
+      expect(mockDialog.open).toHaveBeenCalledTimes(2);
+    });
+
+    it('does not open a stale dialog if a preset is selected while sync setup is loading', async () => {
+      const setupPromise = component.setupSync();
+      component.selectedPreset.set(ONBOARDING_PRESETS[0]);
+
+      await setupPromise;
+
+      expect(mockDialog.open).not.toHaveBeenCalled();
+      expect(component.isSyncSetupInProgress()).toBeFalse();
     });
   });
 });
diff --git a/src/app/features/onboarding/onboarding-preset-selection.component.ts b/src/app/features/onboarding/onboarding-preset-selection.component.ts
index 694ceda84b..cbab1a3d4a 100644
--- a/src/app/features/onboarding/onboarding-preset-selection.component.ts
+++ b/src/app/features/onboarding/onboarding-preset-selection.component.ts
@@ -12,6 +12,9 @@ import { ONBOARDING_PRESETS, OnboardingPreset } from './onboarding-presets.const
 import { GlobalConfigService } from '../config/global-config.service';
 import { LS } from '../../core/persistence/storage-keys.const';
 
+type DialogSyncCfgComponentType =
+  typeof import('../../imex/sync/dialog-sync-cfg/dialog-sync-cfg.component').DialogSyncCfgComponent;
+
 @Component({
   selector: 'onboarding-preset-selection',
   changeDetection: ChangeDetectionStrategy.OnPush,
@@ -26,6 +29,7 @@ export class OnboardingPresetSelectionComponent {
   presetSelected = output<void>();
   dismissed = output<void>();
   selectedPreset = signal<OnboardingPreset | null>(null);
+  isSyncSetupInProgress = signal(false);
 
   selectPreset(preset: OnboardingPreset): void {
     if (this.selectedPreset()) {
@@ -38,19 +42,35 @@ export class OnboardingPresetSelectionComponent {
   }
 
   async setupSync(): Promise<void> {
-    if (this.selectedPreset()) {
+    if (this.selectedPreset() || this.isSyncSetupInProgress()) {
       return;
     }
-    const { DialogSyncCfgComponent } =
-      await import('../../imex/sync/dialog-sync-cfg/dialog-sync-cfg.component');
+    this.isSyncSetupInProgress.set(true);
+
+    let DialogSyncCfgComponent: DialogSyncCfgComponentType;
+    try {
+      ({ DialogSyncCfgComponent } =
+        await import('../../imex/sync/dialog-sync-cfg/dialog-sync-cfg.component'));
+    } catch (e) {
+      this.isSyncSetupInProgress.set(false);
+      throw e;
+    }
+
+    if (this.selectedPreset()) {
+      this.isSyncSetupInProgress.set(false);
+      return;
+    }
+
     const dialogRef = this._matDialog.open(DialogSyncCfgComponent);
     dialogRef.afterClosed().subscribe(() => {
+      this.isSyncSetupInProgress.set(false);
       // A returning user who actually enabled sync (e.g. to restore data from
       // another device) should not be forced to pick a preset afterwards —
       // that would overwrite the appFeatures config they just synced down.
       // Dismiss onboarding instead, without starting the new-user hint tour.
       if (this._globalConfigService.cfg()?.sync.isEnabled) {
         localStorage.setItem(LS.ONBOARDING_PRESET_DONE, 'true');
+        localStorage.setItem(LS.ONBOARDING_HINTS_DONE, 'true');
         this.dismissed.emit();
       }
     });
diff --git a/src/assets/i18n/en.json b/src/assets/i18n/en.json
index b35e24c8d6..ac682fc3e0 100644
--- a/src/assets/i18n/en.json
+++ b/src/assets/i18n/en.json
@@ -2638,7 +2638,7 @@
   "ONBOARDING": {
     "TITLE": "How do you want to use Super Productivity?",
     "SUBTITLE": "Pick a setup that matches your workflow. You can always change this later in Settings.",
-    "PRIVACY_HINT": "Your data stays securely on your device. Nothing is sent to a server.",
+    "PRIVACY_HINT": "Your data stays securely on your device unless you choose to set up sync.",
     "SYNC_CTA": "Already using Super Productivity on another device?",
     "SYNC_CTA_LINK": "Set up sync",
     "PRESETS": {
diff --git a/src/styles/_css-variables.scss b/src/styles/_css-variables.scss
index 4bfa436021..5b4208ed14 100644
--- a/src/styles/_css-variables.scss
+++ b/src/styles/_css-variables.scss
@@ -161,6 +161,7 @@
   // NOTE needs to be below the z-index of cdk-overlay which is 1000
   --z-add-task-bar: 999;
   --z-search-bar: 999;
+  --z-onboarding-presets: 999;
   --z-mobile-bottom-nav: 50;
   --z-right-panel: 44;
   --z-right-panel-resize-handle: 65;