-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New desktop onboarding experience #1681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -638,6 +638,15 @@ async fn set_camera_input( | |||||||||||||||||||||||||||||||||
| drop(app); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if id == current_id && camera_in_use { | ||||||||||||||||||||||||||||||||||
| if id.is_some() && !skip_camera_window.unwrap_or(false) { | ||||||||||||||||||||||||||||||||||
| let show_result = ShowCapWindow::Camera { centered: false } | ||||||||||||||||||||||||||||||||||
| .show(&app_handle) | ||||||||||||||||||||||||||||||||||
| .await; | ||||||||||||||||||||||||||||||||||
| show_result | ||||||||||||||||||||||||||||||||||
| .map_err(|err| error!("Failed to show camera preview window: {err}")) | ||||||||||||||||||||||||||||||||||
| .ok(); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return Ok(()); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -2693,16 +2702,19 @@ async fn reset_camera_permissions(_app: AppHandle) -> Result<(), String> { | |||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| #[tauri::command] | ||||||||||||||||||||||||||||||||||
| #[specta::specta] | ||||||||||||||||||||||||||||||||||
| #[instrument(skip(app))] | ||||||||||||||||||||||||||||||||||
| async fn reset_microphone_permissions(app: AppHandle) -> Result<(), ()> { | ||||||||||||||||||||||||||||||||||
| let bundle_id = app.config().identifier.clone(); | ||||||||||||||||||||||||||||||||||
| #[instrument(skip(_app))] | ||||||||||||||||||||||||||||||||||
| async fn reset_microphone_permissions(_app: AppHandle) -> Result<(), String> { | ||||||||||||||||||||||||||||||||||
| #[cfg(target_os = "macos")] | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| let bundle_id = _app.config().identifier.clone(); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Command::new("tccutil") | ||||||||||||||||||||||||||||||||||
| .arg("reset") | ||||||||||||||||||||||||||||||||||
| .arg("Microphone") | ||||||||||||||||||||||||||||||||||
| .arg(bundle_id) | ||||||||||||||||||||||||||||||||||
| .output() | ||||||||||||||||||||||||||||||||||
| .expect("Failed to reset microphone permissions"); | ||||||||||||||||||||||||||||||||||
| Command::new("tccutil") | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only errors on spawn failure; if
Suggested change
|
||||||||||||||||||||||||||||||||||
| .arg("reset") | ||||||||||||||||||||||||||||||||||
| .arg("Microphone") | ||||||||||||||||||||||||||||||||||
| .arg(bundle_id) | ||||||||||||||||||||||||||||||||||
| .output() | ||||||||||||||||||||||||||||||||||
| .map_err(|_| "Failed to reset microphone permissions".to_string())?; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
@@ -3336,7 +3348,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) { | |||||||||||||||||||||||||||||||||
| flags | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| .with_denylist(&[ | ||||||||||||||||||||||||||||||||||
| CapWindowId::Setup.label().as_str(), | ||||||||||||||||||||||||||||||||||
| CapWindowId::Onboarding.label().as_str(), | ||||||||||||||||||||||||||||||||||
| CapWindowId::Main.label().as_str(), | ||||||||||||||||||||||||||||||||||
| "window-capture-occluder", | ||||||||||||||||||||||||||||||||||
| "target-select-overlay", | ||||||||||||||||||||||||||||||||||
|
|
@@ -3501,18 +3513,24 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) { | |||||||||||||||||||||||||||||||||
| tokio::spawn({ | ||||||||||||||||||||||||||||||||||
| let app = app.clone(); | ||||||||||||||||||||||||||||||||||
| async move { | ||||||||||||||||||||||||||||||||||
| if !permissions.screen_recording.permitted() | ||||||||||||||||||||||||||||||||||
| || !permissions.accessibility.permitted() | ||||||||||||||||||||||||||||||||||
| || GeneralSettingsStore::get(&app) | ||||||||||||||||||||||||||||||||||
| .ok() | ||||||||||||||||||||||||||||||||||
| .flatten() | ||||||||||||||||||||||||||||||||||
| .map(|s| !s.has_completed_startup) | ||||||||||||||||||||||||||||||||||
| .unwrap_or(false) | ||||||||||||||||||||||||||||||||||
| let settings = GeneralSettingsStore::get(&app).ok().flatten(); | ||||||||||||||||||||||||||||||||||
| let startup_completed = settings | ||||||||||||||||||||||||||||||||||
| .as_ref() | ||||||||||||||||||||||||||||||||||
| .map(|s| s.has_completed_startup) | ||||||||||||||||||||||||||||||||||
| .unwrap_or(false); | ||||||||||||||||||||||||||||||||||
| let onboarding_completed = settings | ||||||||||||||||||||||||||||||||||
| .as_ref() | ||||||||||||||||||||||||||||||||||
| .map(|s| s.has_completed_onboarding) | ||||||||||||||||||||||||||||||||||
| .unwrap_or(false); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if !startup_completed | ||||||||||||||||||||||||||||||||||
| || !onboarding_completed | ||||||||||||||||||||||||||||||||||
| || !permissions.necessary_granted() | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| let _ = ShowCapWindow::Setup.show(&app).await; | ||||||||||||||||||||||||||||||||||
| println!("Showing onboarding"); | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: printlnprintln! used instead of tracing macros for startup logs. Structured logging is bypassed. Use info! or debug! macros instead. View DetailsLocation: Analysisprintln! used instead of tracing macros for startup logs. Structured logging is bypassed
How to reproduce1. Build and run the desktop app. 2. Observe stdout for 'Showing onboarding' or 'Showing main window' messages. 3. Note these don't appear in structured logs.Patch Details- println!("Showing onboarding");
+ info!("Showing onboarding");AI Fix PromptTip: Reply with |
||||||||||||||||||||||||||||||||||
| let _ = ShowCapWindow::Onboarding.show(&app).await; | ||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||
| println!("Permissions granted, showing main window"); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| println!("Showing main window"); | ||||||||||||||||||||||||||||||||||
| let _ = ShowCapWindow::Main { | ||||||||||||||||||||||||||||||||||
| init_target_mode: None, | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
@@ -3864,12 +3882,18 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) { | |||||||||||||||||||||||||||||||||
| .run(move |_handle, event| match event { | ||||||||||||||||||||||||||||||||||
| #[cfg(target_os = "macos")] | ||||||||||||||||||||||||||||||||||
| tauri::RunEvent::Reopen { .. } => { | ||||||||||||||||||||||||||||||||||
| if let Some(onboarding) = CapWindowId::Onboarding.get(_handle) { | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Dock Reopen handler always focuses onboarding over all windowsDock Reopen handler always focuses onboarding over all windows. Users can't access other windows via dock click. Move onboarding check after other window checks. View DetailsLocation: AnalysisDock Reopen handler always focuses onboarding over all windows
How to reproduce1. Open the main window. 2. Click 'Help & Tour' to open onboarding. 3. Open an editor window. 4. Click the dock icon (macOS).AI Fix PromptTip: Reply with |
||||||||||||||||||||||||||||||||||
| onboarding.show().ok(); | ||||||||||||||||||||||||||||||||||
| onboarding.set_focus().ok(); | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| let has_window = _handle.webview_windows().iter().any(|(label, _)| { | ||||||||||||||||||||||||||||||||||
| label.starts_with("editor-") | ||||||||||||||||||||||||||||||||||
| || label.starts_with("screenshot-editor-") | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "settings" | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "signin" | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "setup" | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "onboarding" | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if has_window { | ||||||||||||||||||||||||||||||||||
|
|
@@ -3881,7 +3905,7 @@ pub async fn run(recording_logging_handle: LoggingHandle, logs_dir: PathBuf) { | |||||||||||||||||||||||||||||||||
| || label.starts_with("screenshot-editor-") | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "settings" | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "signin" | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "setup" | ||||||||||||||||||||||||||||||||||
| || label.as_str() == "onboarding" | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| .map(|(_, window)| window.clone()) | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serdedefault andDefaultimpl disagree onhas_completed_onboardingThe field uses
#[serde(default = "default_true")](deserialises missing field astrue) butDefault::default()returnsfalse. The intent is clear: existing users whose settings files pre-date this field should be treated as having completed onboarding, while a truly fresh install (no file →Default::default()) must go through the flow. However, the mismatch is a maintenance footgun — any code that constructs aGeneralSettingsStoreviaDefault::default()(e.g., in tests or fallback paths) silently getsfalse, which would force unexpected onboarding. A comment on the field explaining the deliberate discrepancy would help future contributors avoid breakage.Prompt To Fix With AI