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.
This commit is contained in:
Johannes Millan 2026-06-03 19:04:36 +02:00
parent 7427cb070d
commit 524e505353
7 changed files with 77 additions and 21 deletions

View file

@ -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

View file

@ -33,7 +33,7 @@
<button
type="button"
class="sync-cta-link"
[disabled]="!!selectedPreset()"
[disabled]="!!selectedPreset() || isSyncSetupInProgress()"
(click)="setupSync()"
>
{{ 'ONBOARDING.SYNC_CTA_LINK' | translate }}

View file

@ -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 {

View file

@ -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();
});
});
});

View file

@ -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();
}
});

View file

@ -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": {

View file

@ -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;