Onboarding in desktop2 #1640
Onboarding in desktop2 #1640
Conversation
📝 WalkthroughWalkthroughThis PR introduces onboarding state management and conditional window display logic. It adds a new Changes
Sequence DiagramsequenceDiagram
participant App as App Startup
participant Store as Desktop Store
participant UI as Frontend Window
App->>Store: Check OnboardingNeeded2
alt Onboarding needed
Store-->>App: true (or missing)
App->>UI: Show Onboarding window
UI->>UI: User completes flow
UI->>Store: setOnboardingNeeded(false)
Store-->>UI: Persisted
App->>UI: Show Main window
App->>App: Close Onboarding window
else Onboarding complete
Store-->>App: false
App->>UI: Show Main window
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/store.rs (1)
5-5: Consider clarifying the "2" suffix in the variant name.The
OnboardingNeeded2name suggests versioning or migration from a previous implementation. If this is intentional for migration purposes, consider adding a comment explaining why. If not, a simpler name likeOnboardingNeededmight be clearer.apps/desktop/src-tauri/src/lib.rs (1)
117-137: Consider extracting the duplicated window logic.The conditional window display logic is duplicated between the setup handler (Lines 117-126) and the Reopen event handler (Lines 131-137). Consider extracting this to a helper function to reduce duplication.
Example refactor:
fn handle_onboarding_windows(app: &AppHandle<tauri::Wry>) { if app.get_onboarding_needed().unwrap_or(true) { if let Err(e) = AppWindow::Main.hide(app) { tracing::error!("failed_to_hide_main_window: {:?}", e); } if let Err(e) = AppWindow::Onboarding.show(app) { tracing::error!("failed_to_show_onboarding_window: {:?}", e); } } else { if let Err(e) = AppWindow::Onboarding.destroy(app) { tracing::error!("failed_to_destroy_onboarding_window: {:?}", e); } if let Err(e) = AppWindow::Main.show(app) { tracing::error!("failed_to_show_main_window: {:?}", e); } } }Then call it from both locations:
// In setup { let app_handle = app.handle().clone(); handle_onboarding_windows(&app_handle); } // In run if let tauri::RunEvent::Reopen { .. } = event { handle_onboarding_windows(&app); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapps/desktop/src/types/tauri.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (10)
apps/desktop/src-tauri/Cargo.toml(2 hunks)apps/desktop/src-tauri/src/commands.rs(2 hunks)apps/desktop/src-tauri/src/ext.rs(1 hunks)apps/desktop/src-tauri/src/lib.rs(4 hunks)apps/desktop/src-tauri/src/store.rs(1 hunks)apps/desktop/src/components/onboarding/permissions.tsx(0 hunks)apps/desktop/src/routes/app/onboarding/index.tsx(3 hunks)crates/notification/src/lib.rs(0 hunks)packages/utils/src/index.ts(0 hunks)plugins/windows/src/ext.rs(2 hunks)
💤 Files with no reviewable changes (3)
- apps/desktop/src/components/onboarding/permissions.tsx
- crates/notification/src/lib.rs
- packages/utils/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (6)
plugins/windows/src/ext.rs (2)
plugins/windows/src/events.rs (1)
app(32-32)plugins/windows/src/overlay.rs (1)
app(37-37)
apps/desktop/src-tauri/src/commands.rs (1)
apps/desktop/src-tauri/src/ext.rs (4)
get_onboarding_needed(6-6)get_onboarding_needed(17-23)set_onboarding_needed(7-7)set_onboarding_needed(26-31)
apps/desktop/src-tauri/src/store.rs (1)
plugins/auth/js/bindings.gen.ts (1)
StoreKey(54-54)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src-tauri/src/commands.rs (2)
get_onboarding_needed(47-51)set_onboarding_needed(55-60)apps/desktop/src-tauri/src/ext.rs (4)
get_onboarding_needed(6-6)get_onboarding_needed(17-23)set_onboarding_needed(7-7)set_onboarding_needed(26-31)
apps/desktop/src-tauri/src/ext.rs (2)
apps/desktop/src-tauri/src/commands.rs (2)
get_onboarding_needed(47-51)set_onboarding_needed(55-60)plugins/store2/src/ext.rs (2)
store(6-6)store(14-18)
apps/desktop/src/routes/app/onboarding/index.tsx (2)
apps/desktop/src/types/tauri.gen.ts (1)
commands(9-34)plugins/windows/js/bindings.gen.ts (1)
commands(9-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (6)
apps/desktop/src-tauri/Cargo.toml (1)
46-46: LGTM!The new dependencies are correctly specified for the onboarding state management feature.
Also applies to: 60-60
apps/desktop/src/routes/app/onboarding/index.tsx (1)
66-73: LGTM!The onboarding completion flow correctly persists state before transitioning to the main window.
apps/desktop/src-tauri/src/commands.rs (1)
45-60: LGTM!The commands provide a clean interface for frontend access to onboarding state.
plugins/windows/src/ext.rs (1)
48-48: LGTM!Making
hideanddestroypublic enables the conditional window display logic in the onboarding flow.Also applies to: 64-64
apps/desktop/src-tauri/src/lib.rs (1)
2-6: LGTM!The module structure and plugin integration are correctly implemented.
Also applies to: 71-71, 146-147
apps/desktop/src-tauri/src/ext.rs (1)
1-32: LGTM!The
AppExttrait provides a clean, well-structured interface for onboarding state management. The default value oftrueforget_onboarding_neededcorrectly handles first-time users, and the tracing instrumentation aids debugging.
No description provided.