From 63614c3aa785652cfebce275da3a242413f749d9 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:38:00 +0200 Subject: [PATCH] fix: persist onboarding notices and expose tip controls --- README.md | 6 +- config.example.toml | 8 +- configurator/src/app/pages/ui/general.rs | 6 + configurator/src/app/search/terms.rs | 8 + configurator/src/app/search/tests.rs | 27 +- configurator/src/app/startup.rs | 15 ++ .../src/models/config/draft/from_config.rs | 1 + configurator/src/models/config/draft/mod.rs | 1 + configurator/src/models/config/setters.rs | 1 + configurator/src/models/config/tests.rs | 16 ++ .../src/models/config/to_config/ui.rs | 1 + configurator/src/models/fields/toggles.rs | 1 + docs/CONFIG.md | 20 +- .../wayland/backend/state_init/config.rs | 33 ++- src/backend/wayland/backend/state_init/mod.rs | 63 ++--- src/backend/wayland/state/onboarding.rs | 44 +++- .../wayland/state/onboarding/first_run.rs | 19 +- src/backend/wayland/state/onboarding/tests.rs | 12 +- src/config/action_meta/entries/ui.rs | 10 + src/config/action_meta/tests.rs | 1 + src/config/keybindings/config/map/edit.rs | 1 + src/config/tests/load.rs | 43 +++ src/config/tests/migration.rs | 13 + src/config/types/ui.rs | 10 + src/configurator_destination.rs | 23 +- src/domain/action.rs | 2 + src/domain/tests.rs | 4 + src/input/events.rs | 2 +- src/input/state/actions/action_ui.rs | 7 +- src/input/state/interaction/actions.rs | 1 + src/onboarding.rs | 245 ++++++++++++++---- src/onboarding/tests.rs | 154 +++++++++-- 32 files changed, 648 insertions(+), 150 deletions(-) diff --git a/README.md b/README.md index c762c599..38db6345 100644 --- a/README.md +++ b/README.md @@ -148,8 +148,8 @@ The v0.9.23+ prebuilt `wayscriber` packages require glibc 2.39 and GTK 4.12 — - Overlay Session panel, configurator Session tab, tray toggle, and CLI overrides - See [Session manager and persistence](#session-manager-and-persistence) for the full workflow -### Toolbars and UI -- Floating toolbars (F9 toggles visibility; F2 cycles the top strip full → micro chip → hidden) +### Toolbar and UI +- Unified floating toolbar (F9 toggles visibility; F2 cycles the top strip full → micro chip → hidden) - Two toolbar frontends: GTK4-rendered bars on layer-shell compositors (Hyprland, KWin, Wayfire, River, ...), with automatic fallback to the built-in Cairo bars everywhere else (GNOME xdg fallback, forced-inline mode, builds without the `toolbar-gtk` feature) - Pick a frontend explicitly with `ui.toolbar.backend = "auto" | "gtk" | "builtin"` or `WAYSCRIBER_TOOLBAR_BACKEND` - Preset slots, icon or text modes @@ -922,7 +922,7 @@ pick_screen_color = ["I"] | Cancel action | Right-click (while drawing) / Escape | | Context menu | Right-click (idle) / Shift+F10 / Menu, Arrow keys + Enter/Space | | Edit selected text/note | Enter (single selection) | -| Toggle toolbars | F9 | +| Toggle toolbar | F9 | | Cycle top strip (full → micro → hidden) | F2 | | Help overlay | F1 / F10 | | Quick reference | Shift+F1 | diff --git a/config.example.toml b/config.example.toml index 175108fe..fad4f761 100644 --- a/config.example.toml +++ b/config.example.toml @@ -187,7 +187,7 @@ toggle_zoom_chip = [] # (unbound; also in the command palette) toggle_focus_mode = [] -# Toggle toolbars (show/hide top and side together) +# Toggle the unified top toolbar toggle_toolbar = ["F9"] # Cycle the top toolbar's display: full strip -> micro chip -> hidden @@ -444,6 +444,10 @@ help_overlay_context_filter = true # Show compositor capabilities warning toast each time the overlay starts show_capabilities_warning = true +# Show automatic first-run guidance, discovery tips, and shortcut coaching. +# The guided tour remains available manually when this is false. +show_onboarding_hints = true + # Command palette action toast duration (ms) command_palette_toast_duration_ms = 1500 @@ -466,7 +470,7 @@ multi_monitor_enabled = true radial_menu_mouse_binding = "middle" # ─────────────────────────────────────────────────────────────────────────────── -# Floating Toolbars (F9 toggles; F2 cycles full -> micro -> hidden) +# Floating Toolbar (F9 toggles; F2 cycles full -> micro -> hidden) # ─────────────────────────────────────────────────────────────────────────────── [ui.toolbar] diff --git a/configurator/src/app/pages/ui/general.rs b/configurator/src/app/pages/ui/general.rs index 5517ec5c..87c20c38 100644 --- a/configurator/src/app/pages/ui/general.rs +++ b/configurator/src/app/pages/ui/general.rs @@ -59,6 +59,12 @@ pub(super) fn build(sender: &ComponentSender) -> BuiltPage { |app| app.draft.ui_show_capabilities_warning, |value| Message::ToggleChanged(ToggleField::UiShowCapabilitiesWarning, value), ) + .switch_row( + "Show automatic guidance and tips", + "Controls first-run guidance, discovery tips, and shortcut coaching. The guided tour remains available manually.", + |app| app.draft.ui_show_onboarding_hints, + |value| Message::ToggleChanged(ToggleField::UiShowOnboardingHints, value), + ) .entry_row( "Command palette toast (ms)", |app| app.draft.ui_command_palette_toast_duration_ms.clone(), diff --git a/configurator/src/app/search/terms.rs b/configurator/src/app/search/terms.rs index 6fc17ad2..9e5cba1a 100644 --- a/configurator/src/app/search/terms.rs +++ b/configurator/src/app/search/terms.rs @@ -171,6 +171,14 @@ pub(super) const UI_GENERAL_TERMS: &[&str] = &[ "enable context menu", "show capabilities warning toast", "capabilities warning", + "show automatic guidance and tips", + "onboarding", + "hints", + "onboarding hints", + "show_onboarding_hints", + "tutorial", + "guidance", + "tips", "command palette toast", ]; pub(super) const UI_TOOLBAR_TERMS: &[&str] = &[ diff --git a/configurator/src/app/search/tests.rs b/configurator/src/app/search/tests.rs index 32c0fe2e..76f0fb1d 100644 --- a/configurator/src/app/search/tests.rs +++ b/configurator/src/app/search/tests.rs @@ -582,13 +582,30 @@ fn exact_pdf_background_field_label_matches_pdf_section() { #[test] fn exact_general_ui_field_labels_match_general_ui_section() { - let (mut app, _effects) = ConfiguratorApp::new_app(); - app.search_query = SearchQuery::new("focus loss"); + for query in [ + "focus loss", + "show automatic guidance and tips", + "onboarding", + "tutorial", + "guidance", + "tips", + "hints", + "onboarding hints", + "show_onboarding_hints", + ] { + let (mut app, _effects) = ConfiguratorApp::new_app(); + app.search_query = SearchQuery::new(query); - let summary = app.search_summary(); - let ui = summary.tab(TabId::Ui).expect("ui match"); + let summary = app.search_summary(); + let ui = summary + .tab(TabId::Ui) + .unwrap_or_else(|| panic!("UI should match visible General UI wording: {query}")); - assert!(ui.area_matches(SearchArea::UiGeneral)); + assert!( + ui.area_matches(SearchArea::UiGeneral), + "query should reveal General UI: {query}" + ); + } } #[test] diff --git a/configurator/src/app/startup.rs b/configurator/src/app/startup.rs index f4062841..c4276817 100644 --- a/configurator/src/app/startup.rs +++ b/configurator/src/app/startup.rs @@ -221,6 +221,21 @@ mod tests { assert!(!app.startup_search_focus_pending); } + #[test] + fn the_onboarding_hint_destination_lands_on_the_general_ui_setting() { + let launched = + wayscriber::configurator_destination::onboarding_hints_destination().as_arg(); + let (app, _dir, _path) = app_launched_with(&["--open", &launched]); + + assert_eq!(app.active_tab, TabId::Ui); + assert_eq!(app.search_query.raw(), "Show automatic guidance and tips"); + let summary = app.search_summary(); + let ui = summary + .tab(TabId::Ui) + .expect("General UI destination match"); + assert!(ui.area_matches(crate::app::search::SearchArea::UiGeneral)); + } + /// The shortcut-action row: subtab plus action search. #[test] fn a_keybinding_action_destination_lands_on_its_subtab_with_the_term_searched() { diff --git a/configurator/src/models/config/draft/from_config.rs b/configurator/src/models/config/draft/from_config.rs index 35dba0dd..c5bc2c93 100644 --- a/configurator/src/models/config/draft/from_config.rs +++ b/configurator/src/models/config/draft/from_config.rs @@ -133,6 +133,7 @@ impl ConfigDraft { ui_show_page_badge_with_status_bar: config.ui.show_floating_badge_always, ui_show_frozen_badge: config.ui.show_frozen_badge, ui_show_capabilities_warning: config.ui.show_capabilities_warning, + ui_show_onboarding_hints: config.ui.show_onboarding_hints, ui_context_menu_enabled: config.ui.context_menu.enabled, ui_preferred_output: config.ui.preferred_output.clone().unwrap_or_default(), ui_xdg_fullscreen: config.ui.xdg_fullscreen, diff --git a/configurator/src/models/config/draft/mod.rs b/configurator/src/models/config/draft/mod.rs index e7709cda..ad6d9a0f 100644 --- a/configurator/src/models/config/draft/mod.rs +++ b/configurator/src/models/config/draft/mod.rs @@ -93,6 +93,7 @@ pub struct ConfigDraft { pub ui_show_page_badge_with_status_bar: bool, pub ui_show_frozen_badge: bool, pub ui_show_capabilities_warning: bool, + pub ui_show_onboarding_hints: bool, pub ui_context_menu_enabled: bool, pub ui_preferred_output: String, pub ui_xdg_fullscreen: bool, diff --git a/configurator/src/models/config/setters.rs b/configurator/src/models/config/setters.rs index aafda684..3a08cfbc 100644 --- a/configurator/src/models/config/setters.rs +++ b/configurator/src/models/config/setters.rs @@ -173,6 +173,7 @@ impl ConfigDraft { ToggleField::UiShowStatusAbout => self.ui_show_status_about = value, ToggleField::UiShowFrozenBadge => self.ui_show_frozen_badge = value, ToggleField::UiShowCapabilitiesWarning => self.ui_show_capabilities_warning = value, + ToggleField::UiShowOnboardingHints => self.ui_show_onboarding_hints = value, ToggleField::UiShowStatusBoardBadge => self.ui_show_status_board_badge = value, ToggleField::UiShowStatusPageBadge => self.ui_show_status_page_badge = value, ToggleField::UiShowToolbarHint => self.ui_show_toolbar_hint = value, diff --git a/configurator/src/models/config/tests.rs b/configurator/src/models/config/tests.rs index d0c4c62c..d0175bfb 100644 --- a/configurator/src/models/config/tests.rs +++ b/configurator/src/models/config/tests.rs @@ -39,6 +39,22 @@ fn config_draft_round_trips_status_bar_interactive() { .expect("status bar interactive round trip"); assert!(!round_trip.ui.status_bar_interactive); } + +#[test] +fn config_draft_round_trips_automatic_onboarding_preference() { + let config = Config::default(); + let mut draft = ConfigDraft::from_config(&config); + assert!( + draft.ui_show_onboarding_hints, + "automatic guidance defaults on" + ); + + draft.set_toggle(ToggleField::UiShowOnboardingHints, false); + let round_trip = draft + .to_config(&config) + .expect("onboarding preference should round trip"); + assert!(!round_trip.ui.show_onboarding_hints); +} use super::super::{ColorMode, NamedColorOption}; use super::{ConfigDraft, RenderProfileSelectionOption}; use wayscriber::config::{ diff --git a/configurator/src/models/config/to_config/ui.rs b/configurator/src/models/config/to_config/ui.rs index 23703ab8..c2f2dc15 100644 --- a/configurator/src/models/config/to_config/ui.rs +++ b/configurator/src/models/config/to_config/ui.rs @@ -24,6 +24,7 @@ impl ConfigDraft { config.ui.show_floating_badge_always = self.ui_show_page_badge_with_status_bar; config.ui.show_frozen_badge = self.ui_show_frozen_badge; config.ui.show_capabilities_warning = self.ui_show_capabilities_warning; + config.ui.show_onboarding_hints = self.ui_show_onboarding_hints; config.ui.context_menu.enabled = self.ui_context_menu_enabled; let preferred_output = self.ui_preferred_output.trim(); config.ui.preferred_output = if preferred_output.is_empty() { diff --git a/configurator/src/models/fields/toggles.rs b/configurator/src/models/fields/toggles.rs index 5a9e2dd5..93c6182f 100644 --- a/configurator/src/models/fields/toggles.rs +++ b/configurator/src/models/fields/toggles.rs @@ -15,6 +15,7 @@ pub enum ToggleField { UiShowStatusAbout, UiShowFrozenBadge, UiShowCapabilitiesWarning, + UiShowOnboardingHints, UiHelpOverlayContextFilter, UiContextMenuEnabled, UiXdgFullscreen, diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 87fc9d7b..4d9823aa 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -704,6 +704,13 @@ show_frozen_badge = false # Filter help overlay sections based on enabled features help_overlay_context_filter = true +# Show compositor capability warnings when the overlay starts +show_capabilities_warning = true + +# Show automatic first-run guidance, discovery tips, and shortcut coaching. +# The guided tour remains available manually when this is false. +show_onboarding_hints = true + # Command palette action toast duration (ms) command_palette_toast_duration_ms = 1500 @@ -977,7 +984,7 @@ wayscriber --light-draw-off Use `--light-draw-on` on key/button press and `--light-draw-off` on release for a non-sticky draw-while-held shortcut. The raw `--daemon-action` form remains available for scripts. -### `[ui.toolbar]` - Floating Toolbars +### `[ui.toolbar]` - Floating Toolbar Controls the unified top toolbar (F9 toggles visibility; F2 cycles the top strip full → micro → hidden). @@ -1170,6 +1177,8 @@ top_controls = [ - **Polygon tools**: Full mode shows Triangle, Parallelogram, Rhombus, Regular Polygon, and Freeform Polygon under the compact Polygons picker. Simple mode exposes them in the Shapes picker. - **Context-aware UI**: `context_aware_ui` shows/hides tool-specific controls (colors, thickness, arrow labels, etc.) based on the active tool; disable to always show all controls. - **Preset toasts**: `show_preset_toasts` enables toast confirmations for preset apply/save/clear. +- **Automatic guidance**: `show_onboarding_hints` controls first-run cards, discovery tips, and shortcut coaching. Set it to `false` to disable all automatic tutorials; the guided tour remains available manually. Completed profiles migrated from onboarding versions before v6 are not enrolled in the later status-bar, Canvas, and zoom tip series. +- **Capability warnings**: `show_capabilities_warning` independently controls compositor limitation warnings; disabling tutorials does not hide safety, configuration, or capability diagnostics. - **Tool preview**: `show_tool_preview` toggles the cursor bubble. - **Offsets**: `top_offset` and `top_offset_y` are the authored default top-toolbar position. Dragging the strip saves its position as a runtime preference in `runtime-ui.toml` and leaves these untouched; editing one here again takes over from the saved drag. - **Force inline**: `force_inline` (or `WAYSCRIBER_FORCE_INLINE_TOOLBARS`) skips layer-shell toolbars. @@ -1631,6 +1640,13 @@ one is a tidy-up that records your decision in the file. `CURRENT_CONFIG_REVISION` bump. A default is only ever offered to an action a configuration omits, and only where the key is free, so a new or moved default cannot land on a shortcut a user bound to something else (#293, #315); the skipped-default diagnostic reports the stand-down instead. +That informational notice is acknowledged in the profile's +`$XDG_DATA_HOME/wayscriber/onboarding.toml` state and appears once for each exact set of skipped +bindings; a later, different skipped default can notify once again. Actual +parse errors, invalid bindings, and conflicts continue to report on every start until they are fixed. +For the historical toolbar pair, either accept the revision-2 proposal (`F9` toggles the unified top +toolbar and `F2` cycles full → micro → hidden) or explicitly write +`cycle_toolbar_display = []` to keep `F2`/`F9` on `toggle_toolbar` without another notice. What is still required: the new default must not collide with another shipped default (`default_keybindings_have_no_conflicts` guards that), and `default_bindings_match_the_checked_in_snapshot` holds a snapshot of every shipped default and fails @@ -1787,7 +1803,7 @@ toggle_zoom_chip = [] # (unbound; also in the command palette) toggle_focus_mode = [] -# Toggle toolbars (show/hide top and side together). +# Toggle the unified top toolbar. # Note: F2 moved to cycle_toolbar_display; hiding is still reachable via # the cycle, and explicit user configs keep whatever they bound. toggle_toolbar = ["F9"] diff --git a/src/backend/wayland/backend/state_init/config.rs b/src/backend/wayland/backend/state_init/config.rs index 5e39b18c..fa123fba 100644 --- a/src/backend/wayland/backend/state_init/config.rs +++ b/src/backend/wayland/backend/state_init/config.rs @@ -8,6 +8,7 @@ use crate::config::{ use crate::input::InputState; use crate::input::state::{Toast, ToastPriority}; use crate::notification; +use crate::onboarding::OnboardingStore; /// How long the keybinding warnings stay up. All of them are on the long side: /// they describe a config problem the user has to leave the overlay to fix. @@ -255,19 +256,28 @@ pub(super) fn notify_invalid_keybindings( pub(super) fn notify_skipped_default_shortcuts( input_state: &mut InputState, tokio_handle: &tokio::runtime::Handle, + onboarding: &mut OnboardingStore, skipped: &[DefaultShortcutSkipped], ) { - if skipped.is_empty() { + if skipped.is_empty() || !onboarding.persistence_available() { return; } - input_state.push_toast( + let notice_id = skipped_default_notice_id(skipped); + if onboarding.startup_notice_acknowledged(¬ice_id) { + return; + } + + let outcome = input_state.push_toast( ToastPriority::Action, "keybindings.skipped-default", Toast::info(skipped_default_toast(skipped)) .action("Shortcuts", Action::OpenConfiguratorKeybindings) .duration_ms(KEYBINDING_CONFLICT_TOAST_MS), ); + if !outcome.accepted() || onboarding.acknowledge_startup_notice(¬ice_id).is_err() { + return; + } notification::send_notification_with_timeout_async( tokio_handle, "New Default Shortcuts".to_string(), @@ -277,6 +287,21 @@ pub(super) fn notify_skipped_default_shortcuts( ); } +fn skipped_default_notice_id(skipped: &[DefaultShortcutSkipped]) -> String { + let mut entries = skipped + .iter() + .map(|entry| { + let skipped_action = entry.config_key().unwrap_or("runtime_action"); + let claimed_by = + crate::config::KeybindingsConfig::config_key_for_action(entry.claimed_by()) + .unwrap_or("runtime_action"); + format!("{}:{skipped_action}:{claimed_by}", entry.binding()) + }) + .collect::>(); + entries.sort_unstable(); + format!("skipped-default:{}", entries.join(",")) +} + fn keybinding_conflict_toast(conflicts: &[KeybindingConflictResolution]) -> String { keybinding_toast( "Shortcut conflict", @@ -495,6 +520,10 @@ mod tests { loaded.keybindings.skipped_default_shortcuts[0].binding(), "F2" ); + assert_eq!( + skipped_default_notice_id(&loaded.keybindings.skipped_default_shortcuts), + "skipped-default:F2:cycle_toolbar_display:toggle_toolbar" + ); let toast = skipped_default_toast(&loaded.keybindings.skipped_default_shortcuts); assert!(toast.contains("F2"), "unexpected toast: {toast}"); diff --git a/src/backend/wayland/backend/state_init/mod.rs b/src/backend/wayland/backend/state_init/mod.rs index 412fee6a..c6e3e4b8 100644 --- a/src/backend/wayland/backend/state_init/mod.rs +++ b/src/backend/wayland/backend/state_init/mod.rs @@ -15,7 +15,7 @@ use crate::{ config::Config, input::InputState, input::state::{CompositorCapabilities, DesktopEnvironment, ShellMode}, - onboarding::{DEFERRED_HINT_REPEAT_MAX, OnboardingStore}, + onboarding::OnboardingStore, }; mod config; @@ -74,11 +74,6 @@ pub(super) fn init_state(backend: &WaylandBackend, setup: WaylandSetup) -> Resul backend.tokio_runtime.handle(), &keybindings.keybinding_conflicts, ); - config::notify_skipped_default_shortcuts( - &mut input_state, - backend.tokio_runtime.handle(), - &keybindings.skipped_default_shortcuts, - ); let runtime_ui_path = crate::paths::runtime_ui_state_file(); let (runtime_ui, runtime_ui_unavailable) = match crate::backend::wayland::runtime_ui_state::ToolbarRuntimeState::start( @@ -132,48 +127,24 @@ pub(super) fn init_state(backend: &WaylandBackend, setup: WaylandSetup) -> Resul }; let mut onboarding = OnboardingStore::load(); + let onboarding_save = onboarding.begin_session(config.ui.show_onboarding_hints); + if let Err(error) = &onboarding_save + && config.ui.show_onboarding_hints { - let state = onboarding.state_mut(); - state.sessions_seen = state.sessions_seen.saturating_add(1); - // Re-arm deferred hints per session until each feature is actually used. - if !state.used_help_overlay && state.hint_help_count < DEFERRED_HINT_REPEAT_MAX { - state.hint_help_shown = false; - } - if !state.used_command_palette && state.hint_palette_count < DEFERRED_HINT_REPEAT_MAX { - state.hint_palette_shown = false; - } - if !state.used_radial_menu - && !state.used_context_menu_right_click - && !state.used_context_menu_keyboard - && state.hint_quick_access_count < DEFERRED_HINT_REPEAT_MAX - { - state.hint_quick_access_shown = false; - } - // M9 surface hints have no per-feature "used" signal, so they re-arm - // purely on the across-session count cap (up to DEFERRED_HINT_REPEAT_MAX - // gentle reminders each). - if state.hint_status_bar_count < DEFERRED_HINT_REPEAT_MAX { - state.hint_status_bar_shown = false; - } - if state.hint_zoom_chip_count < DEFERRED_HINT_REPEAT_MAX { - state.hint_zoom_chip_shown = false; - } - if state.hint_canvas_popover_count < DEFERRED_HINT_REPEAT_MAX { - state.hint_canvas_popover_shown = false; - } - if !state.first_run_completed && !state.first_run_skipped { - state - .active_step - .get_or_insert(crate::onboarding::FirstRunStep::BackgroundModeSetup); - } else { - state.active_step = None; - state.quick_access_requires_toolbar = false; - } - // Keep legacy flags marked so older checks never re-trigger. - state.welcome_shown = true; - state.tour_shown = true; + input_state.push_toast( + crate::input::state::ToastPriority::Critical, + "onboarding.persistence", + crate::input::state::Toast::warning(format!( + "Automatic guidance is off because onboarding progress could not be saved: {error}" + )), + ); } - onboarding.save(); + config::notify_skipped_default_shortcuts( + &mut input_state, + backend.tokio_runtime.handle(), + &mut onboarding, + &keybindings.skipped_default_shortcuts, + ); // Seed the palette's recent-commands history from its persisted store. let palette_recents_store = crate::palette_recents::PaletteRecentsStore::load(); diff --git a/src/backend/wayland/state/onboarding.rs b/src/backend/wayland/state/onboarding.rs index d21e8959..8bbb783a 100644 --- a/src/backend/wayland/state/onboarding.rs +++ b/src/backend/wayland/state/onboarding.rs @@ -109,7 +109,12 @@ impl WaylandState { pub(in crate::backend::wayland) fn apply_onboarding_hints(&mut self) { // Show capability warning toast first if applicable. self.apply_capability_toast(); - // Honest legacy notice: nudge panel-mode users toward the new homes. + if !automatic_onboarding_allowed( + self.config.ui.show_onboarding_hints, + self.onboarding.persistence_available(), + ) { + return; + } // Capture the coach's slow-path signal before apply_first_run_progress // drains pending_onboarding_usage. let coach_slow_path = self @@ -195,7 +200,7 @@ impl WaylandState { let outcome = self.input_state.push_toast( ToastPriority::Hint, "onboarding.coach", - Toast::info(message), + Toast::info(message).action("Tip settings…", Action::OpenConfiguratorOnboardingHints), ); if outcome.accepted() { let session = &mut self.data.shortcut_coach; @@ -208,7 +213,7 @@ impl WaylandState { if state.coach_hint_count >= DEFERRED_HINT_REPEAT_MAX { state.coach_hint_shown = true; } - self.onboarding.save(); + self.save_onboarding_state(); } } @@ -318,8 +323,8 @@ impl WaylandState { } } - if changed { - self.onboarding.save(); + if changed && !self.save_onboarding_state() { + hint_kind = None; } if let Some(kind) = hint_kind { let message = match kind { @@ -365,7 +370,8 @@ impl WaylandState { self.input_state.push_toast( ToastPriority::Hint, "onboarding.hint", - Toast::info(message), + Toast::info(message) + .action("Tip settings…", Action::OpenConfiguratorOnboardingHints), ); } } @@ -396,7 +402,7 @@ impl WaylandState { ); if outcome.accepted() { self.onboarding.state_mut().toolbar_hint_shown = true; - self.onboarding.save(); + self.save_onboarding_state(); } } @@ -405,6 +411,23 @@ impl WaylandState { .unwrap_or_else(|| fallback.to_string()) } + fn save_onboarding_state(&mut self) -> bool { + match self.onboarding.save() { + Ok(()) => true, + Err(error) => { + self.input_state.push_toast( + ToastPriority::Critical, + "onboarding.persistence", + Toast::warning(format!( + "Automatic guidance is off because onboarding progress could not be saved: {error}" + )) + .once_per_content(), + ); + false + } + } + } + fn shortcut_label_opt(&self, action: Action) -> Option { self.input_state.shortcut_for_action(action) } @@ -434,6 +457,13 @@ impl WaylandState { } } +pub(super) fn automatic_onboarding_allowed( + preference_enabled: bool, + persistence_available: bool, +) -> bool { + preference_enabled && persistence_available +} + /// The capability warning to raise, if any: only when this exact capability /// set has not been evaluated yet this session (once per session unless the /// detected capabilities change) and something is actually limited. The diff --git a/src/backend/wayland/state/onboarding/first_run.rs b/src/backend/wayland/state/onboarding/first_run.rs index 0e587827..e6dd0e18 100644 --- a/src/backend/wayland/state/onboarding/first_run.rs +++ b/src/backend/wayland/state/onboarding/first_run.rs @@ -26,7 +26,7 @@ impl WaylandState { match crate::daemon::setup::setup_background_mode() { Ok(summary) => { mark_background_mode_prompt(self.onboarding.state_mut(), true); - self.onboarding.save(); + self.save_onboarding_state(); self.input_state.push_toast( ToastPriority::Info, "onboarding.first_run", @@ -38,7 +38,7 @@ impl WaylandState { } Err(err) => { mark_background_mode_prompt(self.onboarding.state_mut(), false); - self.onboarding.save(); + self.save_onboarding_state(); self.input_state.push_toast(ToastPriority::Critical, "onboarding.first_run", Toast::error(format!( "Background mode setup failed: {err}. You can set this up later in Background Mode settings." ))); @@ -46,7 +46,7 @@ impl WaylandState { } } else { mark_background_mode_prompt(self.onboarding.state_mut(), false); - self.onboarding.save(); + self.save_onboarding_state(); self.input_state.push_toast( ToastPriority::Info, "onboarding.first_run", @@ -73,7 +73,7 @@ impl WaylandState { state.first_run_completed = true; state.active_step = None; state.quick_access_requires_toolbar = false; - self.onboarding.save(); + self.save_onboarding_state(); self.input_state.push_toast( ToastPriority::Info, "onboarding.first_run", @@ -223,7 +223,12 @@ impl WaylandState { } fn first_run_onboarding_card_visible(&self) -> bool { - if !self.surface.is_configured() || self.overlay_suppressed() { + if !super::automatic_onboarding_allowed( + self.config.ui.show_onboarding_hints, + self.onboarding.persistence_available(), + ) || !self.surface.is_configured() + || self.overlay_suppressed() + { return false; } !first_run_card_hidden_by_ui_state( @@ -381,7 +386,7 @@ impl WaylandState { } if changed { - self.onboarding.save(); + self.save_onboarding_state(); } if first_run_ui_changed { self.input_state @@ -465,7 +470,7 @@ impl WaylandState { if state.quick_access_requires_toolbar { items.push(OnboardingChecklistItem { label: format!( - "Show toolbars ({})", + "Show toolbar ({})", self.shortcut_label(Action::ToggleToolbar, "Toggle toolbar") ), done: self.input_state.toolbar_visible() || state.used_toolbar_toggle, diff --git a/src/backend/wayland/state/onboarding/tests.rs b/src/backend/wayland/state/onboarding/tests.rs index c14e4390..da1122c2 100644 --- a/src/backend/wayland/state/onboarding/tests.rs +++ b/src/backend/wayland/state/onboarding/tests.rs @@ -4,8 +4,8 @@ use super::first_run::{ first_run_step_eyebrow, quick_access_completed, radial_flick_completed, shortcut_rebind_footer, }; use super::{ - canvas_popover_hint_relevant, capability_toast_message, shortcut_coach_should_fire, - status_bar_board_picker_entry, + automatic_onboarding_allowed, canvas_popover_hint_relevant, capability_toast_message, + shortcut_coach_should_fire, status_bar_board_picker_entry, }; use crate::config::{RadialMenuMouseBinding, ToolbarRebindModifier}; use crate::input::state::CompositorCapabilities; @@ -50,6 +50,14 @@ fn first_run_card_remains_visible_without_modal_states() { )); } +#[test] +fn automatic_onboarding_requires_the_preference_and_durable_progress() { + assert!(automatic_onboarding_allowed(true, true)); + assert!(!automatic_onboarding_allowed(false, true)); + assert!(!automatic_onboarding_allowed(true, false)); + assert!(!automatic_onboarding_allowed(false, false)); +} + #[test] fn first_run_eyebrow_shows_progress() { assert_eq!( diff --git a/src/config/action_meta/entries/ui.rs b/src/config/action_meta/entries/ui.rs index 52db4422..dd0e4728 100644 --- a/src/config/action_meta/entries/ui.rs +++ b/src/config/action_meta/entries/ui.rs @@ -278,6 +278,16 @@ pub const ENTRIES: &[ActionMeta] = &[ false, &["edit quick colors", "palette", "swatches", "color library"] ), + meta!( + OpenConfiguratorOnboardingHints, + "Tip Settings…", + Some("Tip Settings"), + "Open General UI at the automatic-guidance preference", + UI, + true, + false, + false + ), meta!( OpenAbout, "About Wayscriber", diff --git a/src/config/action_meta/tests.rs b/src/config/action_meta/tests.rs index f7f127b2..95634464 100644 --- a/src/config/action_meta/tests.rs +++ b/src/config/action_meta/tests.rs @@ -237,6 +237,7 @@ const EXPECTED_COMMAND_PALETTE_ACTIONS: &[Action] = &[ Action::OpenConfiguratorPresets, Action::OpenConfiguratorBoards, Action::OpenConfiguratorQuickColors, + Action::OpenConfiguratorOnboardingHints, Action::OpenAbout, Action::ClearSavedToolState, Action::ToggleCommandPalette, diff --git a/src/config/keybindings/config/map/edit.rs b/src/config/keybindings/config/map/edit.rs index 5065d158..62ca291b 100644 --- a/src/config/keybindings/config/map/edit.rs +++ b/src/config/keybindings/config/map/edit.rs @@ -222,6 +222,7 @@ define_action_binding_accessors! { OpenConfiguratorPresets, OpenConfiguratorBoards, OpenConfiguratorQuickColors, + OpenConfiguratorOnboardingHints, ] } diff --git a/src/config/tests/load.rs b/src/config/tests/load.rs index 7de71940..fb2967a1 100644 --- a/src/config/tests/load.rs +++ b/src/config/tests/load.rs @@ -54,6 +54,20 @@ fn ui_theme_defaults_to_auto_and_parses_explicit_values() { } } +#[test] +fn automatic_onboarding_hints_default_on_and_can_be_disabled() { + let default_config: Config = toml::from_str("").expect("empty config should use defaults"); + assert!(default_config.ui.show_onboarding_hints); + + let disabled: Config = toml::from_str("[ui]\nshow_onboarding_hints = false\n") + .expect("onboarding preference should parse"); + assert!(!disabled.ui.show_onboarding_hints); + + let serialized = toml::to_string(&disabled).expect("config should serialize"); + let reloaded: Config = toml::from_str(&serialized).expect("round trip should parse"); + assert!(!reloaded.ui.show_onboarding_hints); +} + #[test] fn presenter_toolbar_mode_defaults_to_hidden_and_round_trips() { let default_config: Config = toml::from_str("").expect("empty config should use defaults"); @@ -341,6 +355,35 @@ fn custom_f2_toggle_toolbar_binding_keeps_f2_and_unbinds_cycle() { }); } +#[test] +fn explicit_empty_cycle_records_legacy_f2_intent_without_a_startup_notice() { + with_temp_config_home(|config_root| { + let primary_dir = config_root.join(PRIMARY_CONFIG_DIR); + fs::create_dir_all(&primary_dir).unwrap(); + fs::write( + primary_dir.join("config.toml"), + format!( + "config_revision = {}\n\n[keybindings]\ntoggle_toolbar = ['F2', 'F9']\ncycle_toolbar_display = []\n", + CURRENT_CONFIG_REVISION + ), + ) + .unwrap(); + + let loaded = Config::load().expect("load succeeds"); + + assert_eq!(loaded.config.keybindings.ui.toggle_toolbar, ["F2", "F9"]); + assert!( + loaded + .config + .keybindings + .ui + .cycle_toolbar_display + .is_empty() + ); + assert!(loaded.validation.skipped_default_shortcuts.is_empty()); + }); +} + /// The authored side of the #293 case survives a save of one of its own /// shortcuts and every load after it. #[test] diff --git a/src/config/tests/migration.rs b/src/config/tests/migration.rs index b32741d7..4cd2c9c7 100644 --- a/src/config/tests/migration.rs +++ b/src/config/tests/migration.rs @@ -116,6 +116,19 @@ fn revision_one_proposes_the_toolbar_f2_split() { assert_eq!(preview.changes()[0].after(), ["F9"]); } +#[test] +fn revision_one_preserves_custom_f2_and_proposes_an_unbound_cycle_action() { + let mut config = config_at_revision(1); + config.keybindings.ui.toggle_toolbar = vec!["F2".to_string()]; + + let preview = MigrationPreview::for_authored_config(&config) + .expect("custom F2 needs an explicit opt-out"); + + assert_eq!(proposed_keys(&preview), ["cycle_toolbar_display"]); + assert_eq!(preview.changes()[0].before(), ["F2"]); + assert!(preview.changes()[0].after().is_empty()); +} + /// A revision-2 file settled the earlier steps already, so the only recipe /// left is the one revision 3 introduced. #[test] diff --git a/src/config/types/ui.rs b/src/config/types/ui.rs index bfef0db2..253e1645 100644 --- a/src/config/types/ui.rs +++ b/src/config/types/ui.rs @@ -116,6 +116,11 @@ pub struct UiConfig { #[serde(default = "default_show_capabilities_warning")] pub show_capabilities_warning: bool, + /// Show automatic first-run guidance, discovery tips, and shortcut coaching. + /// The guided tour remains available manually when this is disabled. + #[serde(default = "default_show_onboarding_hints")] + pub show_onboarding_hints: bool, + /// Preferred output name for the xdg-shell fallback overlay (GNOME). /// Falls back to last entered output or first available. #[serde(default)] @@ -193,6 +198,7 @@ impl Default for UiConfig { help_overlay_style: HelpOverlayStyle::default(), help_overlay_context_filter: default_help_overlay_context_filter(), show_capabilities_warning: default_show_capabilities_warning(), + show_onboarding_hints: default_show_onboarding_hints(), preferred_output: None, multi_monitor_enabled: default_multi_monitor_enabled(), active_output_badge: default_active_output_badge(), @@ -311,6 +317,10 @@ fn default_show_capabilities_warning() -> bool { true } +fn default_show_onboarding_hints() -> bool { + true +} + fn default_command_palette_toast_duration_ms() -> u64 { 1500 } diff --git a/src/configurator_destination.rs b/src/configurator_destination.rs index cfc517c8..95fdc45b 100644 --- a/src/configurator_destination.rs +++ b/src/configurator_destination.rs @@ -36,11 +36,22 @@ const SEARCH_KEY: &str = "search="; /// tab's color search terms. const QUICK_COLORS_SEARCH: &str = "Quick Colors"; +/// The visible General UI label for automatic onboarding and discovery hints. +/// +/// Keeping the launcher term identical to the control label makes the +/// Configurator reveal the owning section without a separate scroll protocol. +const ONBOARDING_HINTS_SEARCH: &str = "Show automatic guidance and tips"; + /// The Drawing screen, focused on the quick-color palette. pub fn quick_colors_destination() -> ConfiguratorDestination { ConfiguratorDestination::with_search(ConfiguratorScreen::Drawing, QUICK_COLORS_SEARCH) } +/// The General UI section, filtered to the automatic-guidance preference. +pub fn onboarding_hints_destination() -> ConfiguratorDestination { + ConfiguratorDestination::with_search(ConfiguratorScreen::UiToolbar, ONBOARDING_HINTS_SEARCH) +} + /// A keybinding subtab, mirroring the configurator's own Keybindings sections. /// /// Duplicated rather than shared because the configurator's tab types are @@ -233,7 +244,8 @@ pub fn keybindings_section_for_action(action: Action) -> Option return None, + | Action::OpenConfiguratorQuickColors + | Action::OpenConfiguratorOnboardingHints => return None, }; Some(section) } @@ -615,6 +627,7 @@ mod tests { Action::OpenConfiguratorPresets, Action::OpenConfiguratorBoards, Action::OpenConfiguratorQuickColors, + Action::OpenConfiguratorOnboardingHints, ] { assert_eq!( keybindings_section_for_action(action), @@ -673,6 +686,14 @@ mod tests { ); } + #[test] + fn the_onboarding_hint_route_searches_the_general_ui_setting() { + assert_eq!( + onboarding_hints_destination().as_arg(), + "ui/toolbar?search=Show automatic guidance and tips" + ); + } + #[test] fn launch_arguments_are_empty_without_a_destination() { assert!(configurator_launch_arguments(None).is_empty()); diff --git a/src/domain/action.rs b/src/domain/action.rs index d4962f32..9841283a 100644 --- a/src/domain/action.rs +++ b/src/domain/action.rs @@ -150,6 +150,8 @@ pub enum Action { OpenConfiguratorBoards, /// Open the configurator's Drawing screen at the quick-color palette. OpenConfiguratorQuickColors, + /// Open General UI at the automatic-guidance preference. + OpenConfiguratorOnboardingHints, ClearSavedToolState, OpenAbout, diff --git a/src/domain/tests.rs b/src/domain/tests.rs index 86331c46..cdb5cc42 100644 --- a/src/domain/tests.rs +++ b/src/domain/tests.rs @@ -172,6 +172,10 @@ fn action_serialization_matches_established_contract() { Action::OpenConfiguratorQuickColors, "open_configurator_quick_colors", ), + ( + Action::OpenConfiguratorOnboardingHints, + "open_configurator_onboarding_hints", + ), (Action::OpenAbout, "open_about"), (Action::ClearSavedToolState, "clear_saved_tool_state"), (Action::SetColorRed, "set_color_red"), diff --git a/src/input/events.rs b/src/input/events.rs index b58f6817..4c858422 100644 --- a/src/input/events.rs +++ b/src/input/events.rs @@ -47,7 +47,7 @@ pub enum Key { Menu, /// F1 function key (help) F1, - /// F2 function key (toolbar toggle) + /// F2 function key (toolbar display cycle) F2, /// F3 function key F3, diff --git a/src/input/state/actions/action_ui.rs b/src/input/state/actions/action_ui.rs index 6c6135df..e4172e25 100644 --- a/src/input/state/actions/action_ui.rs +++ b/src/input/state/actions/action_ui.rs @@ -1,5 +1,6 @@ use crate::configurator_destination::{ - ConfiguratorDestination, ConfiguratorScreen, quick_colors_destination, + ConfiguratorDestination, ConfiguratorScreen, onboarding_hints_destination, + quick_colors_destination, }; use crate::domain::Action; use crate::input::state::{Toast, ToastPriority}; @@ -321,6 +322,10 @@ impl InputState { self.launch_configurator(Some(quick_colors_destination())); true } + Action::OpenConfiguratorOnboardingHints => { + self.launch_configurator(Some(onboarding_hints_destination())); + true + } Action::OpenAbout => { self.launch_about(); true diff --git a/src/input/state/interaction/actions.rs b/src/input/state/interaction/actions.rs index 78655524..0687efa8 100644 --- a/src/input/state/interaction/actions.rs +++ b/src/input/state/interaction/actions.rs @@ -111,6 +111,7 @@ pub(crate) fn classify_action(action: Action) -> ActionRoute { | Action::OpenConfiguratorPresets | Action::OpenConfiguratorBoards | Action::OpenConfiguratorQuickColors + | Action::OpenConfiguratorOnboardingHints | Action::OpenAbout | Action::ClearSavedToolState | Action::OpenCaptureFolder diff --git a/src/onboarding.rs b/src/onboarding.rs index ef01d6d6..4638ae35 100644 --- a/src/onboarding.rs +++ b/src/onboarding.rs @@ -7,7 +7,8 @@ use std::io::ErrorKind; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; -const ONBOARDING_VERSION: u32 = 5; +const ONBOARDING_VERSION: u32 = 6; +const STARTUP_NOTICE_ACKNOWLEDGEMENT_MAX: usize = 32; pub(crate) const DRAWER_HINT_MAX: u32 = 2; pub(crate) const DEFERRED_HINT_REPEAT_MAX: u32 = 3; const ONBOARDING_FILE: &str = "onboarding.toml"; @@ -154,6 +155,11 @@ pub struct OnboardingState { /// Number of deferred Canvas-popover hints shown across sessions (M9). #[serde(default)] pub hint_canvas_popover_count: u32, + /// Stable content identifiers for informational startup notices the user + /// has already seen. Errors and authored conflicts are deliberately not + /// stored here because they must remain visible until resolved. + #[serde(default)] + pub acknowledged_startup_notices: Vec, } impl Default for OnboardingState { @@ -201,6 +207,7 @@ impl Default for OnboardingState { hint_zoom_chip_count: 0, hint_canvas_popover_shown: false, hint_canvas_popover_count: 0, + acknowledged_startup_notices: Vec::new(), } } } @@ -214,14 +221,53 @@ impl OnboardingState { pub struct OnboardingStore { state: OnboardingState, path: Option, + persistence_available: bool, +} + +#[derive(Debug)] +pub(crate) enum OnboardingSaveError { + Unavailable, + CreateDirectory { + path: PathBuf, + source: std::io::Error, + }, + Serialize(toml::ser::Error), + Write { + path: PathBuf, + source: crate::durable_io::DurableIoError, + }, +} + +impl std::fmt::Display for OnboardingSaveError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Unavailable => formatter.write_str("no user data directory is available"), + Self::CreateDirectory { path, source } => write!( + formatter, + "failed to create onboarding state directory {}: {source}", + path.display() + ), + Self::Serialize(source) => { + write!(formatter, "failed to serialize onboarding state: {source}") + } + Self::Write { path, source } => write!( + formatter, + "failed to write onboarding state {}: {source}", + path.display() + ), + } + } } +impl std::error::Error for OnboardingSaveError {} + impl OnboardingStore { pub fn load() -> Self { let Some(path) = onboarding_path() else { return Self { state: OnboardingState::default(), path: None, + persistence_available: false, }; }; @@ -233,12 +279,13 @@ impl OnboardingStore { Ok(raw) => match toml::from_str::(&raw) { Ok(mut state) => { let needs_save = migrate_onboarding_state(&mut state); - let store = Self { + let mut store = Self { state, path: Some(path), + persistence_available: true, }; if needs_save { - store.save(); + let _ = store.save(); } return store; } @@ -248,11 +295,7 @@ impl OnboardingStore { path.display(), err ); - let state = recover_onboarding_file(&path, Some(&raw)); - return Self { - state, - path: Some(path), - }; + return recover_onboarding_file(path, Some(&raw)); } }, Err(err) if err.kind() == ErrorKind::NotFound => {} @@ -262,17 +305,14 @@ impl OnboardingStore { path.display(), err ); - let state = recover_onboarding_file(&path, None); - return Self { - state, - path: Some(path), - }; + return recover_onboarding_file(path, None); } } Self { state: OnboardingState::default(), path: Some(path), + persistence_available: true, } } @@ -284,44 +324,129 @@ impl OnboardingStore { &mut self.state } - pub fn save(&self) { + pub fn persistence_available(&self) -> bool { + self.persistence_available + } + + pub fn startup_notice_acknowledged(&self, notice_id: &str) -> bool { + self.state + .acknowledged_startup_notices + .iter() + .any(|saved| saved == notice_id) + } + + pub fn acknowledge_startup_notice( + &mut self, + notice_id: &str, + ) -> Result<(), OnboardingSaveError> { + if !self.startup_notice_acknowledged(notice_id) { + self.state + .acknowledged_startup_notices + .push(notice_id.to_string()); + let excess = self + .state + .acknowledged_startup_notices + .len() + .saturating_sub(STARTUP_NOTICE_ACKNOWLEDGEMENT_MAX); + if excess > 0 { + self.state.acknowledged_startup_notices.drain(..excess); + } + } + self.save() + } + + pub fn begin_session( + &mut self, + automatic_guidance_enabled: bool, + ) -> Result<(), OnboardingSaveError> { + let state = &mut self.state; + state.sessions_seen = state.sessions_seen.saturating_add(1); + + if automatic_guidance_enabled { + if !state.used_help_overlay && state.hint_help_count < DEFERRED_HINT_REPEAT_MAX { + state.hint_help_shown = false; + } + if !state.used_command_palette && state.hint_palette_count < DEFERRED_HINT_REPEAT_MAX { + state.hint_palette_shown = false; + } + if !state.used_radial_menu + && !state.used_context_menu_right_click + && !state.used_context_menu_keyboard + && state.hint_quick_access_count < DEFERRED_HINT_REPEAT_MAX + { + state.hint_quick_access_shown = false; + } + if state.hint_status_bar_count < DEFERRED_HINT_REPEAT_MAX { + state.hint_status_bar_shown = false; + } + if state.hint_zoom_chip_count < DEFERRED_HINT_REPEAT_MAX { + state.hint_zoom_chip_shown = false; + } + if state.hint_canvas_popover_count < DEFERRED_HINT_REPEAT_MAX { + state.hint_canvas_popover_shown = false; + } + } + + if automatic_guidance_enabled && !state.first_run_completed && !state.first_run_skipped { + state + .active_step + .get_or_insert(FirstRunStep::BackgroundModeSetup); + } else { + state.active_step = None; + state.quick_access_requires_toolbar = false; + } + // Keep legacy flags marked so older checks never re-trigger. + state.welcome_shown = true; + state.tour_shown = true; + self.save() + } + + pub fn save(&mut self) -> Result<(), OnboardingSaveError> { let Some(path) = &self.path else { - return; + self.persistence_available = false; + return Err(OnboardingSaveError::Unavailable); }; if let Some(parent) = path.parent() - && let Err(err) = fs::create_dir_all(parent) + && let Err(source) = fs::create_dir_all(parent) { - warn!( - "Failed to create onboarding state dir {}: {}", - parent.display(), - err - ); - return; + let error = OnboardingSaveError::CreateDirectory { + path: parent.to_path_buf(), + source, + }; + warn!("{error}"); + self.persistence_available = false; + return Err(error); } - match toml::to_string_pretty(&self.state) { - Ok(contents) => { - if let Err(err) = crate::durable_io::write_text_atomic( - path, - &contents, - AtomicWriteOptions { - overwrite: OverwriteMode::Replace, - permissions: PermissionPolicy::PreserveExistingOrMode(0o644), - symlink: SymlinkPolicy::Reject, - sync_file: true, - sync_parent: true, - }, - ) { - warn!( - "Failed to write onboarding state {}: {}", - path.display(), - err - ); - } - } - Err(err) => { - warn!("Failed to serialize onboarding state: {}", err); + let contents = match toml::to_string_pretty(&self.state) { + Ok(contents) => contents, + Err(source) => { + let error = OnboardingSaveError::Serialize(source); + warn!("{error}"); + self.persistence_available = false; + return Err(error); } + }; + if let Err(source) = crate::durable_io::write_text_atomic( + path, + &contents, + AtomicWriteOptions { + overwrite: OverwriteMode::Replace, + permissions: PermissionPolicy::PreserveExistingOrMode(0o644), + symlink: SymlinkPolicy::Reject, + sync_file: true, + sync_parent: true, + }, + ) { + let error = OnboardingSaveError::Write { + path: path.clone(), + source, + }; + warn!("{error}"); + self.persistence_available = false; + return Err(error); } + self.persistence_available = true; + Ok(()) } } @@ -374,6 +499,20 @@ fn migrate_onboarding_state(state: &mut OnboardingState) -> bool { state.first_run_background_mode_prompted = true; needs_save = true; } + if old_version < 6 && state.first_run_completed { + // These surface-discovery hints were added after many profiles had + // already completed onboarding. A version bump must not silently + // enroll those users in several new rounds of automatic tips. + state.hint_status_bar_shown = true; + state.hint_status_bar_count = state.hint_status_bar_count.max(DEFERRED_HINT_REPEAT_MAX); + state.hint_zoom_chip_shown = true; + state.hint_zoom_chip_count = state.hint_zoom_chip_count.max(DEFERRED_HINT_REPEAT_MAX); + state.hint_canvas_popover_shown = true; + state.hint_canvas_popover_count = state + .hint_canvas_popover_count + .max(DEFERRED_HINT_REPEAT_MAX); + needs_save = true; + } if state.quick_access_requires_toolbar && state.active_step != Some(FirstRunStep::QuickAccess) { state.quick_access_requires_toolbar = false; needs_save = true; @@ -420,10 +559,10 @@ fn migrate_onboarding_state(state: &mut OnboardingState) -> bool { needs_save } -fn recover_onboarding_file(path: &Path, _raw: Option<&str>) -> OnboardingState { +fn recover_onboarding_file(path: PathBuf, _raw: Option<&str>) -> OnboardingStore { if path.exists() { - let backup = backup_path(path); - if let Err(err) = fs::rename(path, &backup) { + let backup = backup_path(&path); + if let Err(err) = fs::rename(&path, &backup) { warn!( "Failed to back up onboarding state {}: {}", path.display(), @@ -478,13 +617,15 @@ fn recover_onboarding_file(path: &Path, _raw: Option<&str>) -> OnboardingState { hint_zoom_chip_count: DEFERRED_HINT_REPEAT_MAX, hint_canvas_popover_shown: true, hint_canvas_popover_count: DEFERRED_HINT_REPEAT_MAX, + acknowledged_startup_notices: Vec::new(), }; - let store = OnboardingStore { - state: state.clone(), - path: Some(path.to_path_buf()), + let mut store = OnboardingStore { + state, + path: Some(path), + persistence_available: true, }; - store.save(); - state + let _ = store.save(); + store } fn backup_path(path: &Path) -> PathBuf { diff --git a/src/onboarding/tests.rs b/src/onboarding/tests.rs index 0aaff1ea..154ed664 100644 --- a/src/onboarding/tests.rs +++ b/src/onboarding/tests.rs @@ -5,13 +5,13 @@ use std::fs; fn onboarding_defaults_when_missing() { let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); let path = tmp.path().join(ONBOARDING_DIR).join(ONBOARDING_FILE); - let store = OnboardingStore::load_from_path(path.clone()); + let mut store = OnboardingStore::load_from_path(path.clone()); assert!(!store.state().welcome_shown); assert!(!store.state().toolbar_hint_shown); assert!(!store.state().first_run_completed); assert!(store.state().active_step.is_none()); - store.save(); + store.save().expect("default state should persist"); assert!(path.exists()); } @@ -23,7 +23,7 @@ fn onboarding_persists_flags() { store.state_mut().welcome_shown = true; store.state_mut().toolbar_hint_shown = true; store.state_mut().used_help_overlay = true; - store.save(); + store.save().expect("updated state should persist"); let reloaded = OnboardingStore::load_from_path(path.clone()); assert!(reloaded.state().welcome_shown); @@ -31,6 +31,114 @@ fn onboarding_persists_flags() { assert!(reloaded.state().used_help_overlay); } +#[cfg(unix)] +#[test] +fn rejected_persistence_disables_automatic_onboarding_for_the_session() { + use std::os::unix::fs::symlink; + + let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); + let target = tmp.path().join("real-onboarding.toml"); + fs::write(&target, "version = 5\n").expect("seed should be writable"); + let path = tmp.path().join(ONBOARDING_DIR).join(ONBOARDING_FILE); + fs::create_dir_all(path.parent().expect("onboarding path has a parent")) + .expect("state directory should be writable"); + symlink(&target, &path).expect("test symlink should be created"); + + let mut store = OnboardingStore::load_from_path(path); + store.state_mut().first_run_completed = true; + + assert!( + store.save().is_err(), + "symlink rejection must reach the caller" + ); + assert!( + !store.persistence_available(), + "a process that cannot remember an acknowledgement must not show automatic onboarding" + ); +} + +#[test] +fn unwritable_state_location_disables_automatic_onboarding_for_the_session() { + let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); + let blocker = tmp.path().join("not-a-directory"); + fs::write(&blocker, "occupied").expect("blocker file should be created"); + let path = blocker.join(ONBOARDING_FILE); + let mut store = OnboardingStore::load_from_path(path); + + assert!( + store.begin_session(true).is_err(), + "a state path below a regular file cannot be persisted" + ); + assert!(!store.persistence_available()); + assert!( + !store.state().first_run_active(), + "an unreadable state location must recover suppressively as well as disabling hints" + ); +} + +#[test] +fn startup_notice_acknowledgement_survives_reload_and_is_content_specific() { + let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); + let path = tmp.path().join(ONBOARDING_DIR).join(ONBOARDING_FILE); + let mut store = OnboardingStore::load_from_path(path.clone()); + + assert!(!store.startup_notice_acknowledged("skipped-default:f2-cycle")); + store + .acknowledge_startup_notice("skipped-default:f2-cycle") + .expect("acknowledgement should persist"); + + let reloaded = OnboardingStore::load_from_path(path); + assert!(reloaded.startup_notice_acknowledged("skipped-default:f2-cycle")); + assert!( + !reloaded.startup_notice_acknowledged("skipped-default:new-binding"), + "a changed diagnostic must be eligible for one new notice" + ); +} + +#[test] +fn deferred_hint_cap_survives_repeated_process_launches() { + let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); + let path = tmp.path().join(ONBOARDING_DIR).join(ONBOARDING_FILE); + + for expected_count in 0..DEFERRED_HINT_REPEAT_MAX { + let mut store = OnboardingStore::load_from_path(path.clone()); + store.state_mut().first_run_completed = true; + store + .begin_session(true) + .expect("session state should persist"); + assert!(!store.state().hint_zoom_chip_shown); + assert_eq!(store.state().hint_zoom_chip_count, expected_count); + + store.state_mut().hint_zoom_chip_shown = true; + store.state_mut().hint_zoom_chip_count += 1; + store.save().expect("shown hint should persist"); + } + + let mut capped = OnboardingStore::load_from_path(path); + capped + .begin_session(true) + .expect("capped state should remain writable"); + assert!(capped.state().hint_zoom_chip_shown); + assert_eq!( + capped.state().hint_zoom_chip_count, + DEFERRED_HINT_REPEAT_MAX + ); +} + +#[test] +fn disabled_automatic_guidance_does_not_activate_first_run() { + let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); + let path = tmp.path().join(ONBOARDING_DIR).join(ONBOARDING_FILE); + let mut store = OnboardingStore::load_from_path(path); + + store + .begin_session(false) + .expect("disabled preference should still persist session state"); + + assert!(store.state().active_step.is_none()); + assert!(!store.state().first_run_completed); +} + #[test] fn onboarding_recovers_from_parse_error() { let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); @@ -124,17 +232,17 @@ used_command_palette = true } #[test] -fn v4_file_migrates_to_v5_with_m9_fields_defaulted() { +fn completed_pre_v6_profile_does_not_receive_new_surface_tips() { let tmp = crate::test_temp::tempdir().expect("tempdir should succeed"); let path = tmp.path().join(ONBOARDING_DIR).join(ONBOARDING_FILE); if let Some(parent) = path.parent() { fs::create_dir_all(parent).expect("create onboarding dir"); } - // A v4 file that finished onboarding and has the coach fields but none of - // the M9 surface-hint / deprecation fields. Migration must bump the version - // to 5 and default the new fields (serde defaults) without re-running setup. + // A v5 file is the state written by the release that introduced the M9 + // surface hints. Upgrading must preserve the user's completed-onboarding + // expectation even when those fields were never durably advanced. let seed = "\ -version = 4 +version = 5 welcome_shown = true toolbar_hint_shown = true first_run_completed = true @@ -143,26 +251,34 @@ used_help_overlay = true used_command_palette = true coach_hint_count = 1 "; - fs::write(&path, seed).expect("write v4 seed"); + fs::write(&path, seed).expect("write v5 seed"); let store = OnboardingStore::load_from_path(path.clone()); assert_eq!(store.state().version, ONBOARDING_VERSION); - assert_eq!(ONBOARDING_VERSION, 5); + assert_eq!(ONBOARDING_VERSION, 6); assert!(store.state().first_run_completed); - // New M9 fields default off/zero — the migration fabricates no progress. - assert!(!store.state().hint_status_bar_shown); - assert_eq!(store.state().hint_status_bar_count, 0); - assert!(!store.state().hint_zoom_chip_shown); - assert_eq!(store.state().hint_zoom_chip_count, 0); - assert!(!store.state().hint_canvas_popover_shown); - assert_eq!(store.state().hint_canvas_popover_count, 0); + assert!(store.state().hint_status_bar_shown); + assert_eq!( + store.state().hint_status_bar_count, + DEFERRED_HINT_REPEAT_MAX + ); + assert!(store.state().hint_zoom_chip_shown); + assert_eq!(store.state().hint_zoom_chip_count, DEFERRED_HINT_REPEAT_MAX); + assert!(store.state().hint_canvas_popover_shown); + assert_eq!( + store.state().hint_canvas_popover_count, + DEFERRED_HINT_REPEAT_MAX + ); // Prior coach bookkeeping is preserved across the bump. assert_eq!(store.state().coach_hint_count, 1); // The bumped file round-trips through a reload unchanged. let reloaded = OnboardingStore::load_from_path(path); assert_eq!(reloaded.state().version, ONBOARDING_VERSION); - assert_eq!(reloaded.state().hint_status_bar_count, 0); + assert_eq!( + reloaded.state().hint_status_bar_count, + DEFERRED_HINT_REPEAT_MAX + ); } #[test] @@ -173,7 +289,7 @@ fn m9_surface_hint_fields_persist_and_reconcile() { store.state_mut().hint_status_bar_count = 2; store.state_mut().hint_zoom_chip_count = 1; store.state_mut().hint_canvas_popover_count = 3; - store.save(); + store.save().expect("surface hint counters should persist"); let reloaded = OnboardingStore::load_from_path(path.clone()); assert_eq!(reloaded.state().hint_status_bar_count, 2);