diff --git a/README.md b/README.md index 4cd4a395a..a53e997ba 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A ZoomIt-like real-time screen annotation tool for Linux/Wayland, written in Rust. -Draw over any app, present with callouts and zoom, keep your boards between sessions — all from a lightweight daemon you toggle with one keybind. +Draw over any app, present with callouts and zoom, keep your boards between sessions. Toggle the lightweight daemon with one keybind. **Docs:** https://wayscriber.com/docs/ @@ -66,7 +66,7 @@ https://github.com/user-attachments/assets/4b5ed159-8d1c-44cb-8fe4-e0f2ea41d818 ## Why wayscriber? - **Annotate live** over any app without disrupting your workflow -- **Professional presentation tools** — presenter mode, numbered callouts, click highlights, screen freeze, zoom +- **Professional presentation tools**: presenter mode, numbered callouts, click highlights, screen freeze, zoom - **Persistent sessions** that survive restarts - **Native Wayland performance** with ZoomIt-like controls - **Lightweight daemon** with instant toggle via keybind @@ -79,7 +79,7 @@ https://github.com/user-attachments/assets/4b5ed159-8d1c-44cb-8fe4-e0f2ea41d818 | GNOME | ⚠️ Partial | Normal overlay and Freeze via portal when available; [light passthrough](#light-passthrough-mode) unavailable | | X11 | ❌ | Not supported | -The v0.9.23+ prebuilt `wayscriber` packages require glibc 2.39 and GTK 4.12 — see the notes in [Debian and Ubuntu](#debian-and-ubuntu) and [Fedora and RHEL](#fedora-and-rhel). The AUR packages and Nix are unaffected. +The v0.9.23+ prebuilt `wayscriber` packages require glibc 2.39 and GTK 4.12. See the notes in [Debian and Ubuntu](#debian-and-ubuntu) and [Fedora and RHEL](#fedora-and-rhel). The AUR packages and Nix are unaffected.
Tested environments @@ -143,7 +143,7 @@ The v0.9.23+ prebuilt `wayscriber` packages require glibc 2.39 and GTK 4.12 — - Full-screen saves, active-window grabs, region capture - Copy to clipboard or save to file - Uses `grim`, `slurp`, `wl-clipboard` (installed automatically by deb/rpm/AUR packages; fallback: xdg-desktop-portal) -- Copy text from screen (OCR): Ctrl+Shift+X, then drag a region of the shown desktop — or Ctrl+A for all of it — and get its text on the clipboard (needs `tesseract`) +- Copy text from screen (OCR): press Ctrl+Shift+X. Drag a desktop region, or press Ctrl+A for the whole desktop. The recognized text goes to the clipboard. Requires `tesseract`. ### Sessions and persistence - Session persistence is enabled by default for boards, undo/redo history, and tool state @@ -160,7 +160,7 @@ The v0.9.23+ prebuilt `wayscriber` packages require glibc 2.39 and GTK 4.12 — - Status bar with independently configurable output, selection, board, page, color, tool, size, context, toolbar-hint, Help, and About/version items - Help overlay (F1), quick reference (Shift+F1) - Command palette (Ctrl+K or Ctrl+Shift+P) -- Search, run, edit, unbind, or reset action shortcuts from the command palette; hold Ctrl+Shift while clicking a bindable toolbar control for direct shortcut capture (the modifier chord is configurable). An accepted shortcut is written straight back to `config.toml` — only that action's entry, with a timestamped `.bak` — and Ctrl+Shift+E on a palette row opens the same shortcut in the configurator +- Search, run, edit, unbind, or reset action shortcuts from the command palette. Hold Ctrl+Shift and click a bindable toolbar control to record its shortcut. You can configure this modifier chord. Wayscriber saves only that action's entry to `config.toml` and creates a timestamped `.bak` backup. Press Ctrl+Shift+E on a palette row to open the same shortcut in the configurator ### Multi-monitor - Move overlay focus between monitors: Ctrl+Alt+Shift+←/Ctrl+Alt+Shift+→ @@ -177,8 +177,8 @@ The v0.9.23+ prebuilt `wayscriber` packages require glibc 2.39 and GTK 4.12 — - Click highlights with configurable colors/radius/duration - Persistent ring while the click highlight tool is active - Presenter mode (Ctrl+Shift+M): hides UI, forces click highlights -- Input HUD (Ctrl+Shift+K): on-screen keystroke and click chips for demos and screencasts (opt-in system-wide capture via the `input-monitor` build feature — see [docs/CONFIG.md](docs/CONFIG.md#uiinput_hud---input-hud-keystrokes-and-clicks)) -- Light passthrough (layer-shell): draw while input passes through to the app underneath — see [Light passthrough mode](#light-passthrough-mode) +- Input HUD (Ctrl+Shift+K): on-screen keystroke and click chips for demos and screencasts (optional system-wide capture through the `input-monitor` build feature). See [docs/CONFIG.md](docs/CONFIG.md#uiinput_hud---input-hud-keystrokes-and-clicks) +- Light passthrough (layer-shell): draw while input passes through to the app underneath. See [Light passthrough mode](#light-passthrough-mode) - Screen freeze (Ctrl+Shift+F): pause the display while apps keep running. Freeze prefers compositor-native `wlr-screencopy` or `ext-image-copy-capture` and falls back to the screenshot portal when available - Spotlight: drag an ellipse to dim everything around it; stack several to highlight multiple areas. Each Spotlight can magnify its opening from 1× to 4×, while dim strength and edge softness remain shared under `[spotlight]`. Magnification uses complete pixels from a solid board, Freeze, Zoom, or a captured/export backdrop; a live transparent board keeps the ordinary opening and prompts you to Freeze. @@ -201,7 +201,7 @@ Pick the path that matches your setup: | Fast CLI install with auto-updates on Debian/Ubuntu/Mint/Pop!_OS | [Debian and Ubuntu](#debian-and-ubuntu) | | Fast CLI install with auto-updates on Fedora/Nobara or RHEL/Rocky/Alma 10+ | [Fedora and RHEL](#fedora-and-rhel) | | Arch, Manjaro, CachyOS, or another Arch-based distro | [AUR or direct release installer](#arch-linux-aur); prefer `wayscriber-bin` when the AUR is available | -| NixOS, or the Nix package manager on another distro | [NixOS and Nix](#nixos-and-nix) — `nixpkgs` for the standard install, the project flake for the newest release and the Configurator | +| NixOS, or the Nix package manager on another distro | [NixOS and Nix](#nixos-and-nix): `nixpkgs` for the standard install, the project flake for the newest release and the Configurator | | One-off package (browser or terminal) without adding a repo | [GitHub Releases](#github-releases-one-off) | | Hacking on wayscriber or building a local binary | [From source](#from-source) | @@ -329,7 +329,7 @@ Wayscriber is packaged in [`nixpkgs`](https://search.nixos.org/packages?query=wa If you are on a stable NixOS channel and want the newest release, use [the project flake](#latest-release-and-configurator) rather than waiting for `nixpkgs`. -**NixOS, from nixpkgs** — add Wayscriber to `configuration.nix` or another NixOS module: +**NixOS, from nixpkgs**. Add Wayscriber to `configuration.nix` or another NixOS module: ```nix { pkgs, ... }: @@ -414,7 +414,7 @@ nix flake update wayscriber sudo nixos-rebuild switch --flake .#myhost ``` -**Nix on another Linux distro** — when running the Nix package manager on Ubuntu, Fedora, or elsewhere, a user profile install is reasonable: +**Nix on another Linux distro**. If you use Nix on Ubuntu, Fedora, or another Linux distribution, you can install it in your user profile: ```bash nix profile install nixpkgs#wayscriber ``` @@ -434,14 +434,14 @@ nix develop github:devmobasa/wayscriber ### GitHub Releases (one-off) -Install the release package directly if you prefer not to add a repo — these are one-off installs with no auto-updates. +If you prefer not to add a repository, install the release package directly. These packages do not update automatically. In a browser: 1. Open the [latest release](https://github.com/devmobasa/wayscriber/releases/latest). 2. Install the main app package that matches your distro: - - [wayscriber-amd64.deb](https://github.com/devmobasa/wayscriber/releases/latest/download/wayscriber-amd64.deb) — v0.9.23+ supports Ubuntu 24.04+, Linux Mint 22+, Pop!_OS 24.04+, Debian 13+, and compatible newer Debian-based distros (see the [release requirement](#debian-and-ubuntu)) - - [wayscriber-x86_64.rpm](https://github.com/devmobasa/wayscriber/releases/latest/download/wayscriber-x86_64.rpm) — v0.9.23+ supports compatible Fedora/Nobara releases and RHEL/Rocky/Alma 10+ (see the [release requirement](#fedora-and-rhel)) + - [wayscriber-amd64.deb](https://github.com/devmobasa/wayscriber/releases/latest/download/wayscriber-amd64.deb): v0.9.23+ supports Ubuntu 24.04+, Linux Mint 22+, Pop!_OS 24.04+, Debian 13+, and compatible newer Debian-based distros (see the [release requirement](#debian-and-ubuntu)) + - [wayscriber-x86_64.rpm](https://github.com/devmobasa/wayscriber/releases/latest/download/wayscriber-x86_64.rpm): v0.9.23+ supports compatible Fedora/Nobara releases and RHEL/Rocky/Alma 10+ (see the [release requirement](#fedora-and-rhel)) 3. Optional: install the configurator package after wayscriber: - [wayscriber-configurator-amd64.deb](https://github.com/devmobasa/wayscriber/releases/latest/download/wayscriber-configurator-amd64.deb) - [wayscriber-configurator-x86_64.rpm](https://github.com/devmobasa/wayscriber/releases/latest/download/wayscriber-configurator-x86_64.rpm) @@ -487,7 +487,7 @@ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh source "$HOME/.cargo/env" ``` -Then clone the repository — the dependency and build steps below run from inside it: +Clone the repository. Run the dependency and build commands from its directory: ```bash git clone https://github.com/devmobasa/wayscriber.git @@ -559,8 +559,7 @@ sudo dnf install wl-clipboard grim slurp # Fedora `Copy text from screen` (Ctrl+Shift+X) recognizes the text in a dragged screen region and copies it to the clipboard; Ctrl+A inside the selector reads the whole screen instead. A band sweeps the region while -Tesseract runs, and a card reports the outcome — never the recognized text, -which goes only to the clipboard. Its toolbar button is hidden until you turn it +Tesseract runs. A card reports the outcome. The recognized text goes only to the clipboard. Its toolbar button is hidden until you turn it on. Install Tesseract and the language data you configure in `[capture].ocr_languages` (default `eng`): @@ -584,15 +583,15 @@ wayscriber --version wayscriber --active ``` -(If you used `nix run` instead of installing, `wayscriber` is not on your `PATH` — keep using the `nix run` command from [Installation](#nixos-and-nix).) +(If you used `nix run` instead of installing, `wayscriber` is not on your `PATH`. Keep using the `nix run` command from [Installation](#nixos-and-nix).) Once the overlay is up: -- F1 or F10 — help overlay -- Shift+F1 — quick reference -- Ctrl+K / Ctrl+Shift+P — command palette -- F11 — [configurator](#configurator-gui) -- Escape — hide or exit +- F1 or F10: help overlay +- Shift+F1: quick reference +- Ctrl+K / Ctrl+Shift+P: command palette +- F11: [configurator](#configurator-gui) +- Escape: hide or exit Discovery and shortcut-coaching tips have **Got it** and **Tip settings…** controls. **Got it** @@ -678,7 +677,7 @@ on shorter or scaled displays: Supported desktops use a theme-adaptive symbolic tray icon. Hosts that do not reliably resolve named icons (including Omarchy/Quickshell, Noctalia/Quickshell, and COSMIC) automatically receive scale-aware colored pixmaps, including a 48px HiDPI rendition. Set `[tray].icon_style` to `"auto"` (default), `"symbolic"`, or `"colored"` to choose the main tray icon style; restart the daemon after changing it. Use `--no-tray` or `WAYSCRIBER_NO_TRAY=1` if you don't have a system tray. If the tray icon is still blank or the menu shows square placeholders, start the daemon with `WAYSCRIBER_TRAY_FORCE_PIXMAP=1`; this environment override takes precedence over the TOML setting. -**Alternative — compositor autostart instead of systemd:** +**Alternative: compositor autostart instead of systemd:** For Hyprland Lua, add these to the matching files: ```lua @@ -713,7 +712,7 @@ wayscriber --active --mode blueprint wayscriber --freeze # start with screen frozen ``` -`whiteboard` and `blackboard` are built in; `blueprint` is an example of a custom board defined in `config.toml` (`[[boards.items]]` — see [Configuration](#configuration)). +`whiteboard` and `blackboard` are built in. `blueprint` is an example of a custom board defined under `[[boards.items]]` in `config.toml`. See [Configuration](#configuration). Bind to a key (Hyprland Lua config): ```lua @@ -731,11 +730,11 @@ The same in-overlay shortcuts apply as in [First launch](#first-launch); Es Light passthrough (layer-shell compositors only) lets normal keyboard and pointer input reach the app underneath while wayscriber stays visible for drawing. -- F6 enters passthrough from the focused overlay. It is a wayscriber in-overlay shortcut, not an OS/global shortcut — once passthrough is active, wayscriber may no longer receive that keypress. +- F6 enters passthrough from the focused overlay. It is a wayscriber in-overlay shortcut, not an OS/global shortcut. Once passthrough is active, wayscriber may no longer receive that keypress. - For reliable control (including getting back out of passthrough), bind compositor/global shortcuts to `wayscriber --light-toggle` and `wayscriber --light-draw-toggle`. - Use `wayscriber --light-draw-on` on press and `wayscriber --light-draw-off` on release for draw-while-held shortcuts. - Hyprland and KDE binding examples are in [docs/SETUP.md](docs/SETUP.md#light-passthrough-controls-on-hyprland); the KDE section follows the Hyprland binding example. -- **Stock GNOME Wayland does not support this mode** — regular app windows cannot provide the required click-through shell overlay. Freeze may still work for still-image capture, but it is not a live passthrough replacement. A GNOME Shell extension approach would be needed for true shell-level passthrough. +- **Stock GNOME Wayland does not support this mode**. Regular app windows cannot provide the required click-through shell overlay. Freeze may still work for still-image capture, but it is not a live passthrough replacement. A GNOME Shell extension approach would be needed for true shell-level passthrough. ### Screenshots and export @@ -796,7 +795,7 @@ In Review you can also remove rows or columns from the captured pixels. Press **Cut** (or X) and drag across the selection; later cuts use the already-collapsed output. **Undo** (Ctrl+Z), **Redo** (Ctrl+Y or Ctrl+Shift+Z), and **Reset** apply only to -this Review — they never change board drawings. After any cut the source crop +this Review. They never change board drawings. After any cut the source crop is locked until you undo all cuts or Reset, and the loupe is hidden while Cut is armed or cuts exist. Copy, Save, Both, and Board all receive the collapsed result; Board keeps the original top-left and shrinks to the output size. @@ -831,7 +830,7 @@ Canvas export commands are available in the command palette and keybindings. `ex - **Check for a newer release:** `wayscriber --check-update` (Wayscriber never installs updates itself; it points at the instructions for your install method). Turn the background check off with `[updates] check = false` or - `WAYSCRIBER_DISABLE_UPDATE_CHECK=1` — see + `WAYSCRIBER_DISABLE_UPDATE_CHECK=1`. See https://wayscriber.com/docs/getting-started/updating.html - **Full docs:** https://wayscriber.com/docs/ @@ -868,25 +867,25 @@ Press F1 for the complete in-app cheat sheet. | Arrow | Ctrl+Shift + drag | | Triangle / parallelogram / rhombus / regular polygon | **Shape picker** in the top strip (bindable) | | Freeform polygon | **Shape picker**, then click vertices; Enter or double-click to finish | -| Blur | **Shape picker** (bindable) — drag a region; style via **Cycle Blur Style** | -| Spotlight | **Shape picker** (bindable) — drag an ellipse; everything else dims; set 1×–4× magnification in the style pill, scroll over the loupe, or select an unlocked loupe and drag its on-canvas knob | +| Blur | **Shape picker** (bindable): drag a region; style via **Cycle Blur Style** | +| Spotlight | **Shape picker** (bindable): drag an ellipse; everything else dims; set 1×–4× magnification in the style pill, scroll over the loupe, or select an unlocked loupe and drag its on-canvas knob | | Step marker tool | Toolbar (bindable) | | Highlight brush | Ctrl+Alt+H | | Text mode | T, Click to place, type, Enter to finish | | Sticky note | N, Click to place, type, Enter to finish | -**Where the Shape picker is.** The top strip shows the common tools inline and puts the rest behind a single **Shape picker** button. What sits inline depends on the strip mode: the simple strip keeps Select, Pen, Marker, Step marker, and Eraser inline, while the full strip adds Line and Arrow. Everything else — rectangle, ellipse, blur, spotlight, and the polygons — is one click away inside the picker. +**Where the Shape picker is.** The top strip shows the common tools inline and puts the rest behind a single **Shape picker** button. The simple strip shows Select, Pen, Marker, Step marker, and Eraser inline. The full strip also shows Line and Arrow. The picker contains rectangle, ellipse, blur, spotlight, and polygon tools. Every tool is also its own toolbar item, so you can show, hide, and reorder them from the settings popover (gear icon) or via `ui.toolbar.items` in `config.toml`. That is how the screenshot button ships hidden by default. -These tools' default keybindings are intentionally empty; bind them under `[keybindings.tools]` if you reach for them often. Drag and mouse-button mappings are configurable — see [Drag-tool mappings](#drag-tool-mappings). +These tools' default keybindings are intentionally empty; bind them under `[keybindings.tools]` if you reach for them often. Drag and mouse-button mappings are configurable. See [Drag-tool mappings](#drag-tool-mappings).
Text editing -While a text block or sticky note is being edited, these keys belong to the editor and are not configurable. Everything else — undo, tool switching, board navigation, capture — still reaches its usual binding. +While a text block or sticky note is being edited, these keys belong to the editor and are not configurable. Undo, tool switching, board navigation, and capture still use their usual bindings. | Action | Key/Mouse | |--------|-----------| @@ -943,7 +942,7 @@ Caret movement and selection follow the rendered layout, so they behave correctl | White | W | | Black | K | -The first eight quick colors map to the shortcuts above and are customizable — see [Quick colors](#quick-colors). +The first eight quick colors map to the shortcuts above and are customizable. See [Quick colors](#quick-colors). Pick a color directly from the displayed desktop with the screen eyedropper: press I, use the eyedropper button in the toolbar or color picker, or search for **Pick screen color** in the command palette (Ctrl+K). Rebind it if you prefer another shortcut: @@ -1059,8 +1058,7 @@ wayscriber-configurator # or press F11 See `docs/CONFIG.md` and https://wayscriber.com/docs/ for the full reference. -`config.toml` contains authored defaults, and it changes only through an explicit user edit action — -never automatically. Two things write it: the configurator's **Save**, and the overlay's three +`config.toml` contains authored defaults. It changes only when you explicitly edit it. Two things write it: the configurator's **Save**, and the overlay's three narrow editors (shortcut editing, preset slots, and the quick-color palette), each of which rewrites only its own key and copies the previous file to a timestamped `.bak` first. Running, using, and quitting Wayscriber never changes the file on its own. An incidental preference toggle applies to @@ -1082,11 +1080,11 @@ item IDs still customize top-toolbar controls and must not be renamed. `wayscriber-configurator` is a native GTK4/libadwaita app built with Relm4, shipped as its own optional package. Open it any time with F11 from the overlay, or run `wayscriber-configurator`. -It covers most of `config.toml`: drawing and arrow defaults, rendering profiles and performance, UI and toolbar layout, presenter mode, history, capture and PDF export, boards, sessions, tablet input, presets, and keybindings. A few things stay hand-edited — `[tray]`, `[updates]`, `[spotlight]`, the `[tablet.stylus_button]` action mappings, and full multi-board setup under `[boards]` — and the configurator leaves all of them untouched when it saves. +It covers most of `config.toml`: drawing and arrow defaults, rendering profiles and performance, UI and toolbar layout, presenter mode, history, capture and PDF export, boards, sessions, tablet input, presets, and keybindings. Some settings require manual edits: `[tray]`, `[updates]`, `[spotlight]`, the `[tablet.stylus_button]` action mappings, and full multi-board setup under `[boards]`. The configurator preserves these settings when it saves. [![The wayscriber configurator editing settings on a Wayland desktop](https://wayscriber.com/img/poster-configurator.webp)](https://wayscriber.com/#configurator) -- Edits authored defaults with validation and automatic backups — your comments, ordering, and keys unknown to this build all survive a save +- Edits authored defaults with validation and automatic backups. Your comments, ordering, and keys unknown to this build all survive a save - ~130 rebindable actions in a searchable list; Ctrl+Shift+E on a command palette row opens that shortcut directly - Installs and manages the daemon service, and applies global shortcuts on GNOME and KDE - Manages sessions: rename, duplicate, move, or clear saved boards and tool state @@ -1116,7 +1114,7 @@ It covers most of `config.toml`: drawing and arrow defaults, rendering profiles
-It is a separate optional package — install the main `wayscriber` package first, then add it from [Installation](#installation). Full reference: https://wayscriber.com/docs/configuration/configurator.html +It is a separate optional package. Install the main `wayscriber` package first. Then add the configurator from [Installation](#installation). Full reference: https://wayscriber.com/docs/configuration/configurator.html ### Key sections @@ -1156,7 +1154,7 @@ size = 3.0 ### Drag-tool mappings -Drag modifier mappings are configurable via `[drawing]` (`drag_tool`, `shift_drag_tool`, `ctrl_drag_tool`, `ctrl_shift_drag_tool`, `tab_drag_tool`) or in the configurator Drawing tab. For per-button workflows, use `[drawing.drag_tools.left]`, `[drawing.drag_tools.right]`, and `[drawing.drag_tools.middle]`; each binding can set a tool and optional color. The polygon tools have intentionally empty default keybindings — select them from the toolbar picker or bind them yourself. Freeform polygon is selectable but not drag-bindable. +Drag modifier mappings are configurable via `[drawing]` (`drag_tool`, `shift_drag_tool`, `ctrl_drag_tool`, `ctrl_shift_drag_tool`, `tab_drag_tool`) or in the configurator Drawing tab. For per-button workflows, use `[drawing.drag_tools.left]`, `[drawing.drag_tools.right]`, and `[drawing.drag_tools.middle]`; each binding can set a tool and optional color. The polygon tools have intentionally empty default keybindings. Select them from the toolbar picker or bind them yourself. Freeform polygon is selectable but not drag-bindable. ### Quick colors @@ -1164,7 +1162,7 @@ The quick color palette is configurable with ordered `[[drawing.quick_colors]]` You can also recolor the palette without opening an editor: **right-click any swatch** to open the color picker for that slot. The swatch updates live as you drag, OK saves that one entry's color back to `config.toml` (leaving your other settings and comments alone, with a timestamped `.bak`), and Cancel restores it. The slot keeps its label and shortcut, so R still selects the red slot after you point it at a different red. Recoloring the swatch you are drawing with moves the live color along with it. Left-click still just selects a swatch, and the leftmost chip still opens the picker for the active tool's own color. -Changed your mind? The recolor picker carries a **Default** button that loads the color wayscriber ships for that slot. It stages the color like any other pick, so the swatch previews it and OK/Cancel still decide. It appears only for the eleven built-in slots — extra colors you added yourself have no shipped default to restore. +Changed your mind? The recolor picker carries a **Default** button that loads the color wayscriber ships for that slot. It stages the color like any other pick, so the swatch previews it and OK/Cancel still decide. It appears only for the eleven built-in slots. Extra colors you added yourself have no shipped default to restore. ### Session manager and persistence @@ -1217,7 +1215,7 @@ Tablet input works out of the box in default builds. Set `[tablet].enabled = fal ### Daemon not starting after reboot -Enabled user services start when you log in, not at boot — if the daemon is missing after a reboot and login, see [Service won't start](#service-wont-start). Enable lingering only if you want the daemon started before login or kept running after logout: +Enabled user services start when you log in. If the daemon is missing after a reboot and login, see [Service won't start](#service-wont-start). Enable lingering only if you want the daemon started before login or kept running after logout: ```bash loginctl enable-linger $USER ``` @@ -1245,7 +1243,7 @@ systemctl --user restart wayscriber.service **Cause:** The "Better Blur DX" effect (or similar blur effects) may blur wayscriber's transparent overlay. -**Solution (Option 1 — configure Better Blur DX):** +**Solution (Option 1: configure Better Blur DX):** 1. Open **System Settings** → **Window Management** → **Desktop Effects** 2. Click the configure button next to "Better Blur DX" 3. Go to the **Force Blur** tab @@ -1253,7 +1251,7 @@ systemctl --user restart wayscriber.service 5. Make sure `Blur all except matching` is selected 6. Click **Apply** -**Solution (Option 2 — use standard blur):** +**Solution (Option 2: use standard blur):** 1. Disable "Better Blur DX" in **Desktop Effects** 2. Enable the standard "Blur" effect instead @@ -1349,7 +1347,7 @@ Future plans are tracked in [GitHub issues](https://github.com/devmobasa/wayscri ## License and credits -**MIT License** — see [LICENSE](LICENSE) +**MIT License**. See [LICENSE](LICENSE) ### Acknowledgments diff --git a/configurator/README.md b/configurator/README.md index 32233a5aa..f7b14578d 100644 --- a/configurator/README.md +++ b/configurator/README.md @@ -1,8 +1,15 @@ # Wayscriber Configurator (GTK4) -Native Rust desktop UI for editing `~/.config/wayscriber/config.toml`. The application is built on GTK4 and libadwaita through [Relm4](https://relm4.org) and reuses the `wayscriber::Config` types directly, so validation and defaults match the CLI. It also retains the original TOML document so comments, ordering, and settings unknown to this build survive a save. +The configurator is a native Rust desktop UI for editing `~/.config/wayscriber/config.toml`. +It uses GTK4, libadwaita, and [Relm4](https://relm4.org). +It shares the `wayscriber::Config` types with the CLI, so validation and defaults match. +It preserves TOML comments, ordering, and settings unknown to this build when it saves. -`config.toml` changes only through an explicit user edit action, never automatically. This program writes it when you press **Save**; the overlay writes it from three narrow editors — shortcut editing, preset slots, and the quick-color palette — each of which rewrites only its own key and backs the file up first. Nothing else in Wayscriber — daemon, tray, startup, shutdown, validation — ever changes it, so an incidental preference toggle applies to that run and sends you here for a durable change. +`config.toml` changes only when you explicitly edit it. The configurator writes the file when you press **Save**. +The overlay can also save shortcut edits, preset slots, and quick colors. +Each overlay editor changes only its own key and backs up the file first. +The daemon, tray, startup, shutdown, and validation do not change the file. +Other preference changes apply to the current run. Use the configurator to change their defaults. This file covers building and running the configurator from source. For screenshots, a demo video, and the user-facing walkthrough, see [Configurator (GUI)](../README.md#configurator-gui) and https://wayscriber.com/docs/configuration/configurator.html @@ -81,3 +88,19 @@ cargo build --release ``` Artifacts land in `target/release/`. No Node toolchain or bundler is required. + +## Workflow ownership + +Each workflow module owns a related set of operations: + +- `app/document_workflow.rs` prevents loads and saves from running at the same time. It passes the loaded document to the save operation. +- `app/migration_workflow.rs` tracks update offers and dismissals for each document destination. +- `app/shortcut_workflow.rs` keeps shortcut recording, text editing, and conflict resolution separate. Only one can be active at a time. +- `app/daemon_workflow.rs` manages background setup actions, status request identities, and typed feedback. + +App update handlers coordinate draft changes and UI effects. + +Saves use `Config::validate_for_save` from the core crate. +It compares persisted typed values to detect changes outside keybindings and rejects those changes. +It also returns keybinding validation reports for user feedback. +Save decisions do not depend on diagnostic text. diff --git a/configurator/src/app/component/view.rs b/configurator/src/app/component/view.rs index 0cfbf88e1..3f6f89244 100644 --- a/configurator/src/app/component/view.rs +++ b/configurator/src/app/component/view.rs @@ -16,14 +16,14 @@ pub(super) fn refresh(app: &ConfiguratorApp, widgets: &mut AppWidgets) { // draft, so Save is not offered while one is on screen: pressing it // would write the last value that parsed and lose the text being typed. let save_enabled = app.is_dirty - && !app.is_saving - && !app.is_loading + && !app.document.is_saving() + && !app.document.is_loading() && app.invalid_color_hex_count() == 0 - && app.pending_shortcut_conflict.is_none(); + && app.shortcuts.conflict().is_none(); if widgets.save_button.is_sensitive() != save_enabled { widgets.save_button.set_sensitive(save_enabled); } - let busy = app.is_loading || app.is_saving; + let busy = app.document.is_loading() || app.document.is_saving(); if widgets.reload_button.is_sensitive() == busy { widgets.reload_button.set_sensitive(!busy); } diff --git a/configurator/src/app/daemon_workflow.rs b/configurator/src/app/daemon_workflow.rs new file mode 100644 index 000000000..e5c5a565f --- /dev/null +++ b/configurator/src/app/daemon_workflow.rs @@ -0,0 +1,217 @@ +//! Background setup workflow; callback identity and feedback policy live together. +use super::effects::Effect; +use crate::models::{DaemonAction, DaemonActionResult, DaemonRuntimeStatus, DesktopEnvironment}; + +#[derive(Debug)] +pub(crate) enum DaemonFeedback { + Status(String), + Action(String), +} +impl DaemonFeedback { + pub(crate) fn text(&self) -> &str { + match self { + Self::Status(text) | Self::Action(text) => text, + } + } +} + +#[derive(Debug)] +pub(crate) struct DaemonWorkflow { + pub(crate) status: Option, + pub(crate) shortcut_input: String, + pub(crate) feedback: Option, + active_action: Option, + pub(crate) next_status_request_id: u64, + pub(crate) latest_status_request_id: u64, + pub(crate) preserve_feedback_status_request_id: Option, +} +impl DaemonWorkflow { + pub(crate) fn is_busy(&self) -> bool { + self.active_action.is_some() + } + pub(crate) fn new(desktop: DesktopEnvironment) -> Self { + Self { + status: None, + shortcut_input: desktop.default_shortcut_input().to_string(), + feedback: Some(DaemonFeedback::Status( + "Detecting background mode setup status...".to_string(), + )), + active_action: None, + next_status_request_id: 2, + latest_status_request_id: 1, + preserve_feedback_status_request_id: None, + } + } +} + +impl DaemonWorkflow { + pub(crate) fn handle_daemon_status_loaded( + &mut self, + request_id: u64, + result: Result, + ) -> Vec { + if request_id != self.latest_status_request_id { + return Vec::new(); + } + let preserve_feedback = self.preserve_feedback_status_request_id == Some(request_id); + if preserve_feedback { + self.preserve_feedback_status_request_id = None; + } + match result { + Ok(status) => { + self.apply_daemon_status(status); + if should_update_feedback_after_status_load( + preserve_feedback, + self.is_busy(), + self.feedback.as_ref(), + ) { + self.feedback = Some(DaemonFeedback::Status( + "Background mode status loaded.".to_string(), + )); + } + } + Err(err) => { + if preserve_feedback && !self.is_busy() { + let previous_feedback = self + .feedback + .as_ref() + .map(DaemonFeedback::text) + .unwrap_or("Background setup action failed."); + self.feedback = Some(DaemonFeedback::Action(format!( + "{previous_feedback}\nStatus refresh failed: {err}" + ))); + } else if !self.is_busy() { + self.feedback = Some(DaemonFeedback::Status(format!( + "Failed to load background setup status: {err}" + ))); + } + } + } + Vec::new() + } + + pub(crate) fn handle_daemon_shortcut_input_changed(&mut self, value: String) -> Vec { + self.shortcut_input = value; + Vec::new() + } + + pub(crate) fn handle_daemon_action_requested(&mut self, action: DaemonAction) -> Vec { + if self.is_busy() { + return Vec::new(); + } + self.invalidate_pending_daemon_status_requests(); + self.active_action = Some(action); + self.feedback = Some(DaemonFeedback::Action(action_pending_message(action))); + let shortcut_input = self.shortcut_input.clone(); + vec![Effect::PerformDaemonAction { + action, + shortcut_input, + }] + } + + pub(crate) fn handle_daemon_action_completed( + &mut self, + result: Result, + ) -> Vec { + self.active_action = None; + match result { + Ok(output) => { + self.apply_daemon_status(output.status); + self.feedback = Some(DaemonFeedback::Action(output.message)); + Vec::new() + } + Err(err) => { + self.feedback = Some(DaemonFeedback::Action(format!( + "Background setup action failed: {err}" + ))); + self.schedule_daemon_status_reload(true) + } + } + } + + fn apply_daemon_status(&mut self, status: DaemonRuntimeStatus) { + if let Some(configured_shortcut) = status.configured_shortcut.clone() { + self.shortcut_input = configured_shortcut; + } else if self.shortcut_input.trim().is_empty() { + self.shortcut_input = status.desktop.default_shortcut_input().to_string(); + } + self.status = Some(status); + } + + fn schedule_daemon_status_reload(&mut self, preserve_feedback: bool) -> Vec { + let request_id = self.next_status_request_id; + self.next_status_request_id = self.next_status_request_id.saturating_add(1); + self.latest_status_request_id = request_id; + if preserve_feedback { + self.preserve_feedback_status_request_id = Some(request_id); + } + vec![Effect::LoadDaemonStatus { request_id }] + } + + fn invalidate_pending_daemon_status_requests(&mut self) { + let invalidation_id = self.next_status_request_id; + self.next_status_request_id = self.next_status_request_id.saturating_add(1); + self.latest_status_request_id = invalidation_id; + self.preserve_feedback_status_request_id = None; + } +} + +fn should_update_feedback_after_status_load( + preserve_feedback: bool, + busy: bool, + feedback: Option<&DaemonFeedback>, +) -> bool { + !preserve_feedback && !busy && matches!(feedback, None | Some(DaemonFeedback::Status(_))) +} + +fn action_pending_message(action: DaemonAction) -> String { + match action { + DaemonAction::RefreshStatus => "Refreshing background setup status...".to_string(), + DaemonAction::InstallOrUpdateService => { + "Installing/updating background service...".to_string() + } + DaemonAction::EnableAndStartService => { + "Enabling and starting background mode...".to_string() + } + DaemonAction::RestartService => "Restarting background service...".to_string(), + DaemonAction::StopAndDisableService => { + "Stopping and disabling background mode...".to_string() + } + DaemonAction::ApplyShortcut => "Applying desktop shortcut setup...".to_string(), + DaemonAction::ApplyLightControls => { + "Applying light passthrough controls setup...".to_string() + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn refresh_policy_depends_on_feedback_kind_not_its_wording() { + let translated_status = DaemonFeedback::Status("Status geladen".into()); + assert!(should_update_feedback_after_status_load( + false, + false, + Some(&translated_status) + )); + let action_using_status_words = + DaemonFeedback::Action("Background mode status loaded.".into()); + assert!(!should_update_feedback_after_status_load( + false, + false, + Some(&action_using_status_words) + )); + assert!(!should_update_feedback_after_status_load( + true, + false, + Some(&translated_status) + )); + assert!(!should_update_feedback_after_status_load( + false, + true, + Some(&translated_status) + )); + } +} diff --git a/configurator/src/app/document_workflow.rs b/configurator/src/app/document_workflow.rs new file mode 100644 index 000000000..aec490ff6 --- /dev/null +++ b/configurator/src/app/document_workflow.rs @@ -0,0 +1,78 @@ +//! Owns document transfer to effects and mutually exclusive load/save phases. +use std::path::PathBuf; +use wayscriber::config::{ConfigDocument, ConfigValidationReport}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum DocumentPhase { + Idle, + Loading, + Saving, +} + +#[derive(Debug)] +pub(crate) struct DocumentWorkflow { + loaded: Option, + phase: DocumentPhase, + pub(crate) pending_validation: ConfigValidationReport, + pub(crate) last_backup_path: Option, +} +impl DocumentWorkflow { + pub(crate) fn loading() -> Self { + Self { + loaded: None, + phase: DocumentPhase::Loading, + pending_validation: Default::default(), + last_backup_path: None, + } + } + pub(crate) fn loaded(&self) -> Option<&ConfigDocument> { + self.loaded.as_ref() + } + pub(crate) fn is_loading(&self) -> bool { + self.phase == DocumentPhase::Loading + } + pub(crate) fn is_saving(&self) -> bool { + self.phase == DocumentPhase::Saving + } + pub(crate) fn begin_reload(&mut self) -> bool { + if self.phase != DocumentPhase::Idle { + return false; + } + self.phase = DocumentPhase::Loading; + true + } + pub(crate) fn finish_load(&mut self, document: Option) { + self.phase = DocumentPhase::Idle; + if document.is_some() { + self.loaded = document; + } + } + pub(crate) fn begin_save(&mut self) -> Option { + if self.phase != DocumentPhase::Idle { + return None; + } + let document = self.loaded.take()?; + self.phase = DocumentPhase::Saving; + Some(document) + } + pub(crate) fn finish_save(&mut self, document: Option) { + self.loaded = document; + self.phase = DocumentPhase::Idle; + } + #[cfg(test)] + pub(crate) fn set_loading_for_test(&mut self, loading: bool) { + self.phase = if loading { + DocumentPhase::Loading + } else { + DocumentPhase::Idle + }; + } + #[cfg(test)] + pub(crate) fn set_saving_for_test(&mut self, saving: bool) { + self.phase = if saving { + DocumentPhase::Saving + } else { + DocumentPhase::Idle + }; + } +} diff --git a/configurator/src/app/migration_workflow.rs b/configurator/src/app/migration_workflow.rs new file mode 100644 index 000000000..14898cc74 --- /dev/null +++ b/configurator/src/app/migration_workflow.rs @@ -0,0 +1,57 @@ +//! Migration offers are tied to a document destination, independent of status text. +use crate::models::{ConfigDraft, KeybindingField}; +use std::path::PathBuf; +use wayscriber::config::{ConfigDocument, MigrationPreview}; + +#[derive(Debug, Default)] +pub(crate) struct MigrationWorkflow { + preview: Option, + dismissed: Option, +} + +pub(crate) struct MigrationApplied { + pub(crate) changed: usize, + pub(crate) kept: Vec<&'static str>, +} + +impl MigrationWorkflow { + pub(crate) fn refresh(&mut self, document: &ConfigDocument) { + if self.dismissed.as_deref() != Some(document.destination()) { + self.dismissed = None; + } + self.preview = MigrationPreview::for_authored_config(document.authored_config()); + } + pub(crate) fn pending(&self) -> Option<&MigrationPreview> { + if self.dismissed.is_some() { + None + } else { + self.preview.as_ref() + } + } + pub(crate) fn dismiss(&mut self, document: Option<&ConfigDocument>) { + if let Some(document) = document { + self.dismissed = Some(document.destination().to_path_buf()); + } + } + pub(crate) fn apply(&mut self, draft: &mut ConfigDraft) -> Option { + self.pending()?; + let preview = self.preview.take()?; + let mut result = MigrationApplied { + changed: 0, + kept: Vec::new(), + }; + for change in preview.changes() { + let Some(field) = KeybindingField::from_field_key(change.config_key()) else { + continue; + }; + if draft.keybindings.parses_to(field, change.before()) { + draft.keybindings.set(field, change.after().join(", ")); + result.changed += 1; + } else if !draft.keybindings.parses_to(field, change.after()) { + result.kept.push(change.action_label()); + } + } + draft.config_revision = Some(preview.proposed_revision()); + Some(result) + } +} diff --git a/configurator/src/app/mod.rs b/configurator/src/app/mod.rs index 26d84ed49..358be7f77 100644 --- a/configurator/src/app/mod.rs +++ b/configurator/src/app/mod.rs @@ -1,11 +1,15 @@ mod blocking_jobs; mod component; mod daemon_setup; +mod daemon_workflow; +mod document_workflow; mod effects; mod io; +mod migration_workflow; mod pages; mod search; mod session_catalog; +mod shortcut_workflow; mod startup; mod state; mod update; diff --git a/configurator/src/app/pages/boards.rs b/configurator/src/app/pages/boards.rs index 560dbb916..0fc4a4efb 100644 --- a/configurator/src/app/pages/boards.rs +++ b/configurator/src/app/pages/boards.rs @@ -165,8 +165,8 @@ fn add_legacy_note(page: &mut PageBuilder) { page.custom(&label); page.bind(move |app, _summary| { let visible = app - .base_document - .as_ref() + .document + .loaded() .is_some_and(|document| document.config().boards.is_none()); if label.is_visible() != visible { label.set_visible(visible); diff --git a/configurator/src/app/pages/daemon.rs b/configurator/src/app/pages/daemon.rs index 70c031836..582a18d35 100644 --- a/configurator/src/app/pages/daemon.rs +++ b/configurator/src/app/pages/daemon.rs @@ -45,7 +45,7 @@ pub(super) fn build(sender: &ComponentSender) -> BuiltPage { // alone: until the environment is known there is nothing for the // setup steps to say. let visible = daemon_section_visible(section, shown_areas(summary)) - && (section == DaemonSection::TechnicalDetails || app.daemon_status.is_some()); + && (section == DaemonSection::TechnicalDetails || app.daemon.status.is_some()); set_visible(&group, visible); })); } diff --git a/configurator/src/app/pages/daemon/groups.rs b/configurator/src/app/pages/daemon/groups.rs index 177c809cd..b0030d640 100644 --- a/configurator/src/app/pages/daemon/groups.rs +++ b/configurator/src/app/pages/daemon/groups.rs @@ -45,13 +45,18 @@ pub(super) fn overview_group( group.add(&body); bindings.push(Box::new(move |app, summary| { - let (text, tone) = overall_status(app.daemon_status.as_ref()); + let (text, tone) = overall_status(app.daemon.status.as_ref()); set_label(&status_label, text); apply_tone(&status_label, tone); set_visible(&status_row, shown_areas(summary).status); - set_sensitive(&refresh, !app.daemon_busy); + set_sensitive(&refresh, !app.daemon.is_busy()); - match app.daemon_feedback.as_deref() { + match app + .daemon + .feedback + .as_ref() + .map(crate::app::daemon_workflow::DaemonFeedback::text) + { Some(feedback) => { set_label(&feedback_label, feedback); apply_tone(&feedback_label, feedback_tone(feedback)); @@ -60,8 +65,8 @@ pub(super) fn overview_group( None => set_visible(&feedback_label, false), } - set_visible(&busy_label, app.daemon_busy); - set_visible(&loading_label, app.daemon_status.is_none()); + set_visible(&busy_label, app.daemon.is_busy()); + set_visible(&loading_label, app.daemon.status.is_none()); })); group @@ -93,7 +98,7 @@ pub(super) fn install_group( set_label(&state_label, text); apply_tone(&state_label, tone); set_button_label(&install, install_button_label(installed)); - set_sensitive(&install, !app.daemon_busy); + set_sensitive(&install, !app.daemon.is_busy()); })); group @@ -146,17 +151,19 @@ pub(super) fn shortcut_group( set_visible(&body, installed); let capability = app - .daemon_status + .daemon + .status .as_ref() .map(|status| status.shortcut_apply_capability); let placeholder = shortcut_placeholder(capability); if entry.placeholder_text().as_deref() != Some(placeholder) { entry.set_placeholder_text(Some(placeholder)); } - set_text_blocked(&entry, &entry_handler, &app.daemon_shortcut_input); + set_text_blocked(&entry, &entry_handler, &app.daemon.shortcut_input); match app - .daemon_status + .daemon + .status .as_ref() .and_then(|status| status.configured_shortcut.as_deref()) { @@ -172,7 +179,7 @@ pub(super) fn shortcut_group( let manual = capability == Some(ShortcutApplyCapability::Manual); set_visible(&manual_label, manual); - set_sensitive(&apply, !app.daemon_busy && !manual); + set_sensitive(&apply, !app.daemon.is_busy() && !manual); })); group @@ -214,7 +221,7 @@ pub(super) fn light_controls_group( group.add(&manual_label); bindings.push(Box::new(move |app, _summary| { - let status = app.daemon_status.as_ref(); + let status = app.daemon.status.as_ref(); match status.and_then(|status| status.light_controls_config_path.as_deref()) { Some(path) => { set_label(&path_label, &format!("Hyprland include: {path}")); @@ -236,7 +243,7 @@ pub(super) fn light_controls_group( light_controls_status(status.is_some_and(|status| status.light_controls_configured)); set_label(&state_label, text); apply_tone(&state_label, tone); - set_sensitive(&install, !app.daemon_busy && installed); + set_sensitive(&install, !app.daemon.is_busy() && installed); })); group @@ -279,7 +286,7 @@ pub(super) fn start_group( set_visible(&locked_label, !installed); set_visible(&body, installed); - let status = app.daemon_status.as_ref(); + let status = app.daemon.status.as_ref(); let running = status.is_some_and(|status| status.service_active); let enabled = status.is_some_and(|status| status.service_enabled); let (text, tone) = service_status(running, enabled); @@ -289,7 +296,7 @@ pub(super) fn start_group( set_visible(&running_row, running); set_visible(&start, !running); for button in [&restart, &stop, &start] { - set_sensitive(button, !app.daemon_busy); + set_sensitive(button, !app.daemon.is_busy()); } })); @@ -329,8 +336,8 @@ pub(super) fn details_group( group.add(&body); bindings.push(Box::new(move |app, _summary| { - set_sensitive(&refresh, !app.daemon_busy); - let Some(status) = app.daemon_status.as_ref() else { + set_sensitive(&refresh, !app.daemon.is_busy()); + let Some(status) = app.daemon.status.as_ref() else { set_visible(&detecting_label, true); for label in [ &desktop_label, diff --git a/configurator/src/app/pages/daemon/status.rs b/configurator/src/app/pages/daemon/status.rs index 0a960098c..4d9878dbf 100644 --- a/configurator/src/app/pages/daemon/status.rs +++ b/configurator/src/app/pages/daemon/status.rs @@ -112,7 +112,8 @@ pub(super) fn missing_tools(status: &DaemonRuntimeStatus) -> Option { } pub(super) fn service_installed(app: &ConfiguratorApp) -> bool { - app.daemon_status + app.daemon + .status .as_ref() .is_some_and(|status| status.service_installed) } diff --git a/configurator/src/app/pages/keybindings/mod.rs b/configurator/src/app/pages/keybindings/mod.rs index 9d20af119..10fae2a1b 100644 --- a/configurator/src/app/pages/keybindings/mod.rs +++ b/configurator/src/app/pages/keybindings/mod.rs @@ -187,7 +187,7 @@ fn conflict_banner( let revealer_for_bind = revealer.clone(); bindings.push(Box::new(move |app, _summary| { - match &app.pending_shortcut_conflict { + match &app.shortcuts.conflict() { Some(conflict) => { set_label(&label, &conflict.prompt()); replace.set_label(conflict.replace_label()); diff --git a/configurator/src/app/pages/keybindings/row.rs b/configurator/src/app/pages/keybindings/row.rs index c257b5cb5..414f83c55 100644 --- a/configurator/src/app/pages/keybindings/row.rs +++ b/configurator/src/app/pages/keybindings/row.rs @@ -262,14 +262,14 @@ pub(super) fn binding_row( } let recording = app - .active_shortcut_recorder - .as_ref() + .shortcuts + .recorder() .filter(|recorder| recorder.field == field); recorder.refresh(recording); let editing = app - .shortcut_text_editor - .as_ref() + .shortcuts + .editor() .filter(|editor| editor.field == field); let editor_text = editing.map(|editor| editor.text.as_str()).unwrap_or(value); let editor_error = editing.and_then(|editor| editor.parse_error()); diff --git a/configurator/src/app/pages/keybindings/toolbar.rs b/configurator/src/app/pages/keybindings/toolbar.rs index b017cdadd..3d4a43882 100644 --- a/configurator/src/app/pages/keybindings/toolbar.rs +++ b/configurator/src/app/pages/keybindings/toolbar.rs @@ -142,13 +142,15 @@ fn actions_bar(sender: &ComponentSender, bindings: &mut Vec { set_label(&blocker_label, blocker); set_visible(&blocker_label, true); @@ -155,7 +155,7 @@ struct CatalogGates { impl CatalogGates { fn of(app: &ConfiguratorApp) -> Self { - let status = app.daemon_status.as_ref(); + let status = app.daemon.status.as_ref(); Self { busy: app.session_catalog.busy || app.session_catalog.is_loading, duplicate_blocked: SessionCatalogOperation::Duplicate diff --git a/configurator/src/app/shortcut_workflow.rs b/configurator/src/app/shortcut_workflow.rs new file mode 100644 index 000000000..ad627aaa8 --- /dev/null +++ b/configurator/src/app/shortcut_workflow.rs @@ -0,0 +1,113 @@ +//! Shortcut edit modes are mutually exclusive and clear as one workflow. +use crate::models::{ + KeybindingField, PendingShortcutConflict, ShortcutRecorderState, ShortcutTextEditor, +}; + +#[derive(Debug, Default)] +enum ShortcutPhase { + #[default] + Idle, + Recording(ShortcutRecorderState), + Text(ShortcutTextEditor), + Conflict(PendingShortcutConflict), +} + +#[derive(Debug, Default)] +pub(crate) struct ShortcutWorkflow { + phase: ShortcutPhase, + pub(crate) review: bool, +} + +impl ShortcutWorkflow { + pub(crate) fn begin_recording(&mut self, recorder: ShortcutRecorderState) { + if self.conflict().is_none() { + self.phase = ShortcutPhase::Recording(recorder); + } + } + pub(crate) fn begin_text_edit(&mut self, editor: ShortcutTextEditor) { + if self.conflict().is_none() { + self.phase = ShortcutPhase::Text(editor); + } + } + pub(crate) fn cancel_recording(&mut self, field: KeybindingField) { + if self + .recorder() + .is_some_and(|recorder| recorder.field == field) + { + self.phase = ShortcutPhase::Idle; + } + } + pub(crate) fn cancel_text_edit(&mut self, field: KeybindingField) { + if self.editor().is_some_and(|editor| editor.field == field) { + self.phase = ShortcutPhase::Idle; + } + } + pub(crate) fn clear(&mut self) { + *self = Self::default(); + } + pub(crate) fn recorder(&self) -> Option<&ShortcutRecorderState> { + match &self.phase { + ShortcutPhase::Recording(value) => Some(value), + _ => None, + } + } + pub(crate) fn recorder_mut(&mut self) -> Option<&mut ShortcutRecorderState> { + match &mut self.phase { + ShortcutPhase::Recording(value) => Some(value), + _ => None, + } + } + pub(crate) fn take_recorder(&mut self) -> Option { + self.recorder()?; + match std::mem::take(&mut self.phase) { + ShortcutPhase::Recording(value) => Some(value), + _ => None, + } + } + pub(crate) fn set_recorder(&mut self, value: Option) { + if let Some(value) = value { + self.phase = ShortcutPhase::Recording(value); + } else if self.recorder().is_some() { + self.phase = ShortcutPhase::Idle; + } + } + pub(crate) fn editor(&self) -> Option<&ShortcutTextEditor> { + match &self.phase { + ShortcutPhase::Text(value) => Some(value), + _ => None, + } + } + pub(crate) fn editor_mut(&mut self) -> Option<&mut ShortcutTextEditor> { + match &mut self.phase { + ShortcutPhase::Text(value) => Some(value), + _ => None, + } + } + pub(crate) fn set_editor(&mut self, value: Option) { + if let Some(value) = value { + self.phase = ShortcutPhase::Text(value); + } else if self.editor().is_some() { + self.phase = ShortcutPhase::Idle; + } + } + pub(crate) fn conflict(&self) -> Option<&PendingShortcutConflict> { + match &self.phase { + ShortcutPhase::Conflict(value) => Some(value), + _ => None, + } + } + pub(crate) fn take_conflict(&mut self) -> Option { + self.conflict()?; + match std::mem::take(&mut self.phase) { + ShortcutPhase::Conflict(value) => Some(value), + _ => None, + } + } + pub(crate) fn set_conflict(&mut self, value: Option) { + if let Some(value) = value { + self.phase = ShortcutPhase::Conflict(value); + } else if self.conflict().is_some() { + self.phase = ShortcutPhase::Idle; + } + } +} diff --git a/configurator/src/app/state.rs b/configurator/src/app/state.rs index db8a49b85..454744631 100644 --- a/configurator/src/app/state.rs +++ b/configurator/src/app/state.rs @@ -1,14 +1,10 @@ use std::collections::HashMap; -use std::path::PathBuf; -use wayscriber::config::{ - Config, ConfigDocument, ConfigValidationReport, MigrationPreview, PRESET_SLOTS_MAX, -}; +use wayscriber::config::{Config, MigrationPreview, PRESET_SLOTS_MAX}; use crate::models::{ - ColorPickerId, ConfigDraft, DaemonRuntimeStatus, DesktopEnvironment, DragMouseButton, - KeybindingField, KeybindingsTabId, PendingShortcutConflict, SearchQuery, SessionCatalogState, - ShortcutManagerFilter, ShortcutManagerSort, ShortcutRecorderState, ShortcutTextEditor, + ColorPickerId, ConfigDraft, DesktopEnvironment, DragMouseButton, KeybindingField, + KeybindingsTabId, SearchQuery, SessionCatalogState, ShortcutManagerFilter, ShortcutManagerSort, StartupRequest, TabId, ToolbarLayoutModeOption, UiTabId, }; @@ -19,11 +15,7 @@ pub(crate) struct ConfiguratorApp { pub(crate) draft: ConfigDraft, pub(crate) baseline: ConfigDraft, pub(crate) defaults: ConfigDraft, - // The source document owns typed config, lossless TOML, and the guarded - // save revision. Owned outright and moved into a running save, so it is - // `None` exactly while a write holds it and for as long as no load has - // produced one. - pub(crate) base_document: Option, + pub(crate) document: super::document_workflow::DocumentWorkflow, pub(crate) status: StatusMessage, pub(crate) active_tab: TabId, pub(crate) active_ui_tab: UiTabId, @@ -33,14 +25,11 @@ pub(crate) struct ConfiguratorApp { pub(crate) shortcut_sort: ShortcutManagerSort, pub(crate) selected_keybinding: Option, pub(crate) keybinding_focus_serial: u64, - pub(crate) shortcut_conflict_review: bool, pub(crate) active_drawing_drag_button: Option, pub(crate) preset_collapsed: Vec, pub(crate) boards_collapsed: Vec, pub(crate) color_picker_hex: HashMap, pub(crate) override_mode: ToolbarLayoutModeOption, - pub(crate) is_loading: bool, - pub(crate) is_saving: bool, pub(crate) is_dirty: bool, /// The destructive question the user can currently answer. /// @@ -48,29 +37,8 @@ pub(crate) struct ConfiguratorApp { /// replaces the other instead of leaving two independently armed actions /// on screen. pub(crate) pending_confirmation: Option, - /// What an accepted migration would change in the loaded configuration. - /// Held here rather than in `status` so an expired or replaced status - /// message cannot take the offer away with it. - pub(crate) migration_preview: Option, - /// The document whose migration offer the user dismissed, named by the file - /// the config path resolved to rather than by the path itself. `None` while - /// no offer has been dismissed. - pub(crate) migration_dismissed: Option, - /// What validating the configuration the running Save is writing had to - /// change in `[keybindings]`, held until that write reports back. - /// - /// The resolution reaches the file, so the reloaded document cannot show - /// it: this is the only carrier from the moment the config is built to the - /// status the finished save renders. - pub(crate) pending_save_validation: ConfigValidationReport, - pub(crate) last_backup_path: Option, - pub(crate) daemon_status: Option, - pub(crate) daemon_shortcut_input: String, - pub(crate) daemon_feedback: Option, - pub(crate) daemon_busy: bool, - pub(crate) daemon_next_status_request_id: u64, - pub(crate) daemon_latest_status_request_id: u64, - pub(crate) daemon_preserve_feedback_status_request_id: Option, + pub(crate) migration: super::migration_workflow::MigrationWorkflow, + pub(crate) daemon: super::daemon_workflow::DaemonWorkflow, pub(crate) session_catalog: SessionCatalogState, pub(crate) search_query: SearchQuery, /// Bumped once per request to put the caret in the search box. The shell @@ -81,9 +49,7 @@ pub(crate) struct ConfiguratorApp { /// What the launching process asked to open, taken by the first config /// load and empty from then on. pub(crate) startup_request: StartupRequest, - pub(crate) active_shortcut_recorder: Option, - pub(crate) shortcut_text_editor: Option, - pub(crate) pending_shortcut_conflict: Option, + pub(crate) shortcuts: super::shortcut_workflow::ShortcutWorkflow, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -225,7 +191,7 @@ impl ConfiguratorApp { draft: baseline.clone(), baseline, defaults, - base_document: None, + document: super::document_workflow::DocumentWorkflow::loading(), status: StatusMessage::info("Loading configuration..."), active_tab: TabId::Daemon, active_ui_tab: UiTabId::Toolbar, @@ -235,39 +201,25 @@ impl ConfiguratorApp { shortcut_sort: ShortcutManagerSort::Category, selected_keybinding: None, keybinding_focus_serial: 0, - shortcut_conflict_review: false, active_drawing_drag_button: None, preset_collapsed: vec![false; PRESET_SLOTS_MAX], boards_collapsed: vec![false; boards_len], color_picker_hex: HashMap::new(), override_mode, - is_loading: true, - is_saving: false, is_dirty: false, pending_confirmation: None, - migration_preview: None, - migration_dismissed: None, - pending_save_validation: ConfigValidationReport::default(), - last_backup_path: None, - daemon_status: None, - daemon_shortcut_input: desktop.default_shortcut_input().to_string(), - daemon_feedback: Some("Detecting background mode setup status...".to_string()), - daemon_busy: false, - daemon_next_status_request_id: 2, - daemon_latest_status_request_id: 1, - daemon_preserve_feedback_status_request_id: None, + migration: super::migration_workflow::MigrationWorkflow::default(), + daemon: super::daemon_workflow::DaemonWorkflow::new(desktop), session_catalog: SessionCatalogState::loading(), search_query: SearchQuery::default(), search_focus_serial: 0, startup_search_focus_pending: true, startup_request: startup, - active_shortcut_recorder: None, - shortcut_text_editor: None, - pending_shortcut_conflict: None, + shortcuts: super::shortcut_workflow::ShortcutWorkflow::default(), }; app.sync_all_color_picker_hex(); - let initial_status_request_id = app.daemon_latest_status_request_id; + let initial_status_request_id = app.daemon.latest_status_request_id; let effects = vec![ Effect::LoadConfig, Effect::LoadDaemonStatus { @@ -352,21 +304,15 @@ impl ConfiguratorApp { /// about, so refreshing the preview clears the dismissal and its offer /// shows. pub(crate) fn pending_migration(&self) -> Option<&MigrationPreview> { - if self.migration_dismissed.is_some() { - return None; - } - self.migration_preview.as_ref() + self.migration.pending() } pub(crate) fn shortcut_recorder_active(&self) -> bool { - self.active_shortcut_recorder.is_some() + self.shortcuts.recorder().is_some() } pub(super) fn clear_shortcut_editing(&mut self) { - self.active_shortcut_recorder = None; - self.shortcut_text_editor = None; - self.pending_shortcut_conflict = None; - self.shortcut_conflict_review = false; + self.shortcuts.clear(); } } diff --git a/configurator/src/app/update/config/defaults.rs b/configurator/src/app/update/config/defaults.rs index 88293faf4..ed067f577 100644 --- a/configurator/src/app/update/config/defaults.rs +++ b/configurator/src/app/update/config/defaults.rs @@ -13,7 +13,8 @@ impl ConfiguratorApp { /// `refresh_dirty_flag`, which is the same standing-down the Cancel /// control asks for explicitly. pub(in crate::app::update) fn handle_reset_to_defaults_requested(&mut self) -> Vec { - if self.is_loading || self.is_saving || self.defaults_reset_pending() { + if self.document.is_loading() || self.document.is_saving() || self.defaults_reset_pending() + { return Vec::new(); } diff --git a/configurator/src/app/update/config/load.rs b/configurator/src/app/update/config/load.rs index 5b4b0e273..84a10bc71 100644 --- a/configurator/src/app/update/config/load.rs +++ b/configurator/src/app/update/config/load.rs @@ -11,7 +11,6 @@ impl ConfiguratorApp { &mut self, result: Result<(Box, Option), String>, ) -> Vec { - self.is_loading = false; match result { Ok((document, repair_warning)) => { let draft = ConfigDraft::from_config(document.config()); @@ -35,9 +34,10 @@ impl ConfiguratorApp { ); // Last, so everything above reads the document by reference and // the model takes ownership of exactly one copy. - self.base_document = Some(*document); + self.document.finish_load(Some(*document)); } Err(err) => { + self.document.finish_load(None); self.status = StatusMessage::error(format!("Failed to load config from disk: {err}")); } @@ -51,8 +51,7 @@ impl ConfiguratorApp { } pub(in crate::app::update) fn handle_reload_requested(&mut self) -> Vec { - if !self.is_loading && !self.is_saving { - self.is_loading = true; + if self.document.begin_reload() { self.clear_defaults_confirmation(); self.status = StatusMessage::info("Reloading configuration..."); return vec![Effect::LoadConfig]; diff --git a/configurator/src/app/update/config/migration.rs b/configurator/src/app/update/config/migration.rs index 82022dabf..fc53d294d 100644 --- a/configurator/src/app/update/config/migration.rs +++ b/configurator/src/app/update/config/migration.rs @@ -1,6 +1,4 @@ -use wayscriber::config::{ConfigDocument, MigrationPreview}; - -use crate::models::KeybindingField; +use wayscriber::config::ConfigDocument; use super::super::super::effects::Effect; use super::super::super::state::{ConfiguratorApp, StatusMessage}; @@ -21,48 +19,18 @@ impl ConfiguratorApp { /// destination is what tells the two apart — the path is the same either /// way. pub(super) fn refresh_migration_preview(&mut self, document: &ConfigDocument) { - if self.migration_dismissed.as_deref() != Some(document.destination()) { - self.migration_dismissed = None; - } - self.migration_preview = MigrationPreview::for_authored_config(document.authored_config()); + self.migration.refresh(document); } pub(in crate::app::update) fn handle_migration_apply_requested(&mut self) -> Vec { - if self.is_loading || self.is_saving { + if self.document.is_loading() || self.document.is_saving() { return Vec::new(); } - let Some(preview) = self.pending_migration().cloned() else { + let Some(result) = self.migration.apply(&mut self.draft) else { return Vec::new(); }; - - let mut applied = 0usize; - let mut kept = Vec::new(); - for change in preview.changes() { - // A key this build has no field for cannot be shown or edited, so - // it is left alone rather than written blind. - let Some(field) = KeybindingField::from_field_key(change.config_key()) else { - continue; - }; - // The preview was computed when the file loaded; the draft has been - // editable ever since. A field that no longer reads as the "before" - // the proposal was built from is the user's own edit, and applying - // the proposal's "after" over it would silently discard what they - // typed — so it is kept and reported instead. - if self.draft.keybindings.parses_to(field, change.before()) { - self.draft.keybindings.set(field, change.after().join(", ")); - applied += 1; - } else if !self.draft.keybindings.parses_to(field, change.after()) { - kept.push(change.action_label()); - } - } - // Apply answers the migration question even when the user's own edits - // cover every proposed field. Those edits are kept above; recording the - // revision says this generation was reviewed, not that every shipped - // default was copied verbatim. Without the stamp, customized fields make - // the recipes decline on the next load anyway, leaving an old revision - // while the status incorrectly promises the offer will return. - self.draft.config_revision = Some(preview.proposed_revision()); - self.migration_preview = None; + let applied = result.changed; + let kept = result.kept; let label = if applied == 1 { "shortcut update" } else { @@ -92,9 +60,7 @@ impl ConfiguratorApp { // this answer covers. Without a document in hand there is no file to // name — no load has produced one, or a running save is holding it — // and an answer already given stands rather than being cleared. - if let Some(document) = self.base_document.as_ref() { - self.migration_dismissed = Some(document.destination().to_path_buf()); - } + self.migration.dismiss(self.document.loaded()); Vec::new() } diff --git a/configurator/src/app/update/config/save.rs b/configurator/src/app/update/config/save.rs index f9cc30ead..c828c46e3 100644 --- a/configurator/src/app/update/config/save.rs +++ b/configurator/src/app/update/config/save.rs @@ -14,7 +14,7 @@ impl ConfiguratorApp { // document when it lands, so a save started underneath it would write // the pre-reload draft and then be judged against a document it never // saw — leaving stale fields marked clean and the next save rejected. - if self.is_saving || self.is_loading { + if self.document.is_saving() || self.document.is_loading() { return Vec::new(); } self.clear_defaults_confirmation(); @@ -29,7 +29,7 @@ impl ConfiguratorApp { return Vec::new(); } - if self.pending_shortcut_conflict.is_some() { + if self.shortcuts.conflict().is_some() { self.status = StatusMessage::error("Resolve the shortcut conflict before saving."); return Vec::new(); } @@ -38,7 +38,7 @@ impl ConfiguratorApp { // copy here and gets one back from `handle_config_saved` either way. // Taking it is also the "nothing loaded" check: there is one `Option` // to read, and reading it is what moves the value. - let Some(document) = self.base_document.take() else { + let Some(document) = self.document.begin_save() else { self.status = StatusMessage::error( "Configuration has not loaded successfully. Reload before saving.", ); @@ -47,7 +47,6 @@ impl ConfiguratorApp { match self.prepare_config_to_save(&document) { Ok(config) => { - self.is_saving = true; self.status = StatusMessage::info("Saving configuration..."); vec![Effect::SaveConfig { document: Box::new(document), @@ -57,7 +56,7 @@ impl ConfiguratorApp { Err(errors) => { // No write starts, so the document goes straight back: this // handler must not be a way to lose it. - self.base_document = Some(document); + self.document.finish_save(Some(document)); let message = errors .into_iter() .map(|err| format!("{}: {}", err.field, err.message)) @@ -84,31 +83,30 @@ impl ConfiguratorApp { &mut self, document: &ConfigDocument, ) -> Result> { - let mut config = self.draft.to_config(document.config())?; - let before_clamp = config.clone(); - self.pending_save_validation = config.validate_and_clamp(); - if save_clamped_non_keybinding_fields(&before_clamp, &config) { - self.pending_save_validation = Default::default(); - return Err(vec![FormError::new( - "config", - "Some values are outside their allowed ranges and would be changed on save. Fix them before saving.", - )]); + let config = self.draft.to_config(document.config())?; + match config.validate_for_save() { + Ok((config, report)) => { + self.document.pending_validation = report; + Ok(config) + } + Err(error) => { + self.document.pending_validation = Default::default(); + Err(vec![FormError::new("config", error.to_string())]) + } } - Ok(config) } pub(in crate::app::update) fn handle_config_saved( &mut self, result: ConfigSaveResult, ) -> Vec { - self.is_saving = false; // Either outcome answers this write; a failed one wrote nothing, so // there is no resolution to report for it. - let validation = std::mem::take(&mut self.pending_save_validation); + let validation = std::mem::take(&mut self.document.pending_validation); match result { Ok((backup, saved_document)) => { let draft = ConfigDraft::from_config(saved_document.config()); - self.last_backup_path = backup.clone(); + self.document.last_backup_path = backup.clone(); self.draft = draft.clone(); self.baseline = draft; self.boards_collapsed = vec![false; self.draft.boards.items.len()]; @@ -129,7 +127,7 @@ impl ConfiguratorApp { status = status.with_note(¬e); } self.status = status; - self.base_document = Some(*saved_document); + self.document.finish_save(Some(*saved_document)); } Err((document, err)) => { // The write borrowed the model's only document; a failure hands @@ -137,7 +135,8 @@ impl ConfiguratorApp { // with nothing to hand back is a blocking job that never // returned, which leaves a reload as the way forward. let restored = document.is_some(); - self.base_document = document.map(|document| *document); + self.document + .finish_save(document.map(|document| *document)); let mut message = format!("Failed to save configuration: {err}"); if !restored { message.push_str( @@ -151,9 +150,3 @@ impl ConfiguratorApp { Vec::new() } } - -fn save_clamped_non_keybinding_fields(before: &Config, after: &Config) -> bool { - let mut before = before.clone(); - before.keybindings = after.keybindings.clone(); - format!("{before:?}") != format!("{after:?}") -} diff --git a/configurator/src/app/update/config/tests.rs b/configurator/src/app/update/config/tests.rs index 9e3ac2774..b0f20c591 100644 --- a/configurator/src/app/update/config/tests.rs +++ b/configurator/src/app/update/config/tests.rs @@ -34,7 +34,7 @@ fn handle_config_loaded_success_resets_loading_and_dirty_state() { let (path, document) = temp_config_document("loaded", ""); let _ = app.handle_config_loaded(Ok((document, None))); - assert!(!app.is_loading); + assert!(!app.document.is_loading()); assert!(!app.is_dirty); assert_eq!(app.boards_collapsed.len(), app.draft.boards.items.len()); assert!(status_contains( @@ -75,10 +75,10 @@ fn handle_config_loaded_error_preserves_the_last_good_document_and_draft() { let _ = app.handle_config_loaded(Err("broken".to_string())); - assert!(!app.is_loading); + assert!(!app.document.is_loading()); assert_eq!( - app.base_document - .as_ref() + app.document + .loaded() .expect("last good document") .destination(), destination, @@ -102,7 +102,7 @@ fn handle_config_loaded_repair_document_allows_saving() { Some("invalid type: string, expected u32".to_string()), ))); - assert!(app.base_document.is_some()); + assert!(app.document.loaded().is_some()); assert!(matches!(app.status, StatusMessage::Warning(_))); assert!(status_contains(&app.status, "loaded for repair")); assert!(status_contains( @@ -110,7 +110,7 @@ fn handle_config_loaded_repair_document_allows_saving() { "malformed TOML content remains only in the backup" )); let _ = app.handle_save_requested(); - assert!(app.is_saving); + assert!(app.document.is_saving()); let _ = std::fs::remove_file(path); } @@ -247,12 +247,12 @@ fn handle_save_requested_blocks_without_loaded_document() { let (mut app, _effects) = ConfiguratorApp::new_app(); // A fresh app is still running its startup load; this test is about // the load having finished without producing a document. - app.is_loading = false; + app.document.set_loading_for_test(false); let effects = app.handle_save_requested(); assert!(effects.is_empty()); - assert!(!app.is_saving); + assert!(!app.document.is_saving()); assert!(status_contains( &app.status, "Configuration has not loaded successfully" @@ -262,14 +262,14 @@ fn handle_save_requested_blocks_without_loaded_document() { #[test] fn handle_save_requested_sets_saving_for_valid_draft() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_saving = false; + app.document.set_saving_for_test(false); let (path, document) = temp_config_document("save-request", ""); let _ = app.handle_config_loaded(Ok((document, None))); let effects = app.handle_save_requested(); assert!(matches!(effects.as_slice(), [Effect::SaveConfig { .. }])); - assert!(app.is_saving); + assert!(app.document.is_saving()); assert!(status_contains(&app.status, "Saving configuration...")); let _ = std::fs::remove_file(path); } @@ -284,7 +284,7 @@ fn the_running_save_holds_the_document_and_the_result_returns_one() { let (document, config) = save_effect(&mut app); assert!( - app.base_document.is_none(), + app.document.loaded().is_none(), "the write holds the document while it runs" ); let (saved, backup) = document @@ -294,7 +294,7 @@ fn the_running_save_holds_the_document_and_the_result_returns_one() { let _ = app.handle_config_saved(Ok((backup, Box::new(saved)))); assert!( - app.base_document.is_some(), + app.document.loaded().is_some(), "a finished save hands a document back" ); } @@ -308,13 +308,13 @@ fn a_failed_save_hands_the_document_back() { app.refresh_dirty_flag(); let (document, _config) = save_effect(&mut app); - assert!(app.base_document.is_none()); + assert!(app.document.loaded().is_none()); let _ = app.handle_config_saved(Err((Some(document), "Permission denied".to_string()))); - assert!(!app.is_saving); + assert!(!app.document.is_saving()); assert!( - app.base_document.is_some(), + app.document.loaded().is_some(), "the document the failed write borrowed must return to the model" ); assert!(app.is_dirty, "the draft is still unsaved"); @@ -343,7 +343,7 @@ fn a_save_whose_job_never_returned_asks_for_a_reload() { let _ = app.handle_config_saved(Err((None, "config save blocking job panicked".to_string()))); - assert!(app.base_document.is_none()); + assert!(app.document.loaded().is_none()); assert!(status_contains(&app.status, "Reload before saving again")); } @@ -357,9 +357,9 @@ fn a_draft_the_converter_rejects_keeps_the_document() { let effects = app.handle_save_requested(); assert!(effects.is_empty()); - assert!(!app.is_saving); + assert!(!app.document.is_saving()); assert!( - app.base_document.is_some(), + app.document.loaded().is_some(), "a refused save must not take the document with it" ); assert!(status_contains( @@ -378,9 +378,9 @@ fn a_draft_with_out_of_range_numbers_keeps_the_document() { let effects = app.handle_save_requested(); assert!(effects.is_empty()); - assert!(!app.is_saving); + assert!(!app.document.is_saving()); assert!( - app.base_document.is_some(), + app.document.loaded().is_some(), "a refused save must not take the document with it" ); assert!(status_contains( @@ -404,8 +404,11 @@ fn a_color_field_holding_invalid_hex_blocks_the_save() { let effects = app.handle_save_requested(); assert!(effects.is_empty()); - assert!(!app.is_saving); - assert!(app.base_document.is_some(), "nothing was written or taken"); + assert!(!app.document.is_saving()); + assert!( + app.document.loaded().is_some(), + "nothing was written or taken" + ); assert!(status_contains(&app.status, "1 color field")); assert!(status_contains( &app.status, @@ -507,7 +510,7 @@ fn every_invalid_color_field_is_counted_for_the_refusal() { #[test] fn reset_to_defaults_requires_confirmation() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft.capture_enabled = !app.defaults.capture_enabled; let changed_draft = app.draft.clone(); @@ -524,7 +527,7 @@ fn reset_to_defaults_requires_confirmation() { #[test] fn reset_to_defaults_repeated_request_is_a_no_op() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft.capture_enabled = !app.defaults.capture_enabled; let changed_draft = app.draft.clone(); @@ -541,7 +544,7 @@ fn reset_to_defaults_repeated_request_is_a_no_op() { #[test] fn reset_to_defaults_confirmed_applies_the_defaults() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft.capture_enabled = !app.defaults.capture_enabled; app.baseline.capture_enabled = !app.defaults.capture_enabled; @@ -562,7 +565,7 @@ fn reset_to_defaults_confirmed_applies_the_defaults() { #[test] fn reset_to_defaults_confirmed_without_a_request_changes_nothing() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft.capture_enabled = !app.defaults.capture_enabled; let changed_draft = app.draft.clone(); @@ -581,7 +584,7 @@ fn reset_to_defaults_confirmed_without_a_request_changes_nothing() { #[test] fn reset_to_defaults_canceled_disarms_and_clears_the_hint() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft.capture_enabled = !app.defaults.capture_enabled; let changed_draft = app.draft.clone(); @@ -600,7 +603,7 @@ fn reset_to_defaults_canceled_disarms_and_clears_the_hint() { #[test] fn reset_to_defaults_canceled_keeps_status_that_replaced_the_hint() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); let _ = app.handle_reset_to_defaults_requested(); app.status = StatusMessage::error("Failed to clear session s-1: nope"); @@ -617,7 +620,7 @@ fn reset_to_defaults_canceled_keeps_status_that_replaced_the_hint() { #[test] fn reset_to_defaults_canceled_without_a_request_keeps_the_status() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.status = StatusMessage::error("Failed to load config from disk: nope"); let _ = app.handle_reset_to_defaults_canceled(); @@ -628,7 +631,7 @@ fn reset_to_defaults_canceled_without_a_request_keeps_the_status() { #[test] fn active_confirmation_cancel_uses_the_typed_owner() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); let _ = app.handle_reset_to_defaults_requested(); let effects = app.handle_active_confirmation_canceled(); @@ -641,7 +644,7 @@ fn active_confirmation_cancel_uses_the_typed_owner() { #[test] fn active_confirmation_cancel_preserves_newer_feedback() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); let _ = app.handle_reset_to_defaults_requested(); app.status = StatusMessage::error("A newer operation failed"); @@ -655,7 +658,7 @@ fn active_confirmation_cancel_preserves_newer_feedback() { #[test] fn reset_to_defaults_confirmation_is_canceled_by_draft_edit() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); let _ = app.handle_reset_to_defaults_requested(); let _ = app.handle_toggle_changed(ToggleField::CaptureEnabled, !app.draft.capture_enabled); @@ -669,7 +672,7 @@ fn reset_to_defaults_confirmation_is_canceled_by_draft_edit() { #[test] fn a_draft_edit_between_request_and_confirm_refuses_the_confirm() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); let _ = app.handle_reset_to_defaults_requested(); let _ = app.handle_toggle_changed(ToggleField::CaptureEnabled, !app.draft.capture_enabled); @@ -696,7 +699,7 @@ fn a_shortcut_typed_for_an_omitted_action_is_arbitrated_not_filtered() { .keybindings .set(KeybindingField::ClearCanvas, "Ctrl+Alt+U".to_string()); - let document = app.base_document.as_ref().expect("a loaded document"); + let document = app.document.loaded().expect("a loaded document"); let mut config = app .draft .to_config(document.config()) @@ -781,9 +784,9 @@ fn a_typed_shortcut_the_parser_rejects_is_reported_by_the_save() { let effects = app.handle_save_requested(); assert!(effects.is_empty()); - assert!(!app.is_saving); + assert!(!app.document.is_saving()); assert!( - app.base_document.is_some(), + app.document.loaded().is_some(), "a refused save must not take the document with it" ); assert!(status_contains( @@ -801,15 +804,15 @@ fn a_typed_shortcut_the_parser_rejects_is_reported_by_the_save() { fn save_is_refused_while_a_reload_is_in_flight() { let (mut app, _effects) = ConfiguratorApp::new_app(); let (path, document) = temp_config_document("save-during-reload", ""); - app.base_document = Some(*document); - app.is_loading = true; + app.document.finish_load(Some(*document)); + app.document.set_loading_for_test(true); app.is_dirty = true; let before = app.status.clone(); let _ = app.handle_save_requested(); assert!( - !app.is_saving, + !app.document.is_saving(), "no save may start under an in-flight reload" ); assert!(app.is_dirty, "the draft stays dirty for the next attempt"); @@ -824,18 +827,18 @@ fn save_is_refused_while_a_reload_is_in_flight() { #[test] fn handle_config_saved_success_clears_dirty_and_records_backup() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_saving = true; + app.document.set_saving_for_test(true); app.is_dirty = true; app.draft.capture_enabled = !app.draft.capture_enabled; let backup = PathBuf::from("/tmp/wayscriber-config.bak"); let (path, document) = temp_config_document("saved", ""); let _ = app.handle_config_saved(Ok((Some(backup.clone()), document))); - assert!(app.base_document.is_some()); + assert!(app.document.loaded().is_some()); - assert!(!app.is_saving); + assert!(!app.document.is_saving()); assert!(!app.is_dirty); - assert_eq!(app.last_backup_path, Some(backup)); + assert_eq!(app.document.last_backup_path, Some(backup)); assert_eq!(app.draft, app.baseline); assert!(status_contains( &app.status, diff --git a/configurator/src/app/update/daemon.rs b/configurator/src/app/update/daemon.rs index bfbf3ee22..09b9bb0c8 100644 --- a/configurator/src/app/update/daemon.rs +++ b/configurator/src/app/update/daemon.rs @@ -1,7 +1,6 @@ -use crate::models::{DaemonAction, DaemonActionResult, DaemonRuntimeStatus}; - use super::super::effects::Effect; use super::super::state::ConfiguratorApp; +use crate::models::{DaemonAction, DaemonActionResult, DaemonRuntimeStatus}; impl ConfiguratorApp { pub(super) fn handle_daemon_status_loaded( @@ -9,142 +8,19 @@ impl ConfiguratorApp { request_id: u64, result: Result, ) -> Vec { - if request_id != self.daemon_latest_status_request_id { - return Vec::new(); - } - let preserve_feedback = self.daemon_preserve_feedback_status_request_id == Some(request_id); - if preserve_feedback { - self.daemon_preserve_feedback_status_request_id = None; - } - match result { - Ok(status) => { - self.apply_daemon_status(status); - if should_update_feedback_after_status_load( - preserve_feedback, - self.daemon_busy, - self.daemon_feedback.as_deref(), - ) { - self.daemon_feedback = Some("Background mode status loaded.".to_string()); - } - } - Err(err) => { - if preserve_feedback && !self.daemon_busy { - let previous_feedback = self - .daemon_feedback - .as_deref() - .unwrap_or("Background setup action failed."); - self.daemon_feedback = - Some(format!("{previous_feedback}\nStatus refresh failed: {err}")); - } else if !self.daemon_busy { - self.daemon_feedback = - Some(format!("Failed to load background setup status: {err}")); - } - } - } - Vec::new() + self.daemon.handle_daemon_status_loaded(request_id, result) } - pub(super) fn handle_daemon_shortcut_input_changed(&mut self, value: String) -> Vec { - self.daemon_shortcut_input = value; - Vec::new() + self.daemon.handle_daemon_shortcut_input_changed(value) } - pub(super) fn handle_daemon_action_requested(&mut self, action: DaemonAction) -> Vec { - if self.daemon_busy { - return Vec::new(); - } - self.invalidate_pending_daemon_status_requests(); - self.daemon_busy = true; - self.daemon_feedback = Some(action_pending_message(action)); - let shortcut_input = self.daemon_shortcut_input.clone(); - vec![Effect::PerformDaemonAction { - action, - shortcut_input, - }] + self.daemon.handle_daemon_action_requested(action) } - pub(super) fn handle_daemon_action_completed( &mut self, result: Result, ) -> Vec { - self.daemon_busy = false; - match result { - Ok(output) => { - self.apply_daemon_status(output.status); - self.daemon_feedback = Some(output.message); - Vec::new() - } - Err(err) => { - self.daemon_feedback = Some(format!("Background setup action failed: {err}")); - self.schedule_daemon_status_reload(true) - } - } - } - - fn apply_daemon_status(&mut self, status: DaemonRuntimeStatus) { - if let Some(configured_shortcut) = status.configured_shortcut.clone() { - self.daemon_shortcut_input = configured_shortcut; - } else if self.daemon_shortcut_input.trim().is_empty() { - self.daemon_shortcut_input = status.desktop.default_shortcut_input().to_string(); - } - self.daemon_status = Some(status); - } - - fn schedule_daemon_status_reload(&mut self, preserve_feedback: bool) -> Vec { - let request_id = self.daemon_next_status_request_id; - self.daemon_next_status_request_id = self.daemon_next_status_request_id.saturating_add(1); - self.daemon_latest_status_request_id = request_id; - if preserve_feedback { - self.daemon_preserve_feedback_status_request_id = Some(request_id); - } - vec![Effect::LoadDaemonStatus { request_id }] - } - - fn invalidate_pending_daemon_status_requests(&mut self) { - let invalidation_id = self.daemon_next_status_request_id; - self.daemon_next_status_request_id = self.daemon_next_status_request_id.saturating_add(1); - self.daemon_latest_status_request_id = invalidation_id; - self.daemon_preserve_feedback_status_request_id = None; - } -} - -fn should_update_feedback_after_status_load( - preserve_feedback: bool, - daemon_busy: bool, - current_feedback: Option<&str>, -) -> bool { - if preserve_feedback || daemon_busy { - return false; - } - let Some(feedback) = current_feedback else { - return true; - }; - let normalized = feedback.to_ascii_lowercase(); - normalized.contains("detecting background mode setup status") - || normalized.contains("refreshing background setup status") - || normalized.contains("detecting daemon setup status") - || normalized.contains("refreshing daemon status") - || normalized == "background mode status loaded." - || normalized == "daemon status loaded." -} - -fn action_pending_message(action: DaemonAction) -> String { - match action { - DaemonAction::RefreshStatus => "Refreshing background setup status...".to_string(), - DaemonAction::InstallOrUpdateService => { - "Installing/updating background service...".to_string() - } - DaemonAction::EnableAndStartService => { - "Enabling and starting background mode...".to_string() - } - DaemonAction::RestartService => "Restarting background service...".to_string(), - DaemonAction::StopAndDisableService => { - "Stopping and disabling background mode...".to_string() - } - DaemonAction::ApplyShortcut => "Applying desktop shortcut setup...".to_string(), - DaemonAction::ApplyLightControls => { - "Applying light passthrough controls setup...".to_string() - } + self.daemon.handle_daemon_action_completed(result) } } @@ -178,14 +54,14 @@ mod tests { #[test] fn daemon_status_loaded_sets_default_shortcut_when_missing() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.daemon_shortcut_input.clear(); + app.daemon.shortcut_input.clear(); let status = test_status(DesktopEnvironment::Kde, None); - app.daemon_latest_status_request_id = 7; + app.daemon.latest_status_request_id = 7; let _ = app.handle_daemon_status_loaded(7, Ok(status)); - assert_eq!(app.daemon_shortcut_input, "Ctrl+Shift+G"); - assert!(app.daemon_status.is_some()); + assert_eq!(app.daemon.shortcut_input, "Ctrl+Shift+G"); + assert!(app.daemon.status.is_some()); } #[test] @@ -193,30 +69,37 @@ mod tests { let (mut app, _effects) = ConfiguratorApp::new_app(); let _ = app.handle_daemon_action_completed(Err("boom".to_string())); assert!( - app.daemon_feedback - .as_deref() + app.daemon + .feedback + .as_ref() + .map(crate::app::daemon_workflow::DaemonFeedback::text) .unwrap_or_default() .contains("Background setup action failed") ); assert_eq!( - app.daemon_preserve_feedback_status_request_id, - Some(app.daemon_latest_status_request_id) + app.daemon.preserve_feedback_status_request_id, + Some(app.daemon.latest_status_request_id) ); } #[test] fn status_loaded_does_not_clear_daemon_busy() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.daemon_busy = true; - app.daemon_feedback = Some("Installing/updating background service...".to_string()); + let _ = app.handle_daemon_action_requested(DaemonAction::RestartService); + app.daemon.feedback = Some(crate::app::daemon_workflow::DaemonFeedback::Action( + "Installing/updating background service...".to_string(), + )); let status = test_status(DesktopEnvironment::Kde, None); - app.daemon_latest_status_request_id = 9; + app.daemon.latest_status_request_id = 9; let _ = app.handle_daemon_status_loaded(9, Ok(status)); - assert!(app.daemon_busy); + assert!(app.daemon.is_busy()); assert_eq!( - app.daemon_feedback.as_deref(), + app.daemon + .feedback + .as_ref() + .map(crate::app::daemon_workflow::DaemonFeedback::text), Some("Installing/updating background service...") ); } @@ -225,31 +108,33 @@ mod tests { fn failed_action_feedback_is_preserved_after_status_reload() { let (mut app, _effects) = ConfiguratorApp::new_app(); let _ = app.handle_daemon_action_completed(Err("boom".to_string())); - let preserved_request_id = app.daemon_latest_status_request_id; + let preserved_request_id = app.daemon.latest_status_request_id; let status = test_status(DesktopEnvironment::Kde, None); let _ = app.handle_daemon_status_loaded(preserved_request_id, Ok(status)); assert!( - app.daemon_feedback - .as_deref() + app.daemon + .feedback + .as_ref() + .map(crate::app::daemon_workflow::DaemonFeedback::text) .unwrap_or_default() .contains("Background setup action failed: boom") ); - assert!(app.daemon_preserve_feedback_status_request_id.is_none()); + assert!(app.daemon.preserve_feedback_status_request_id.is_none()); } #[test] fn stale_status_callback_does_not_consume_preserve_flag() { let (mut app, _effects) = ConfiguratorApp::new_app(); let _ = app.handle_daemon_action_completed(Err("boom".to_string())); - let preserved_request_id = app.daemon_latest_status_request_id; + let preserved_request_id = app.daemon.latest_status_request_id; let stale_request_id = preserved_request_id.saturating_sub(1); let stale_status = test_status(DesktopEnvironment::Kde, None); let _ = app.handle_daemon_status_loaded(stale_request_id, Ok(stale_status)); assert_eq!( - app.daemon_preserve_feedback_status_request_id, + app.daemon.preserve_feedback_status_request_id, Some(preserved_request_id) ); } @@ -258,9 +143,11 @@ mod tests { fn preserved_error_is_not_applied_while_new_action_is_busy() { let (mut app, _effects) = ConfiguratorApp::new_app(); let _ = app.handle_daemon_action_completed(Err("boom".to_string())); - let preserved_request_id = app.daemon_latest_status_request_id; - app.daemon_busy = true; - app.daemon_feedback = Some("Restarting background service...".to_string()); + let preserved_request_id = app.daemon.latest_status_request_id; + let _ = app.handle_daemon_action_requested(DaemonAction::RestartService); + app.daemon.feedback = Some(crate::app::daemon_workflow::DaemonFeedback::Action( + "Restarting background service...".to_string(), + )); let _ = app.handle_daemon_status_loaded( preserved_request_id, @@ -268,7 +155,10 @@ mod tests { ); assert_eq!( - app.daemon_feedback.as_deref(), + app.daemon + .feedback + .as_ref() + .map(crate::app::daemon_workflow::DaemonFeedback::text), Some("Restarting background service...") ); } @@ -290,7 +180,7 @@ mod tests { new_status.service_unit_path = Some("/tmp/wayscriber.service".to_string()); let _ = app.handle_daemon_action_completed(Err("old failure".to_string())); - let old_request_id = app.daemon_latest_status_request_id; + let old_request_id = app.daemon.latest_status_request_id; let effects = app.handle_daemon_action_requested(DaemonAction::RefreshStatus); assert!(matches!( @@ -307,13 +197,20 @@ mod tests { let _ = app.handle_daemon_status_loaded(old_request_id, Ok(old_status)); - assert_eq!(app.daemon_shortcut_input.as_str(), "new"); + assert_eq!(app.daemon.shortcut_input.as_str(), "new"); assert_eq!( - app.daemon_status + app.daemon + .status .as_ref() .and_then(|status| status.configured_shortcut.as_deref()), Some("new") ); - assert_eq!(app.daemon_feedback.as_deref(), Some("refresh complete")); + assert_eq!( + app.daemon + .feedback + .as_ref() + .map(crate::app::daemon_workflow::DaemonFeedback::text), + Some("refresh complete") + ); } } diff --git a/configurator/src/app/update/session_catalog.rs b/configurator/src/app/update/session_catalog.rs index 778e5bec8..663db72df 100644 --- a/configurator/src/app/update/session_catalog.rs +++ b/configurator/src/app/update/session_catalog.rs @@ -97,7 +97,7 @@ impl ConfiguratorApp { return Vec::new(); } if let Some(blocker) = - SessionCatalogOperation::Duplicate.cached_status_blocker(self.daemon_status.as_ref()) + SessionCatalogOperation::Duplicate.cached_status_blocker(self.daemon.status.as_ref()) { self.status = StatusMessage::warning(blocker); return Vec::new(); @@ -135,7 +135,7 @@ impl ConfiguratorApp { return Vec::new(); } if let Some(blocker) = - SessionCatalogOperation::Move.cached_status_blocker(self.daemon_status.as_ref()) + SessionCatalogOperation::Move.cached_status_blocker(self.daemon.status.as_ref()) { self.status = StatusMessage::warning(blocker); return Vec::new(); @@ -177,7 +177,7 @@ impl ConfiguratorApp { return Vec::new(); } if let Some(blocker) = SessionCatalogOperation::ClearToolState - .cached_status_blocker(self.daemon_status.as_ref()) + .cached_status_blocker(self.daemon.status.as_ref()) { self.status = StatusMessage::warning(blocker); return Vec::new(); @@ -198,7 +198,7 @@ impl ConfiguratorApp { return Vec::new(); } if let Some(blocker) = - SessionCatalogOperation::Clear.cached_status_blocker(self.daemon_status.as_ref()) + SessionCatalogOperation::Clear.cached_status_blocker(self.daemon.status.as_ref()) { self.status = StatusMessage::warning(blocker); return Vec::new(); diff --git a/configurator/src/app/update/session_catalog/tests.rs b/configurator/src/app/update/session_catalog/tests.rs index 0db52cacb..cad776983 100644 --- a/configurator/src/app/update/session_catalog/tests.rs +++ b/configurator/src/app/update/session_catalog/tests.rs @@ -142,7 +142,7 @@ fn catalog_load_clears_only_a_session_confirmation() { #[test] fn catalog_load_preserves_a_defaults_confirmation() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); let _ = app.handle_reset_to_defaults_requested(); let _ = app.handle_session_catalog_loaded(Ok(vec![catalog_item("s-1", "Lecture")])); @@ -158,7 +158,7 @@ fn duplicate_request_blocks_without_daemon_status() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = None; + app.daemon.status = None; let effects = app.handle_session_catalog_duplicate_requested("s-1".to_string()); @@ -175,7 +175,7 @@ fn duplicate_request_sets_busy_when_safe() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let effects = app.handle_session_catalog_duplicate_requested("s-1".to_string()); @@ -195,7 +195,7 @@ fn move_request_blocks_without_daemon_status() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = None; + app.daemon.status = None; let effects = app.handle_session_catalog_move_requested("s-1".to_string()); @@ -212,7 +212,7 @@ fn move_request_sets_busy_when_safe() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let effects = app.handle_session_catalog_move_requested("s-1".to_string()); @@ -232,7 +232,7 @@ fn clear_request_blocks_without_daemon_status() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = None; + app.daemon.status = None; let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); @@ -248,7 +248,7 @@ fn clear_tool_state_request_blocks_without_daemon_status() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = None; + app.daemon.status = None; let _ = app.handle_session_catalog_clear_tool_state_requested("s-1".to_string()); @@ -265,7 +265,7 @@ fn clear_tool_state_request_sets_busy_when_safe() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let effects = app.handle_session_catalog_clear_tool_state_requested("s-1".to_string()); @@ -286,7 +286,7 @@ fn clear_request_sets_pending_confirmation_when_safe() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let effects = app.handle_session_catalog_clear_requested("s-1".to_string()); @@ -300,7 +300,7 @@ fn clear_request_sets_pending_confirmation_when_safe() { fn clear_request_rejects_a_session_that_is_no_longer_present() { let (mut app, _effects) = ConfiguratorApp::new_app(); app.session_catalog = SessionCatalogState::loading(); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let effects = app.handle_session_catalog_clear_requested("missing".to_string()); @@ -312,11 +312,11 @@ fn clear_request_rejects_a_session_that_is_no_longer_present() { #[test] fn session_clear_request_replaces_the_defaults_confirmation() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_reset_to_defaults_requested(); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); @@ -329,11 +329,11 @@ fn session_clear_request_replaces_the_defaults_confirmation() { #[test] fn defaults_request_replaces_the_session_clear_confirmation() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); let _ = app.handle_reset_to_defaults_requested(); @@ -349,7 +349,7 @@ fn clear_canceled_disarms_and_clears_its_confirmation_status() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); let effects = app.handle_session_catalog_clear_canceled("s-1".to_string()); @@ -365,7 +365,7 @@ fn active_confirmation_cancel_disarms_session_clear() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); let effects = app.handle_active_confirmation_canceled(); @@ -381,7 +381,7 @@ fn clear_canceled_preserves_status_that_replaced_its_confirmation() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); app.status = StatusMessage::error("A newer session operation failed"); @@ -415,7 +415,7 @@ fn stale_clear_cancel_does_not_disarm_a_newer_confirmation() { catalog_item("s-1", "Lecture"), catalog_item("s-2", "Workshop"), ]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); let _ = app.handle_session_catalog_clear_requested("s-2".to_string()); @@ -437,7 +437,7 @@ fn clear_confirmed_consumes_the_pending_confirmation() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); let effects = app.handle_session_catalog_clear_confirmed("s-1".to_string()); @@ -461,7 +461,7 @@ fn clear_confirmed_twice_starts_only_one_clear() { app.session_catalog = SessionCatalogState::loading(); app.session_catalog .replace_items(vec![catalog_item("s-1", "Lecture")]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); let _ = app.handle_session_catalog_clear_confirmed("s-1".to_string()); @@ -485,7 +485,7 @@ fn clear_confirmed_for_another_row_leaves_the_pending_one_armed() { catalog_item("s-1", "Lecture"), catalog_item("s-2", "Seminar"), ]); - app.daemon_status = Some(inactive_daemon_status()); + app.daemon.status = Some(inactive_daemon_status()); let _ = app.handle_session_catalog_clear_requested("s-1".to_string()); let effects = app.handle_session_catalog_clear_confirmed("s-2".to_string()); diff --git a/configurator/src/app/update/shortcuts.rs b/configurator/src/app/update/shortcuts.rs index 5f03d1d61..ea32631fe 100644 --- a/configurator/src/app/update/shortcuts.rs +++ b/configurator/src/app/update/shortcuts.rs @@ -25,11 +25,8 @@ impl ConfiguratorApp { &mut self, field: KeybindingField, ) -> Vec { - if self.pending_shortcut_conflict.is_some() { - return Vec::new(); - } - self.shortcut_text_editor = None; - self.active_shortcut_recorder = Some(ShortcutRecorderState::new(field)); + self.shortcuts + .begin_recording(ShortcutRecorderState::new(field)); Vec::new() } @@ -37,11 +34,8 @@ impl ConfiguratorApp { &mut self, field: KeybindingField, ) -> Vec { - if self.pending_shortcut_conflict.is_some() { - return Vec::new(); - } - self.shortcut_text_editor = None; - self.active_shortcut_recorder = Some(ShortcutRecorderState::new_sequence(field)); + self.shortcuts + .begin_recording(ShortcutRecorderState::new_sequence(field)); Vec::new() } @@ -49,13 +43,7 @@ impl ConfiguratorApp { &mut self, field: KeybindingField, ) -> Vec { - if self - .active_shortcut_recorder - .as_ref() - .is_some_and(|recorder| recorder.field == field) - { - self.active_shortcut_recorder = None; - } + self.shortcuts.cancel_recording(field); Vec::new() } @@ -64,7 +52,7 @@ impl ConfiguratorApp { keyval: u32, modifiers: KeyboardModifiers, ) -> Vec { - let Some(recorder) = self.active_shortcut_recorder.as_mut() else { + let Some(recorder) = self.shortcuts.recorder_mut() else { return Vec::new(); }; match normalize_key_event(keyval, modifiers) { @@ -80,7 +68,7 @@ impl ConfiguratorApp { let field = recorder.field; let finished = recorder.push_keyboard_step(binding); if let Some(shortcut) = finished { - self.active_shortcut_recorder = None; + self.shortcuts.set_recorder(None); self.commit_recorded_binding(field, shortcut) } else { Vec::new() @@ -95,7 +83,7 @@ impl ConfiguratorApp { kind: RecorderDeviceKind, modifiers: KeyboardModifiers, ) -> Vec { - let Some(recorder) = self.active_shortcut_recorder.as_mut() else { + let Some(recorder) = self.shortcuts.recorder_mut() else { return Vec::new(); }; let field = recorder.field; @@ -109,27 +97,27 @@ impl ConfiguratorApp { recorder.prompt = sequence_keyboard_only_message().to_string(); return Vec::new(); } - self.active_shortcut_recorder = None; + self.shortcuts.set_recorder(None); self.commit_recorded_binding(field, trigger.into()) } } } pub(super) fn handle_shortcut_sequence_finish(&mut self) -> Vec { - let Some(recorder) = self.active_shortcut_recorder.take() else { + let Some(recorder) = self.shortcuts.take_recorder() else { return Vec::new(); }; match recorder.finish_sequence() { Some(shortcut) => self.commit_recorded_binding(recorder.field, shortcut), None => { - self.active_shortcut_recorder = Some(recorder); + self.shortcuts.set_recorder(Some(recorder)); Vec::new() } } } pub(super) fn handle_shortcut_sequence_remove_last_step(&mut self) -> Vec { - if let Some(recorder) = self.active_shortcut_recorder.as_mut() { + if let Some(recorder) = self.shortcuts.recorder_mut() { recorder.remove_last_step(); } Vec::new() @@ -140,7 +128,7 @@ impl ConfiguratorApp { field: KeybindingField, binding: Shortcut, ) -> Vec { - if self.pending_shortcut_conflict.is_some() { + if self.shortcuts.conflict().is_some() { return Vec::new(); } match remove_binding(&mut self.draft.keybindings, field, &binding) { @@ -159,7 +147,7 @@ impl ConfiguratorApp { &mut self, field: KeybindingField, ) -> Vec { - if self.pending_shortcut_conflict.is_some() { + if self.shortcuts.conflict().is_some() { return Vec::new(); } reset_field( @@ -176,46 +164,43 @@ impl ConfiguratorApp { &mut self, field: KeybindingField, ) -> Vec { - if self.pending_shortcut_conflict.is_some() { - return Vec::new(); - } - self.active_shortcut_recorder = None; let text = self .draft .keybindings .value_for(field) .unwrap_or_default() .to_string(); - self.shortcut_text_editor = Some(ShortcutTextEditor::new(field, text)); + self.shortcuts + .begin_text_edit(ShortcutTextEditor::new(field, text)); Vec::new() } pub(super) fn handle_shortcut_text_edit_changed(&mut self, text: String) -> Vec { - if let Some(editor) = self.shortcut_text_editor.as_mut() { + if let Some(editor) = self.shortcuts.editor_mut() { editor.text = text; } Vec::new() } pub(super) fn handle_shortcut_text_edit_applied(&mut self) -> Vec { - let Some(editor) = self.shortcut_text_editor.clone() else { + let Some(editor) = self.shortcuts.editor().cloned() else { return Vec::new(); }; match parse_keybindings(&editor.text) { Ok(parsed) => { let conflicts = text_conflicts_for(&self.draft.keybindings, editor.field, &parsed); if conflicts.is_empty() { - self.shortcut_text_editor = None; + self.shortcuts.set_editor(None); return self .handle_keybinding_changed(editor.field, editor.text.trim().to_string()); } - self.shortcut_text_editor = None; - self.pending_shortcut_conflict = - Some(crate::models::PendingShortcutConflict::Text { + self.shortcuts.set_editor(None); + self.shortcuts + .set_conflict(Some(crate::models::PendingShortcutConflict::Text { target: editor.field, new_value: editor.text, conflicts, - }); + })); } Err(error) => { self.status = StatusMessage::error(error); @@ -228,18 +213,12 @@ impl ConfiguratorApp { &mut self, field: KeybindingField, ) -> Vec { - if self - .shortcut_text_editor - .as_ref() - .is_some_and(|editor| editor.field == field) - { - self.shortcut_text_editor = None; - } + self.shortcuts.cancel_text_edit(field); Vec::new() } pub(super) fn handle_shortcut_conflict_replace_confirmed(&mut self) -> Vec { - let Some(pending) = self.pending_shortcut_conflict.take() else { + let Some(pending) = self.shortcuts.take_conflict() else { return Vec::new(); }; let result = match pending { @@ -257,7 +236,7 @@ impl ConfiguratorApp { match result { Ok(()) => { self.refresh_dirty_flag(); - if self.shortcut_conflict_review { + if self.shortcuts.review { self.arm_next_shortcut_conflict(); } else { self.status = StatusMessage::idle(); @@ -271,8 +250,8 @@ impl ConfiguratorApp { } pub(super) fn handle_shortcut_conflict_canceled(&mut self) -> Vec { - self.pending_shortcut_conflict = None; - self.shortcut_conflict_review = false; + self.shortcuts.set_conflict(None); + self.shortcuts.review = false; Vec::new() } @@ -342,7 +321,10 @@ impl ConfiguratorApp { } pub(super) fn handle_shortcut_reset_visible_requested(&mut self) -> Vec { - if self.is_loading || self.is_saving || self.shortcut_reset_visible_pending() { + if self.document.is_loading() + || self.document.is_saving() + || self.shortcut_reset_visible_pending() + { return Vec::new(); } let fields = self.visible_keybinding_fields(); @@ -371,7 +353,10 @@ impl ConfiguratorApp { } pub(super) fn handle_shortcut_reset_all_requested(&mut self) -> Vec { - if self.is_loading || self.is_saving || self.shortcut_reset_all_pending() { + if self.document.is_loading() + || self.document.is_saving() + || self.shortcut_reset_all_pending() + { return Vec::new(); } self.pending_confirmation = Some(PendingConfirmation::ShortcutResetAll); @@ -400,12 +385,12 @@ impl ConfiguratorApp { pub(super) fn handle_shortcut_conflict_review_started(&mut self) -> Vec { if !self.shortcut_manager_summary().has_conflicts() { - self.shortcut_conflict_review = false; + self.shortcuts.review = false; self.status = StatusMessage::info("No shortcut conflicts to review."); return Vec::new(); } - self.shortcut_conflict_review = true; - if self.pending_shortcut_conflict.is_some() { + self.shortcuts.review = true; + if self.shortcuts.conflict().is_some() { return Vec::new(); } self.arm_next_shortcut_conflict(); @@ -416,16 +401,17 @@ impl ConfiguratorApp { match next_review_conflict(&self.draft.keybindings) { Some((field, binding, claimants)) => { self.select_keybinding_field(field); - self.pending_shortcut_conflict = - Some(crate::models::PendingShortcutConflict::Recorded { + self.shortcuts.set_conflict(Some( + crate::models::PendingShortcutConflict::Recorded { target: field, binding, claimants, - }); + }, + )); self.status = StatusMessage::info("Review the next conflicting shortcut."); } None => { - self.shortcut_conflict_review = false; + self.shortcuts.review = false; self.status = StatusMessage::success("No remaining shortcut conflicts."); } } @@ -438,12 +424,12 @@ impl ConfiguratorApp { ) -> Vec { let claimants = other_claimants(&self.draft.keybindings, field, &binding); if !claimants.is_empty() { - self.pending_shortcut_conflict = - Some(crate::models::PendingShortcutConflict::Recorded { + self.shortcuts + .set_conflict(Some(crate::models::PendingShortcutConflict::Recorded { target: field, binding, claimants, - }); + })); return Vec::new(); } match append_binding(&mut self.draft.keybindings, field, &binding) { diff --git a/configurator/src/app/update/shortcuts/tests.rs b/configurator/src/app/update/shortcuts/tests.rs index fe6ad342e..8cde91995 100644 --- a/configurator/src/app/update/shortcuts/tests.rs +++ b/configurator/src/app/update/shortcuts/tests.rs @@ -57,9 +57,7 @@ fn starting_one_recorder_closes_any_older_recorder() { let _ = app.handle_shortcut_recording_started(KeybindingField::ClearCanvas); let _ = app.handle_shortcut_recording_started(KeybindingField::Undo); assert_eq!( - app.active_shortcut_recorder - .as_ref() - .map(|recorder| recorder.field), + app.shortcuts.recorder().map(|recorder| recorder.field), Some(KeybindingField::Undo) ); } @@ -82,14 +80,14 @@ fn confirmation_and_shortcut_conflict_do_not_consume_each_other() { app.pending_confirmation.is_some(), "recording a conflict must not disarm Defaults" ); - assert!(app.pending_shortcut_conflict.is_some()); + assert!(app.shortcuts.conflict().is_some()); let _ = app.handle_window_escape_pressed(); assert!( app.pending_confirmation.is_none(), "Escape still cancels Defaults when the recorder is closed" ); assert!( - app.pending_shortcut_conflict.is_some(), + app.shortcuts.conflict().is_some(), "Escape must not take the shortcut conflict with it" ); } @@ -144,7 +142,7 @@ fn recorder_escape_does_not_cancel_defaults_confirmation() { let _ = app.handle_shortcut_recording_started(KeybindingField::ToggleFloatingBadge); let _ = app.handle_window_escape_pressed(); assert!(app.pending_confirmation.is_some()); - assert!(app.active_shortcut_recorder.is_some()); + assert!(app.shortcuts.recorder().is_some()); } #[test] @@ -153,9 +151,9 @@ fn conflict_cancel_leaves_the_draft_byte_for_byte() { let before = app.draft.clone(); let _ = app.handle_shortcut_recording_started(KeybindingField::ToggleFloatingBadge); let _ = app.handle_shortcut_recorder_key(u32::from(b'e'), KeyboardModifiers::default()); - assert!(app.pending_shortcut_conflict.is_some()); + assert!(app.shortcuts.conflict().is_some()); let _ = app.handle_shortcut_conflict_canceled(); - assert!(app.pending_shortcut_conflict.is_none()); + assert!(app.shortcuts.conflict().is_none()); assert_eq!(app.draft, before); } @@ -171,7 +169,7 @@ fn invalid_raw_text_blocks_save_and_stays_visible() { .set(KeybindingField::Exit, "Ctrl+Shift".to_string()); let effects = app.handle_save_requested(); assert!(effects.is_empty()); - assert!(app.base_document.is_some()); + assert!(app.document.loaded().is_some()); assert_eq!( app.draft.keybindings.value_for(KeybindingField::Exit), Some("Ctrl+Shift") @@ -198,7 +196,7 @@ fn save_after_confirmed_replacement_writes_only_the_intended_fields() { super_held: false, }, ); - assert!(app.pending_shortcut_conflict.is_some()); + assert!(app.shortcuts.conflict().is_some()); let _ = app.handle_shortcut_conflict_replace_confirmed(); save_draft(&mut app); @@ -231,7 +229,7 @@ fn pending_conflict_blocks_save_without_taking_the_document() { let _ = app.handle_shortcut_recorder_key(u32::from(b'e'), KeyboardModifiers::default()); let effects = app.handle_save_requested(); assert!(effects.is_empty()); - assert!(app.base_document.is_some()); + assert!(app.document.loaded().is_some()); assert!(status_contains(&app.status, "shortcut conflict")); } @@ -243,9 +241,9 @@ fn text_editor_keeps_invalid_text_until_canceled() { let _ = app.handle_shortcut_text_edit_changed("Ctrl+Shift".to_string()); let _ = app.handle_shortcut_text_edit_applied(); assert_eq!(app.draft, before); - assert!(app.shortcut_text_editor.is_some()); + assert!(app.shortcuts.editor().is_some()); let _ = app.handle_shortcut_text_edit_canceled(KeybindingField::Undo); - assert!(app.shortcut_text_editor.is_none()); + assert!(app.shortcuts.editor().is_none()); assert_eq!(app.draft, before); } @@ -347,7 +345,7 @@ fn active_confirmation_canceled_clears_defaults_without_touching_conflicts() { let effects = app.handle_active_confirmation_canceled(); assert!(effects.is_empty()); assert!(app.pending_confirmation.is_none()); - assert!(app.pending_shortcut_conflict.is_some()); + assert!(app.shortcuts.conflict().is_some()); } #[test] @@ -360,7 +358,7 @@ fn auxiliary_mouse_button_records_into_the_draft() { KeyboardModifiers::default(), ); assert!(effects.is_empty()); - assert!(app.active_shortcut_recorder.is_none()); + assert!(app.shortcuts.recorder().is_none()); assert_eq!( app.draft .keybindings @@ -384,8 +382,8 @@ fn recording_stylus_primary_prompts_to_move_the_default_legacy_barrel() { KeyboardModifiers::default(), ); let pending = app - .pending_shortcut_conflict - .as_ref() + .shortcuts + .conflict() .expect("legacy barrel is already assigned"); assert_eq!(pending.replace_label(), "Move Legacy Binding"); let effects = app.handle_shortcut_conflict_replace_confirmed(); @@ -411,17 +409,17 @@ fn recording_a_two_step_sequence_commits_on_finish() { }; let effects = app.handle_shortcut_recorder_key(u32::from(b'k'), chord); assert!(effects.is_empty()); - assert!(app.active_shortcut_recorder.is_some()); + assert!(app.shortcuts.recorder().is_some()); let effects = app.handle_shortcut_recorder_key(u32::from(b'c'), chord); assert!(effects.is_empty()); assert!( - app.active_shortcut_recorder - .as_ref() + app.shortcuts + .recorder() .is_some_and(|recorder| recorder.can_finish()) ); let effects = app.handle_shortcut_sequence_finish(); assert!(effects.is_empty()); - assert!(app.active_shortcut_recorder.is_none()); + assert!(app.shortcuts.recorder().is_none()); assert_eq!( app.draft .keybindings @@ -444,7 +442,7 @@ fn third_sequence_step_finishes_automatically() { let _ = app.handle_shortcut_recorder_key(u32::from(b'c'), chord); let effects = app.handle_shortcut_recorder_key(u32::from(b'v'), chord); assert!(effects.is_empty()); - assert!(app.active_shortcut_recorder.is_none()); + assert!(app.shortcuts.recorder().is_none()); assert_eq!( app.draft .keybindings @@ -463,10 +461,10 @@ fn sequence_recording_rejects_device_buttons() { KeyboardModifiers::default(), ); assert!(effects.is_empty()); - assert!(app.active_shortcut_recorder.is_some()); + assert!(app.shortcuts.recorder().is_some()); assert!( - app.active_shortcut_recorder - .as_ref() + app.shortcuts + .recorder() .is_some_and(|recorder| recorder.prompt.contains("keyboard-only")) ); } @@ -485,8 +483,8 @@ fn sequence_prefix_conflict_uses_the_same_replace_flow() { let _ = app.handle_shortcut_recorder_key(u32::from(b'c'), ctrl); let _ = app.handle_shortcut_sequence_finish(); let pending = app - .pending_shortcut_conflict - .as_ref() + .shortcuts + .conflict() .expect("Ctrl+K is the command palette"); let prompt = pending.prompt(); assert!(prompt.contains("Ctrl+K"), "{prompt}"); @@ -506,7 +504,7 @@ fn text_editor_accepts_a_sequence_beside_a_single() { .handle_shortcut_text_edit_changed("F5, Ctrl+Alt+Shift+K > Ctrl+Alt+Shift+C".to_string()); let effects = app.handle_shortcut_text_edit_applied(); assert!(effects.is_empty()); - assert!(app.pending_shortcut_conflict.is_none()); + assert!(app.shortcuts.conflict().is_none()); assert_eq!( app.draft .keybindings @@ -536,7 +534,7 @@ fn reset_visible_affects_exactly_the_filtered_identity_set() { use crate::models::ShortcutManagerFilter; let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.keybindings_show_all = true; app.draft .keybindings @@ -593,7 +591,7 @@ fn reset_visible_does_not_touch_fields_outside_the_filter() { use crate::models::ShortcutManagerFilter; let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.keybindings_show_all = true; app.draft .keybindings @@ -623,7 +621,7 @@ fn reset_visible_does_not_touch_fields_outside_the_filter() { #[test] fn reset_all_requires_confirmation_and_stays_draft_only() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft .keybindings .set(KeybindingField::Undo, "F9".to_string()); @@ -657,7 +655,7 @@ fn reset_all_requires_confirmation_and_stays_draft_only() { #[test] fn reset_all_confirm_without_request_changes_nothing() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft .keybindings .set(KeybindingField::Undo, "F9".to_string()); @@ -671,7 +669,7 @@ fn reset_all_confirm_without_request_changes_nothing() { #[test] fn conflict_review_queue_arms_the_next_conflict_after_replace() { let (mut app, _effects) = ConfiguratorApp::new_app(); - app.is_loading = false; + app.document.set_loading_for_test(false); app.draft .keybindings .set(KeybindingField::ClearCanvas, "Ctrl+Shift+Q".to_string()); @@ -686,21 +684,21 @@ fn conflict_review_queue_arms_the_next_conflict_after_replace() { .set(KeybindingField::Redo, "Ctrl+Alt+Shift+Y".to_string()); let _ = app.handle_shortcut_conflict_review_started(); - assert!(app.shortcut_conflict_review); - assert!(app.pending_shortcut_conflict.is_some()); - let first_target = match app.pending_shortcut_conflict.as_ref() { + assert!(app.shortcuts.review); + assert!(app.shortcuts.conflict().is_some()); + let first_target = match app.shortcuts.conflict() { Some(crate::models::PendingShortcutConflict::Recorded { target, .. }) => *target, other => panic!("expected a recorded conflict, got {other:?}"), }; let _ = app.handle_shortcut_conflict_replace_confirmed(); assert!( - app.shortcut_conflict_review, + app.shortcuts.review, "the queue continues after one replace" ); let second = app - .pending_shortcut_conflict - .as_ref() + .shortcuts + .conflict() .expect("the next conflict should be armed"); match second { crate::models::PendingShortcutConflict::Recorded { target, .. } => { @@ -721,8 +719,8 @@ fn conflict_review_cancel_stops_the_queue() { .set(KeybindingField::ToggleToolbar, "Ctrl+Shift+Q".to_string()); let _ = app.handle_shortcut_conflict_review_started(); let _ = app.handle_shortcut_conflict_canceled(); - assert!(!app.shortcut_conflict_review); - assert!(app.pending_shortcut_conflict.is_none()); + assert!(!app.shortcuts.review); + assert!(app.shortcuts.conflict().is_none()); } #[test] @@ -736,8 +734,8 @@ fn jump_to_conflict_selects_the_other_claimant() { .set(KeybindingField::ToggleToolbar, "Ctrl+Shift+Q".to_string()); let _ = app.handle_shortcut_conflict_review_started(); let jump = app - .pending_shortcut_conflict - .as_ref() + .shortcuts + .conflict() .and_then(crate::models::PendingShortcutConflict::jump_field) .expect("a claimant to jump to"); let _ = app.handle_shortcut_manager_jump_to(jump); diff --git a/docs/codebase-overview.md b/docs/codebase-overview.md index ef38e370b..dcf9a8e21 100644 --- a/docs/codebase-overview.md +++ b/docs/codebase-overview.md @@ -185,6 +185,17 @@ separate lifecycles. --- +Canvas selection edits use `src/input/state/core/editing.rs::CanvasEdit`. It captures +unlocked shapes, applies previews, commits one history entry, and restores snapshots +on cancellation. `EditEffects` carries cache, redraw, and persistence work back to +`InputState`. Translation previews avoid cloning shape paths on every motion event; +resize previews borrow the gesture's starting snapshots. A multi-shape resize is one +Undo/Redo operation, and an unchanged final snapshot adds no history entry. + +The command palette prepares a `CommandPaletteView` before painting. Its list rows, +geometry, query, selection, shortcut labels, and tooltip are application values; +the painter only consumes the view and the retained `UiTextEngine`. + ## 5. Capture Pipeline **New structure (all under `src/capture/`):** @@ -241,6 +252,11 @@ Notifications are sent via `notification::send_notification_async`, keeping all --- +Frozen and zoom preflight use `CapturePreflight`: `Pending` carries backend policy +and the source layout identity; dispatch changes it to `Capturing` while retaining +that identity for stale-layout checks and fallback. Terminal cleanup resets the +phase. Their backend selection policies and acquisition identities remain separate. + ## 6. Toolbar Frontends - `src/ui/toolbar/model/top_spec.rs` owns the renderer-neutral top-toolbar contract: stable @@ -259,6 +275,10 @@ Notifications are sent via `notification::send_notification_async`, keeping all --- +Each GTK top-strip `PopoverOwner` groups its mounted popover and capture surface, +expected-open flag, content key, and value updaters. It owns open/close and teardown; +capture suppression operates on the paired resources without runtime pairing checks. + ## 7. Domain Values and Dependency Direction - **`src/domain/`** is the canonical owner of dependency-light action, tool, color, and board diff --git a/src/backend/wayland/capture_preflight.rs b/src/backend/wayland/capture_preflight.rs new file mode 100644 index 000000000..62e283bcf --- /dev/null +++ b/src/backend/wayland/capture_preflight.rs @@ -0,0 +1,76 @@ +//! Capture admission and the layout identity retained through fallback. +#[derive(Debug, Clone, Copy)] +pub(super) struct CaptureLayout { + output_id: Option, + generation: u64, +} + +#[derive(Debug, Clone, Copy, Default)] +pub(super) enum CapturePreflight { + #[default] + Idle, + Pending { + backend: B, + layout: CaptureLayout, + }, + Capturing { + layout: CaptureLayout, + }, +} + +impl CapturePreflight { + pub(super) fn begin(&mut self, backend: B, output_id: Option, generation: u64) { + *self = Self::Pending { + backend, + layout: CaptureLayout { + output_id, + generation, + }, + }; + } + + pub(super) fn is_pending(&self) -> bool { + matches!(self, Self::Pending { .. }) + } + + pub(super) fn take_pending(&mut self) -> Option { + let Self::Pending { backend, layout } = *self else { + return None; + }; + *self = Self::Capturing { layout }; + Some(backend) + } + + pub(super) fn layout_matches(&self, output_id: Option, generation: u64) -> bool { + let layout = match self { + Self::Idle => return true, + Self::Pending { layout, .. } | Self::Capturing { layout } => layout, + }; + super::portal_capture::layout_token_matches( + layout.output_id, + layout.generation, + output_id, + generation, + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn dispatch_retains_layout_until_reset() { + let mut phase = CapturePreflight::default(); + phase.begin(7, Some(3), 10); + assert!(phase.is_pending()); + assert_eq!(phase.take_pending(), Some(7)); + assert!(!phase.is_pending()); + assert_eq!(phase.take_pending(), None); + assert!(phase.layout_matches(Some(3), 10)); + assert!(!phase.layout_matches(Some(4), 10)); + assert!(!phase.layout_matches(Some(3), 11)); + phase = CapturePreflight::Idle; + assert!(phase.layout_matches(Some(4), 11)); + } +} diff --git a/src/backend/wayland/frozen/capture.rs b/src/backend/wayland/frozen/capture.rs index f049158c6..b237c2b3e 100644 --- a/src/backend/wayland/frozen/capture.rs +++ b/src/backend/wayland/frozen/capture.rs @@ -84,18 +84,20 @@ impl CaptureSession { impl FrozenState { /// Start a screencopy capture for the active output. pub fn start_capture(&mut self) -> Result<()> { - if self.direct_capture.is_some() || self.portal_in_progress || self.preflight_pending { + if self.direct_capture.is_some() || self.portal_in_progress || self.preflight.is_pending() { warn!("Frozen-mode capture already in progress; ignoring toggle"); return Ok(()); } self.capture_done = false; - self.preflight_backend = Some( - self.preferred_backend() - .context("no frozen capture backend is available")?, + let backend = self + .preferred_backend() + .context("no frozen capture backend is available")?; + self.preflight.begin( + backend, + self.active_output_id, + self.output_layout_generation, ); - self.snapshot_preflight_layout(); - self.preflight_pending = true; Ok(()) } diff --git a/src/backend/wayland/frozen/state.rs b/src/backend/wayland/frozen/state.rs index 6f983ea5b..b3401c20c 100644 --- a/src/backend/wayland/frozen/state.rs +++ b/src/backend/wayland/frozen/state.rs @@ -1,3 +1,4 @@ +use crate::backend::wayland::capture_preflight::CapturePreflight; use log::info; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -132,10 +133,7 @@ pub struct FrozenState { pub(super) portal_in_progress: bool, pub(super) portal_target_output_id: Option, pub(super) runtime_wake: Option, - pub(super) preflight_pending: bool, - pub(super) preflight_backend: Option, - preflight_output_id: Option, - preflight_layout_generation: Option, + pub(super) preflight: CapturePreflight, pub(super) capture_done: bool, pending_image: Option, acquisition_attempt: Option<(ScreenAcquisitionId, ScreenAcquisitionOwner)>, @@ -201,10 +199,7 @@ impl FrozenState { portal_in_progress: false, portal_target_output_id: None, runtime_wake, - preflight_pending: false, - preflight_backend: None, - preflight_output_id: None, - preflight_layout_generation: None, + preflight: CapturePreflight::Idle, capture_done: false, pending_image: None, acquisition_attempt: None, @@ -352,35 +347,30 @@ impl FrozenState { pub fn is_in_progress(&self) -> bool { self.direct_capture.is_some() || self.portal_in_progress - || self.preflight_pending + || self.preflight.is_pending() || self.pending_image.is_some() } pub(in crate::backend::wayland) fn take_preflight_pending( &mut self, ) -> Option { - if !self.preflight_pending { - return None; - } - self.preflight_pending = false; - self.preflight_backend.take() + self.preflight.take_pending() } + #[cfg(test)] pub(super) fn snapshot_preflight_layout(&mut self) { - self.preflight_output_id = self.active_output_id; - self.preflight_layout_generation = Some(self.output_layout_generation); + self.preflight.begin( + FrozenCaptureBackend::Portal, + self.active_output_id, + self.output_layout_generation, + ); } pub(super) fn ensure_preflight_layout_current(&self) -> Result<(), String> { - let Some(generation) = self.preflight_layout_generation else { - return Ok(()); - }; - if layout_token_matches( - self.preflight_output_id, - generation, - self.active_output_id, - self.output_layout_generation, - ) { + if self + .preflight + .layout_matches(self.active_output_id, self.output_layout_generation) + { Ok(()) } else { Err("Freeze failed after the display layout changed".to_string()) @@ -462,10 +452,7 @@ impl FrozenState { if let Some(capture) = self.direct_capture.take() { capture.destroy(); } - self.preflight_pending = false; - self.preflight_backend = None; - self.preflight_output_id = None; - self.preflight_layout_generation = None; + self.preflight = CapturePreflight::Idle; self.portal_in_progress = false; if let Some(mut task) = self.portal_task.take() { task.cancel(); diff --git a/src/backend/wayland/mod.rs b/src/backend/wayland/mod.rs index e18aa58aa..ec46ae63f 100644 --- a/src/backend/wayland/mod.rs +++ b/src/backend/wayland/mod.rs @@ -1,6 +1,7 @@ mod acquisition; mod backend; mod capture; +mod capture_preflight; mod clipboard; mod config_edits; mod frozen; diff --git a/src/backend/wayland/state/render/ui.rs b/src/backend/wayland/state/render/ui.rs index 6713f8429..17471777f 100644 --- a/src/backend/wayland/state/render/ui.rs +++ b/src/backend/wayland/state/render/ui.rs @@ -431,13 +431,8 @@ impl WaylandState { &card, ); } - crate::ui::render_command_palette_with_engine( - self.render.ui_text(), - ctx, - &self.input_state, - width, - height, - ); + let palette_view = crate::ui::CommandPaletteView::prepare(&self.input_state, width, height); + crate::ui::paint_command_palette(self.render.ui_text(), ctx, &palette_view, width, height); crate::ui::render_tour_with_engine( self.render.ui_text(), ctx, diff --git a/src/backend/wayland/zoom/capture.rs b/src/backend/wayland/zoom/capture.rs index 511595dbd..1115d186f 100644 --- a/src/backend/wayland/zoom/capture.rs +++ b/src/backend/wayland/zoom/capture.rs @@ -126,16 +126,18 @@ impl ZoomState { self.source_terminal.is_none(), "a zoom capture terminal is still pending" ); - if self.capture.is_some() || self.portal_in_progress || self.preflight_pending { + if self.capture.is_some() || self.portal_in_progress || self.preflight.is_pending() { warn!("Zoom capture already in progress; ignoring request"); return Ok(()); } self.begin_identified_capture(); self.capture_done = false; - self.preflight_use_fallback = use_fallback || self.manager.is_none(); - self.snapshot_preflight_layout(); - self.preflight_pending = true; + self.preflight.begin( + use_fallback || self.manager.is_none(), + self.active_output_id, + self.output_layout_generation, + ); Ok(()) } diff --git a/src/backend/wayland/zoom/state.rs b/src/backend/wayland/zoom/state.rs index 2416c43a3..57ee7b4b2 100644 --- a/src/backend/wayland/zoom/state.rs +++ b/src/backend/wayland/zoom/state.rs @@ -1,3 +1,4 @@ +use crate::backend::wayland::capture_preflight::CapturePreflight; use std::sync::Arc; use wayland_client::protocol::wl_output; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1; @@ -5,7 +6,6 @@ use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::Z use crate::backend::wayland::RuntimeWakeHandle; use crate::backend::wayland::frozen::{FrozenImage, ScreenImageProvenance}; use crate::backend::wayland::frozen_geometry::OutputGeometry; -use crate::backend::wayland::portal_capture::layout_token_matches; use crate::backend::wayland::portal_task::PortalTask; use crate::input::InputState; @@ -117,10 +117,7 @@ pub struct ZoomState { pub(super) portal_in_progress: bool, pub(super) portal_target_output_id: Option, pub(super) runtime_wake: Option, - pub(super) preflight_pending: bool, - pub(super) preflight_use_fallback: bool, - preflight_output_id: Option, - preflight_layout_generation: Option, + pub(super) preflight: CapturePreflight, pub(super) capture_done: bool, next_capture_id: u64, current_capture_id: Option, @@ -166,10 +163,7 @@ impl ZoomState { portal_in_progress: false, portal_target_output_id: None, runtime_wake, - preflight_pending: false, - preflight_use_fallback: false, - preflight_output_id: None, - preflight_layout_generation: None, + preflight: CapturePreflight::Idle, capture_done: false, next_capture_id: 1, current_capture_id: None, @@ -279,39 +273,29 @@ impl ZoomState { } pub fn is_in_progress(&self) -> bool { - self.capture.is_some() || self.portal_in_progress || self.preflight_pending + self.capture.is_some() || self.portal_in_progress || self.preflight.is_pending() } #[cfg(test)] pub fn preflight_pending(&self) -> bool { - self.preflight_pending + self.preflight.is_pending() } pub fn take_preflight_pending(&mut self) -> Option { - if !self.preflight_pending { - return None; - } - let use_fallback = self.preflight_use_fallback; - self.preflight_pending = false; - self.preflight_use_fallback = false; - Some(use_fallback) + self.preflight.take_pending() } + #[cfg(test)] pub(super) fn snapshot_preflight_layout(&mut self) { - self.preflight_output_id = self.active_output_id; - self.preflight_layout_generation = Some(self.output_layout_generation); + self.preflight + .begin(true, self.active_output_id, self.output_layout_generation); } pub(super) fn ensure_preflight_layout_current(&self) -> Result<(), String> { - let Some(generation) = self.preflight_layout_generation else { - return Ok(()); - }; - if layout_token_matches( - self.preflight_output_id, - generation, - self.active_output_id, - self.output_layout_generation, - ) { + if self + .preflight + .layout_matches(self.active_output_id, self.output_layout_generation) + { Ok(()) } else { Err("Zoom failed after the display layout changed".to_string()) @@ -327,11 +311,6 @@ impl ZoomState { self.cancel_with_outcome(input_state, false, ZoomSourceOutcome::StaleLayout); } - fn clear_preflight_layout_snapshot(&mut self) { - self.preflight_output_id = None; - self.preflight_layout_generation = None; - } - pub fn take_capture_done(&mut self) -> bool { let done = self.capture_done; self.capture_done = false; @@ -407,12 +386,10 @@ impl ZoomState { capture.frame.destroy(); changed = true; } - if self.preflight_pending || self.portal_in_progress { + if self.preflight.is_pending() || self.portal_in_progress { changed = true; } - self.preflight_pending = false; - self.preflight_use_fallback = false; - self.clear_preflight_layout_snapshot(); + self.preflight = CapturePreflight::Idle; self.portal_in_progress = false; if let Some(mut task) = self.portal_task.take() { task.cancel(); @@ -491,9 +468,7 @@ impl ZoomState { if let Some(capture) = self.capture.take() { capture.frame.destroy(); } - self.preflight_pending = false; - self.preflight_use_fallback = false; - self.clear_preflight_layout_snapshot(); + self.preflight = CapturePreflight::Idle; self.capture_done = true; self.portal_in_progress = false; if let Some(mut task) = self.portal_task.take() { diff --git a/src/config/mod.rs b/src/config/mod.rs index 718605b8e..5bdc89fb8 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -98,7 +98,7 @@ pub(crate) use types::{ #[allow(unused_imports)] pub use validate::{ ConfigValidationReport, DefaultShortcutSkipped, InvalidKeybinding, - KeybindingConflictResolution, KeybindingProblem, + KeybindingConflictResolution, KeybindingProblem, SaveValidationError, }; // Re-export for public API (unused internally but part of public interface) diff --git a/src/config/validate/mod.rs b/src/config/validate/mod.rs index 825cd22ff..b10b4020e 100644 --- a/src/config/validate/mod.rs +++ b/src/config/validate/mod.rs @@ -12,7 +12,9 @@ mod keybindings; mod performance; mod presets; mod render_profiles; +mod save; mod session; +pub use save::SaveValidationError; mod spotlight; #[cfg(feature = "tablet-input")] mod tablet; diff --git a/src/config/validate/save.rs b/src/config/validate/save.rs new file mode 100644 index 000000000..cb2463ba0 --- /dev/null +++ b/src/config/validate/save.rs @@ -0,0 +1,71 @@ +use super::{Config, ConfigValidationReport}; + +/// Why an authored configuration cannot be saved without losing user input. +#[derive(Debug)] +pub enum SaveValidationError { + CorrectedValues, + Representation(toml::ser::Error), +} + +impl std::fmt::Display for SaveValidationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::CorrectedValues => f.write_str("Some values are outside their allowed ranges and would be changed on save. Fix them before saving."), + Self::Representation(error) => write!(f, "Configuration could not be represented for validation: {error}"), + } + } +} +impl std::error::Error for SaveValidationError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Representation(error) => Some(error), + Self::CorrectedValues => None, + } + } +} +impl From for SaveValidationError { + fn from(error: toml::ser::Error) -> Self { + Self::Representation(error) + } +} + +impl Config { + /// Validate an authored save, rejecting corrections outside keybindings. + /// Keybinding arbitration is intentional and returned for user feedback. + pub fn validate_for_save( + mut self, + ) -> Result<(Self, ConfigValidationReport), SaveValidationError> { + let mut before = toml::Value::try_from(&self)?; + let report = self.validate_and_clamp(); + let mut after = toml::Value::try_from(&self)?; + // Compare persisted typed values, independent of diagnostic formatting + // and document-only keybinding authorship metadata. + for value in [&mut before, &mut after] { + if let Some(table) = value.as_table_mut() { + table.remove("keybindings"); + } + } + if before != after { + return Err(SaveValidationError::CorrectedValues); + } + Ok((self, report)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn authored_out_of_range_values_are_rejected() { + let mut config = Config::default(); + config.drawing.default_thickness = 999.0; + assert!(matches!( + config.validate_for_save(), + Err(SaveValidationError::CorrectedValues) + )); + } + #[test] + fn unchanged_values_are_accepted() { + assert!(Config::default().validate_for_save().is_ok()); + } +} diff --git a/src/draw/frame/types.rs b/src/draw/frame/types.rs index d38378b11..8d08272b8 100644 --- a/src/draw/frame/types.rs +++ b/src/draw/frame/types.rs @@ -93,7 +93,7 @@ impl DrawnShape { } /// Snapshot of a shape used for undo/redo of modifications. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct ShapeSnapshot { pub shape: Shape, pub locked: bool, diff --git a/src/draw/shape/types.rs b/src/draw/shape/types.rs index 64be2a0aa..da0d451c9 100644 --- a/src/draw/shape/types.rs +++ b/src/draw/shape/types.rs @@ -24,7 +24,7 @@ pub struct EmbeddedImage { } /// Brush options for eraser strokes. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct EraserBrush { /// Brush diameter in pixels (logical coordinates) pub size: f64, @@ -168,7 +168,7 @@ impl ArrowStyle { } /// Label metadata for numbered arrows. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct ArrowLabel { /// Numeric label value. pub value: u32, @@ -179,7 +179,7 @@ pub struct ArrowLabel { } /// Label metadata for numbered step markers. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct StepMarkerLabel { /// Numeric label value. pub value: u32, @@ -193,7 +193,7 @@ pub struct StepMarkerLabel { /// /// Each variant represents a different drawing tool/primitive with its specific parameters. /// All shapes store their own color and size information for independent rendering. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub enum Shape { /// Freehand drawing - polyline connecting mouse drag points Freehand { diff --git a/src/input/state/core/editing.rs b/src/input/state/core/editing.rs new file mode 100644 index 000000000..6685cc213 --- /dev/null +++ b/src/input/state/core/editing.rs @@ -0,0 +1,243 @@ +//! Canvas mutation transactions. InputState applies UI and persistence effects. +use crate::draw::frame::{Frame, ShapeSnapshot, UndoAction}; +use crate::draw::{Shape, ShapeId, TextMeasurer}; +use crate::util::Rect; +use std::borrow::Cow; + +pub(in crate::input::state) struct CanvasEdit<'a> { + snapshots: Cow<'a, [(ShapeId, ShapeSnapshot)]>, +} + +#[derive(Default)] +#[must_use = "apply edit effects to keep canvas caches, redraw, and persistence coherent"] +pub(in crate::input::state) struct EditEffects { + regions: Vec<(ShapeId, Option, Option)>, + committed: bool, +} + +impl<'a> CanvasEdit<'a> { + pub(in crate::input::state) fn capture(frame: &Frame, ids: &[ShapeId]) -> Self { + Self { + snapshots: Cow::Owned( + ids.iter() + .filter_map(|id| { + let drawn = frame.shape(*id)?; + (!drawn.locked).then(|| { + ( + *id, + ShapeSnapshot { + shape: drawn.shape.clone(), + locked: drawn.locked, + }, + ) + }) + }) + .collect(), + ), + } + } + + pub(in crate::input::state) fn from_snapshots( + snapshots: Vec<(ShapeId, ShapeSnapshot)>, + ) -> Self { + Self { + snapshots: Cow::Owned(snapshots), + } + } + + /// Live resize borrows the gesture's original snapshots; motion events + /// must not clone whole freehand paths just to read their starting geometry. + pub(in crate::input::state) fn borrow_snapshots( + snapshots: &'a [(ShapeId, ShapeSnapshot)], + ) -> Self { + Self { + snapshots: Cow::Borrowed(snapshots), + } + } + + pub(in crate::input::state) fn preview_current( + frame: &mut Frame, + ids: &[ShapeId], + measurer: &TextMeasurer, + mut apply: impl FnMut(&mut Shape) -> bool, + ) -> EditEffects { + let mut effects = EditEffects::default(); + for id in ids { + effects.preview_shape(frame, *id, measurer, &mut apply); + } + effects + } + + pub(in crate::input::state) fn into_snapshots(self) -> Vec<(ShapeId, ShapeSnapshot)> { + self.snapshots.into_owned() + } + + pub(in crate::input::state) fn preview( + &self, + frame: &mut Frame, + measurer: &TextMeasurer, + mut apply: impl FnMut(&mut Shape, &ShapeSnapshot) -> bool, + ) -> EditEffects { + let mut effects = EditEffects::default(); + for (id, snapshot) in self.snapshots.iter() { + effects.preview_shape(frame, *id, measurer, |shape| apply(shape, snapshot)); + } + effects + } + + pub(in crate::input::state) fn commit(self, frame: &mut Frame, limit: usize) -> EditEffects { + let actions = self + .snapshots + .into_owned() + .into_iter() + .filter_map(|(id, before)| { + let drawn = frame.shape(id)?; + let after = ShapeSnapshot { + shape: drawn.shape.clone(), + locked: drawn.locked, + }; + (before != after).then(|| UndoAction::modify_from_snapshots(id, before, after)) + }) + .collect(); + EditEffects { + committed: record(frame, limit, actions), + ..EditEffects::default() + } + } + + pub(in crate::input::state) fn rollback( + self, + frame: &mut Frame, + measurer: &TextMeasurer, + ) -> EditEffects { + let mut effects = EditEffects::default(); + for (id, snapshot) in self.snapshots.into_owned() { + let Some(drawn) = frame.shape_mut(id) else { + continue; + }; + let before = drawn.bounding_box_with(measurer); + drawn.set_shape(snapshot.shape); + drawn.locked = snapshot.locked; + effects + .regions + .push((id, before, drawn.bounding_box_with(measurer))); + } + effects + } + + pub(in crate::input::state) fn apply_selection( + frame: &mut Frame, + ids: &[ShapeId], + measurer: &TextMeasurer, + limit: usize, + mut applicable: impl FnMut(&Shape) -> bool, + mut apply: impl FnMut(&mut Shape) -> bool, + ) -> (usize, usize, usize, EditEffects) { + let mut applicable_count = 0; + let mut locked = 0; + let mut editable = Vec::new(); + for id in ids { + let Some(drawn) = frame.shape(*id) else { + continue; + }; + if !applicable(&drawn.shape) { + continue; + } + applicable_count += 1; + if drawn.locked { + locked += 1; + } else { + editable.push(*id); + } + } + let edit = Self::capture(frame, &editable); + let mut effects = edit.preview(frame, measurer, |shape, _| apply(shape)); + let changed = effects.regions.len(); + effects.committed = edit.commit(frame, limit).committed; + (changed, locked, applicable_count, effects) + } + + pub(in crate::input::state) fn delete( + frame: &mut Frame, + ids: &std::collections::HashSet, + measurer: &TextMeasurer, + limit: usize, + ) -> EditEffects { + let removed: Vec<_> = frame + .shapes + .iter() + .enumerate() + .filter(|(_, shape)| ids.contains(&shape.id) && !shape.locked) + .map(|(index, shape)| (index, shape.clone())) + .collect(); + let mut effects = EditEffects::default(); + for (index, shape) in removed.iter().rev() { + frame.remove_shape_at(*index); + effects + .regions + .push((shape.id, shape.bounding_box_with(measurer), None)); + } + if !removed.is_empty() { + effects.committed = record(frame, limit, vec![UndoAction::Delete { shapes: removed }]); + } + effects + } +} + +impl EditEffects { + fn preview_shape( + &mut self, + frame: &mut Frame, + id: ShapeId, + measurer: &TextMeasurer, + apply: impl FnOnce(&mut Shape) -> bool, + ) { + let Some(drawn) = frame.shape_mut(id) else { + return; + }; + if drawn.locked { + return; + } + let before = drawn.bounding_box_with(measurer); + if apply(&mut drawn.shape) { + drawn.invalidate_bounds(); + self.regions + .push((id, before, drawn.bounding_box_with(measurer))); + } + } +} + +fn record(frame: &mut Frame, limit: usize, mut actions: Vec) -> bool { + let action = match actions.len() { + 0 => return false, + 1 => actions.remove(0), + _ => UndoAction::Compound { actions }, + }; + frame.push_undo_action(action, limit); + true +} + +impl super::base::InputState { + pub(in crate::input::state) fn apply_edit_effects( + &mut self, + measurer: &TextMeasurer, + effects: EditEffects, + ) -> bool { + let changed = effects.committed || !effects.regions.is_empty(); + for (id, before, after) in effects.regions { + self.mark_selection_dirty_region(before); + self.mark_selection_dirty_region(after); + self.invalidate_hit_cache_for_with(measurer, id); + } + if effects.committed { + self.mark_session_dirty(); + } + if changed { + self.needs_redraw = true; + } + changed + } +} + +#[cfg(test)] +mod tests; diff --git a/src/input/state/core/editing/tests.rs b/src/input/state/core/editing/tests.rs new file mode 100644 index 000000000..5a3d1ad38 --- /dev/null +++ b/src/input/state/core/editing/tests.rs @@ -0,0 +1,112 @@ +use super::*; + +fn rectangle(x: i32) -> Shape { + Shape::Rect { + x, + y: 10, + w: 20, + h: 20, + fill: false, + color: crate::draw::WHITE, + thick: 2.0, + } +} + +#[test] +fn rollback_restores_preview_without_touching_history_or_locked_shapes() { + let mut frame = Frame::new(); + let first = frame.add_shape(rectangle(10)); + let locked = frame.add_shape(rectangle(50)); + frame.shape_mut(locked).unwrap().locked = true; + let history = frame.undo_stack_len(); + let measurer = TextMeasurer::default(); + let edit = CanvasEdit::capture(&frame, &[first, locked]); + let effects = edit.preview(&mut frame, &measurer, |shape, _| { + shape.translate(25, 0); + true + }); + assert_eq!(effects.regions.len(), 1); + assert!(!effects.committed); + assert_eq!(frame.shape(first).unwrap().shape, rectangle(35)); + assert_eq!(frame.shape(locked).unwrap().shape, rectangle(50)); + let effects = edit.rollback(&mut frame, &measurer); + assert_eq!(effects.regions.len(), 1); + assert!(!effects.committed); + assert_ne!(effects.regions[0].1, effects.regions[0].2); + assert_eq!(frame.shape(first).unwrap().shape, rectangle(10)); + assert!(frame.shape(locked).unwrap().locked); + assert_eq!(frame.undo_stack_len(), history); +} + +#[test] +fn returning_to_original_geometry_keeps_redo_and_history() { + let mut frame = Frame::new(); + let id = frame.add_shape(rectangle(10)); + let second = frame.add_shape(rectangle(50)); + frame.push_undo_action( + UndoAction::Create { + shapes: vec![(1, frame.shape(second).unwrap().clone())], + }, + 100, + ); + frame.undo_last().unwrap(); + let history = frame.undo_stack_len(); + let measurer = TextMeasurer::default(); + let edit = CanvasEdit::capture(&frame, &[id]); + let _ = edit.preview(&mut frame, &measurer, |shape, _| { + shape.translate(20, 0); + true + }); + let _ = edit.preview(&mut frame, &measurer, |shape, _| { + shape.translate(-20, 0); + true + }); + assert!(!edit.commit(&mut frame, 100).committed); + assert_eq!(frame.undo_stack_len(), history); + assert!(frame.redo_last().is_some()); + assert_eq!(frame.shapes.len(), 2); +} + +#[test] +fn deletion_preserves_locked_shapes_and_restores_original_order_in_one_undo() { + let mut frame = Frame::new(); + let first = frame.add_shape(rectangle(10)); + let locked = frame.add_shape(rectangle(50)); + let last = frame.add_shape(rectangle(90)); + frame.shape_mut(locked).unwrap().locked = true; + let effects = CanvasEdit::delete( + &mut frame, + &[first, locked, last].into_iter().collect(), + &TextMeasurer::default(), + 100, + ); + assert!(effects.committed); + assert_eq!(effects.regions.len(), 2); + assert_eq!( + frame + .shapes + .iter() + .map(|shape| shape.id) + .collect::>(), + vec![locked] + ); + frame.undo_last().unwrap(); + assert_eq!( + frame + .shapes + .iter() + .map(|shape| shape.id) + .collect::>(), + vec![first, locked, last] + ); + assert!(frame.shape(locked).unwrap().locked); + frame.redo_last().unwrap(); + assert_eq!( + frame + .shapes + .iter() + .map(|shape| shape.id) + .collect::>(), + vec![locked] + ); +} diff --git a/src/input/state/core/mod.rs b/src/input/state/core/mod.rs index c681ebaed..2b06b7909 100644 --- a/src/input/state/core/mod.rs +++ b/src/input/state/core/mod.rs @@ -5,6 +5,7 @@ mod captured_image; pub(crate) mod color_picker_popup; mod command_palette; mod dirty; +pub(in crate::input::state) mod editing; mod eyedropper; mod feedback; mod font_cycle; diff --git a/src/input/state/core/properties/apply_selection/helpers.rs b/src/input/state/core/properties/apply_selection/helpers.rs index fcae0bc5a..48a1be4f4 100644 --- a/src/input/state/core/properties/apply_selection/helpers.rs +++ b/src/input/state/core/properties/apply_selection/helpers.rs @@ -58,90 +58,29 @@ impl InputState { pub(in crate::input::state::core) fn apply_selection_change_with( &mut self, measurer: &TextMeasurer, - mut applicable: A, - mut apply: F, + applicable: A, + apply: F, ) -> SelectionApplyResult where A: FnMut(&Shape) -> bool, F: FnMut(&mut Shape) -> bool, { - let ids_len = self.selected_shape_ids().len(); - if ids_len == 0 { - return SelectionApplyResult::default(); - } - - let mut result = SelectionApplyResult::default(); - let mut actions = Vec::new(); - let mut dirty_regions = Vec::new(); - - for idx in 0..ids_len { - let id = self.selected_shape_ids()[idx]; - let frame = self.boards.active_frame_mut(); - let Some(drawn) = frame.shape_mut(id) else { - continue; - }; - if !applicable(&drawn.shape) { - continue; - } - result.applicable += 1; - if drawn.locked { - result.locked += 1; - continue; - } - - let before_bounds = drawn.bounding_box_with(measurer); - let before_snapshot = crate::draw::frame::ShapeSnapshot { - shape: drawn.shape.clone(), - locked: drawn.locked, - }; - - let changed = apply(&mut drawn.shape); - drawn.invalidate_bounds(); - if !changed { - continue; - } - - let after_bounds = drawn.bounding_box_with(measurer); - let after_snapshot = crate::draw::frame::ShapeSnapshot { - shape: drawn.shape.clone(), - locked: drawn.locked, - }; - - actions.push(crate::draw::frame::UndoAction::Modify { - shape_id: drawn.id, - before: before_snapshot, - after: after_snapshot, - }); - dirty_regions.push((drawn.id, before_bounds, after_bounds)); - result.changed += 1; - } - - if actions.is_empty() { - return result; - } - - let undo_action = if actions.len() == 1 { - let Some(action) = actions.pop() else { - return result; - }; - action - } else { - crate::draw::frame::UndoAction::Compound { actions } - }; - - self.boards - .active_frame_mut() - .push_undo_action(undo_action, self.history_limits.undo_stack_limit()); - self.mark_session_dirty(); - - for (shape_id, before, after) in dirty_regions { - self.mark_selection_dirty_region(before); - self.mark_selection_dirty_region(after); - self.invalidate_hit_cache_for_with(measurer, shape_id); + let ids = self.selected_shape_ids().to_vec(); + let (changed, locked, applicable, effects) = + crate::input::state::core::editing::CanvasEdit::apply_selection( + self.boards.active_frame_mut(), + &ids, + measurer, + self.history_limits.undo_stack_limit(), + applicable, + apply, + ); + self.apply_edit_effects(measurer, effects); + SelectionApplyResult { + changed, + locked, + applicable, } - self.needs_redraw = true; - - result } pub(in crate::input::state::core) fn report_selection_apply_result( diff --git a/src/input/state/core/properties/panel_layout/layout.rs b/src/input/state/core/properties/panel_layout/layout.rs index f7204c0dd..5d3af7556 100644 --- a/src/input/state/core/properties/panel_layout/layout.rs +++ b/src/input/state/core/properties/panel_layout/layout.rs @@ -47,6 +47,7 @@ impl InputState { }; let mut max_line_width: f64 = 0.0; + let mut body_ascent: f64 = 0.0; let mut max_label_width: f64 = 0.0; let mut max_value_width: f64 = 0.0; @@ -71,6 +72,7 @@ impl InputState { for line in &panel.lines { let extents = engine.layout(ctx, body_style, line, None).ink_extents(); max_line_width = max_line_width.max(extents.width()); + body_ascent = body_ascent.max(-extents.y_bearing()); } for entry in &panel.entries { let extents = engine @@ -185,7 +187,9 @@ impl InputState { } let title_baseline_y = origin_y + PANEL_PADDING_Y + PANEL_TITLE_FONT; - let info_start_y = title_baseline_y + PANEL_INFO_OFFSET; + // The info offset is a gap below the title box, not a baseline offset. + let divider_y = origin_y + PANEL_PADDING_Y + title_height; + let info_start_y = divider_y + PANEL_INFO_OFFSET + body_ascent; let mut entry_start_y = origin_y + PANEL_PADDING_Y + title_height + info_height; if !panel.entries.is_empty() { entry_start_y += PANEL_SECTION_GAP; @@ -200,6 +204,7 @@ impl InputState { width: panel_width, height: panel_height, title_baseline_y, + divider_y, info_start_y, entry_start_y, entry_row_height: PANEL_ROW_HEIGHT, @@ -252,6 +257,60 @@ mod engine_tests { surface.data().unwrap().to_vec() } + #[test] + fn info_text_clears_divider_and_entries_without_growing_panel() { + let engine = UiTextEngine::default(); + let mut state = crate::input::state::test_support::make_test_input_state(); + let id = state.boards.active_frame_mut().add_shape(Shape::Text { + x: 40, + y: 60, + text: "Shape info".into(), + color: crate::draw::Color::new(1.0, 0.0, 0.0, 1.0), + size: 18.0, + font_descriptor: Default::default(), + background_enabled: false, + wrap_width: None, + }); + state.set_selection(vec![id]); + assert!(state.show_properties_panel_with(&TextMeasurer::default())); + let surface = cairo::ImageSurface::create(cairo::Format::ARgb32, 800, 600).unwrap(); + let ctx = cairo::Context::new(&surface).unwrap(); + state.update_properties_panel_layout(&ctx, 800, 600); + let panel = state.properties_panel().unwrap(); + let layout = state.properties_panel_layout().unwrap(); + assert!(!panel.lines.is_empty()); + assert!(!panel.entries.is_empty()); + let style = UiTextStyle { + family: "Sans", + slant: cairo::FontSlant::Normal, + weight: cairo::FontWeight::Normal, + size: PANEL_BODY_FONT, + }; + for (index, line) in panel.lines.iter().enumerate() { + let ink = engine.layout(&ctx, style, line, None).ink_extents(); + let top = layout.info_start_y + index as f64 * PANEL_LINE_HEIGHT + ink.y_bearing(); + assert!(top > layout.divider_y, "info ink must clear the divider"); + assert!( + top + ink.height() < layout.entry_start_y, + "info ink must clear entries" + ); + } + let expected_height = PANEL_PADDING_Y * 2.0 + + PANEL_TITLE_FONT + + 4.0 + + PANEL_INFO_OFFSET + + PANEL_LINE_HEIGHT * panel.lines.len() as f64 + + PANEL_SECTION_GAP + + PANEL_ROW_HEIGHT * panel.entries.len() as f64; + assert_eq!(layout.height, expected_height.ceil()); + let rows_bottom = + layout.entry_start_y + panel.entries.len() as f64 * layout.entry_row_height; + assert_eq!( + layout.origin_y + layout.height - rows_bottom, + PANEL_PADDING_Y + ); + } + #[test] fn deferred_property_refresh_uses_both_owners_before_layout_and_paint() { let engine = UiTextEngine::default(); diff --git a/src/input/state/core/properties/types.rs b/src/input/state/core/properties/types.rs index 61ce139d5..a64990807 100644 --- a/src/input/state/core/properties/types.rs +++ b/src/input/state/core/properties/types.rs @@ -29,6 +29,7 @@ pub struct PropertiesPanelLayout { pub width: f64, pub height: f64, pub title_baseline_y: f64, + pub divider_y: f64, pub info_start_y: f64, pub entry_start_y: f64, pub entry_row_height: f64, diff --git a/src/input/state/core/selection_actions/delete.rs b/src/input/state/core/selection_actions/delete.rs index ce3a55de6..86217a8e3 100644 --- a/src/input/state/core/selection_actions/delete.rs +++ b/src/input/state/core/selection_actions/delete.rs @@ -1,7 +1,6 @@ use super::super::base::InputState; use crate::draw::ShapeId; use crate::draw::TextMeasurer; -use crate::draw::frame::UndoAction; use std::borrow::Cow; use std::collections::HashSet; @@ -39,46 +38,17 @@ impl InputState { return false; } - let mut removed = Vec::new(); - let mut dirty = Vec::new(); - { - let frame = self.boards.active_frame(); - for (index, shape) in frame.shapes.iter().enumerate() { - if id_set.contains(&shape.id) { - if shape.locked { - continue; - } - dirty.push((shape.id, shape.bounding_box_with(measurer))); - removed.push((index, shape.clone())); - } - } - } - - if removed.is_empty() { + let effects = crate::input::state::core::editing::CanvasEdit::delete( + self.boards.active_frame_mut(), + id_set, + measurer, + self.history_limits.undo_stack_limit(), + ); + if !self.apply_edit_effects(measurer, effects) { return false; } - { - let frame = self.boards.active_frame_mut(); - for (index, _) in removed.iter().rev() { - frame - .remove_shape_at(*index) - .expect("recorded selection index remains valid during reverse deletion"); - } - frame.push_undo_action( - UndoAction::Delete { shapes: removed }, - self.history_limits.undo_stack_limit(), - ); - } - - for (shape_id, bounds) in dirty { - self.mark_selection_dirty_region(bounds); - self.invalidate_hit_cache_for_with(measurer, shape_id); - } - self.clear_selection(); - self.needs_redraw = true; - self.mark_session_dirty(); true } diff --git a/src/input/state/core/selection_actions/resize.rs b/src/input/state/core/selection_actions/resize.rs index 1ce4d5e0a..9953f6cb3 100644 --- a/src/input/state/core/selection_actions/resize.rs +++ b/src/input/state/core/selection_actions/resize.rs @@ -40,23 +40,7 @@ impl InputState { /// Capture snapshots of selected shapes for resize operation. pub(crate) fn capture_resize_selection_snapshots(&self) -> Vec<(ShapeId, ShapeSnapshot)> { - let ids = self.selected_shape_ids(); - let frame = self.boards.active_frame(); - let mut snapshots = Vec::with_capacity(ids.len()); - for id in ids { - if let Some(shape) = frame.shape(*id) - && !shape.locked - { - snapshots.push(( - *id, - ShapeSnapshot { - shape: shape.shape.clone(), - locked: shape.locked, - }, - )); - } - } - snapshots + self.capture_movable_selection_snapshots() } /// Apply resize transformation to all selected shapes. @@ -79,23 +63,16 @@ impl InputState { let (scale_x, scale_y, anchor_x, anchor_y) = Self::compute_scale_factors(handle, original_bounds, dx, dy); - // Collect IDs to invalidate after the loop - let mut ids_to_invalidate = Vec::with_capacity(snapshots.len()); - - { - let frame = self.boards.active_frame_mut(); - for (shape_id, snapshot) in snapshots { - if let Some(drawn) = frame.shape_mut(*shape_id) { - // Apply scaling transformation to the shape - drawn.set_shape(snapshot.shape.scaled(scale_x, scale_y, anchor_x, anchor_y)); - ids_to_invalidate.push(*shape_id); - } - } - } - - for shape_id in ids_to_invalidate { - self.invalidate_hit_cache_for_with(measurer, shape_id); - } + let edit = crate::input::state::core::editing::CanvasEdit::borrow_snapshots(snapshots); + let effects = edit.preview( + self.boards.active_frame_mut(), + measurer, + |shape, snapshot| { + *shape = snapshot.shape.scaled(scale_x, scale_y, anchor_x, anchor_y); + true + }, + ); + self.apply_edit_effects(measurer, effects); self.mark_selection_dirty_region(self.selection_bounds_with(measurer)); } @@ -106,24 +83,8 @@ impl InputState { snapshots: &[(ShapeId, ShapeSnapshot)], ) { let previous_bounds = self.selection_bounds_with(measurer); - let mut ids_to_invalidate = Vec::with_capacity(snapshots.len()); - - { - let frame = self.boards.active_frame_mut(); - for (shape_id, snapshot) in snapshots { - if let Some(drawn) = frame.shape_mut(*shape_id) { - drawn.set_shape(snapshot.shape.clone()); - drawn.locked = snapshot.locked; - ids_to_invalidate.push(*shape_id); - } - } - } - + self.restore_selection_from_snapshots_with(measurer, snapshots.to_vec()); self.mark_selection_dirty_region(previous_bounds); self.mark_selection_dirty_region(self.selection_bounds_with(measurer)); - for shape_id in ids_to_invalidate { - self.invalidate_hit_cache_for_with(measurer, shape_id); - } - self.needs_redraw = true; } } diff --git a/src/input/state/core/selection_actions/translation/mod.rs b/src/input/state/core/selection_actions/translation/mod.rs index 340ac2999..efc652cd7 100644 --- a/src/input/state/core/selection_actions/translation/mod.rs +++ b/src/input/state/core/selection_actions/translation/mod.rs @@ -9,25 +9,11 @@ mod undo; impl InputState { pub(crate) fn capture_movable_selection_snapshots(&self) -> Vec<(ShapeId, ShapeSnapshot)> { - let frame = self.boards.active_frame(); - self.selected_shape_ids() - .iter() - .filter_map(|id| { - frame.shape(*id).and_then(|shape| { - if shape.locked { - None - } else { - Some(( - *id, - ShapeSnapshot { - shape: shape.shape.clone(), - locked: shape.locked, - }, - )) - } - }) - }) - .collect() + crate::input::state::core::editing::CanvasEdit::capture( + self.boards.active_frame(), + self.selected_shape_ids(), + ) + .into_snapshots() } pub(crate) fn apply_translation_to_selection_with( @@ -51,38 +37,17 @@ impl InputState { return false; } - let mut moved_any = false; - for idx in 0..ids_len { - let id = self.selected_shape_ids()[idx]; - let bounds = { - let frame = self.boards.active_frame_mut(); - if let Some(shape) = frame.shape_mut(id) { - if shape.locked { - None - } else { - let before = shape.bounding_box_with(measurer); - shape.shape.translate(dx, dy); - shape.invalidate_bounds(); - let after = shape.bounding_box_with(measurer); - Some((before, after)) - } - } else { - None - } - }; - - if let Some((before_bounds, after_bounds)) = bounds { - self.mark_selection_dirty_region(before_bounds); - self.mark_selection_dirty_region(after_bounds); - self.invalidate_hit_cache_for_with(measurer, id); - moved_any = true; - } - } - - if moved_any { - self.needs_redraw = true; - } - moved_any + let ids = self.selected_shape_ids().to_vec(); + let effects = crate::input::state::core::editing::CanvasEdit::preview_current( + self.boards.active_frame_mut(), + &ids, + measurer, + |shape| { + shape.translate(dx, dy); + true + }, + ); + self.apply_edit_effects(measurer, effects) } pub(crate) fn translate_selection_with_undo_with( @@ -101,7 +66,7 @@ impl InputState { if !self.apply_translation_to_selection_with(measurer, dx, dy) { return false; } - self.push_translation_undo(before); + self.push_translation_undo(measurer, before); true } diff --git a/src/input/state/core/selection_actions/translation/restore.rs b/src/input/state/core/selection_actions/translation/restore.rs index 5e618f0f5..ea94885e5 100644 --- a/src/input/state/core/selection_actions/translation/restore.rs +++ b/src/input/state/core/selection_actions/translation/restore.rs @@ -9,29 +9,8 @@ impl InputState { measurer: &TextMeasurer, snapshots: Vec<(ShapeId, ShapeSnapshot)>, ) { - if snapshots.is_empty() { - return; - } - - for (shape_id, snapshot) in snapshots { - let bounds = { - let frame = self.boards.active_frame_mut(); - if let Some(shape) = frame.shape_mut(shape_id) { - let before = shape.bounding_box_with(measurer); - shape.set_shape(snapshot.shape); - shape.locked = snapshot.locked; - let after = shape.bounding_box_with(measurer); - Some((before, after)) - } else { - None - } - }; - if let Some((before_bounds, after_bounds)) = bounds { - self.mark_selection_dirty_region(before_bounds); - self.mark_selection_dirty_region(after_bounds); - self.invalidate_hit_cache_for_with(measurer, shape_id); - } - } - self.needs_redraw = true; + let effects = crate::input::state::core::editing::CanvasEdit::from_snapshots(snapshots) + .rollback(self.boards.active_frame_mut(), measurer); + self.apply_edit_effects(measurer, effects); } } diff --git a/src/input/state/core/selection_actions/translation/undo.rs b/src/input/state/core/selection_actions/translation/undo.rs index 765f6b7cd..ebce8484d 100644 --- a/src/input/state/core/selection_actions/translation/undo.rs +++ b/src/input/state/core/selection_actions/translation/undo.rs @@ -1,48 +1,18 @@ use crate::draw::ShapeId; -use crate::draw::frame::{ShapeSnapshot, UndoAction}; +use crate::draw::frame::ShapeSnapshot; use crate::input::InputState; +use crate::input::state::core::editing::CanvasEdit; impl InputState { - pub(crate) fn push_translation_undo(&mut self, before: Vec<(ShapeId, ShapeSnapshot)>) -> bool { - if before.is_empty() { - return false; - } - - let mut actions = Vec::new(); - { - let frame = self.boards.active_frame(); - for (shape_id, before_snapshot) in &before { - if let Some(shape) = frame.shape(*shape_id) { - let after_snapshot = ShapeSnapshot { - shape: shape.shape.clone(), - locked: shape.locked, - }; - actions.push(UndoAction::modify_from_snapshots( - *shape_id, - before_snapshot.clone(), - after_snapshot, - )); - } - } - } - - if actions.is_empty() { - return false; - } - - let undo_action = if actions.len() == 1 { - let Some(action) = actions.pop() else { - return false; - }; - action - } else { - UndoAction::Compound { actions } - }; - - self.boards - .active_frame_mut() - .push_undo_action(undo_action, self.history_limits.undo_stack_limit()); - self.mark_session_dirty(); - true + pub(crate) fn push_translation_undo( + &mut self, + measurer: &crate::draw::TextMeasurer, + before: Vec<(ShapeId, ShapeSnapshot)>, + ) -> bool { + let effects = CanvasEdit::from_snapshots(before).commit( + self.boards.active_frame_mut(), + self.history_limits.undo_stack_limit(), + ); + self.apply_edit_effects(measurer, effects) } } diff --git a/src/input/state/mouse/release/mod.rs b/src/input/state/mouse/release/mod.rs index 77de4f5d5..c8dc8bbb6 100644 --- a/src/input/state/mouse/release/mod.rs +++ b/src/input/state/mouse/release/mod.rs @@ -114,7 +114,7 @@ impl InputState { DrawingState::MovingSelection { snapshots, moved, .. } => { - selection::finish_moving_selection(self, snapshots, moved); + selection::finish_moving_selection(self, measurer, snapshots, moved); } DrawingState::Selecting { start_x, diff --git a/src/input/state/mouse/release/selection.rs b/src/input/state/mouse/release/selection.rs index 74fcf902c..07057aa15 100644 --- a/src/input/state/mouse/release/selection.rs +++ b/src/input/state/mouse/release/selection.rs @@ -6,11 +6,12 @@ use super::super::SELECTION_DRAG_THRESHOLD; pub(super) fn finish_moving_selection( state: &mut InputState, + measurer: &crate::draw::TextMeasurer, snapshots: Vec<(ShapeId, ShapeSnapshot)>, moved: bool, ) { if moved { - state.push_translation_undo(snapshots); + state.push_translation_undo(measurer, snapshots); } } @@ -119,33 +120,76 @@ pub(super) fn finish_selection_resize( measurer: &crate::draw::TextMeasurer, snapshots: &[(ShapeId, ShapeSnapshot)], ) { - // Capture after-snapshots and push undo actions - let mut has_changes = false; - let frame = state.boards.active_frame_mut(); - for (shape_id, before_snapshot) in snapshots { - if let Some(shape) = frame.shape(*shape_id) { - let after_snapshot = ShapeSnapshot { - shape: shape.shape.clone(), - locked: shape.locked, - }; - // Check if shape bounds changed (simpler than full PartialEq on Shape) - let before_bounds = before_snapshot.shape.bounding_box_with(measurer); - let after_bounds = after_snapshot.shape.bounding_box_with(measurer); - if before_bounds != after_bounds { - frame.push_undo_action( - UndoAction::modify_from_snapshots( - *shape_id, - before_snapshot.clone(), - after_snapshot, - ), - state.history_limits.undo_stack_limit(), - ); - has_changes = true; - } + let effects = + crate::input::state::core::editing::CanvasEdit::from_snapshots(snapshots.to_vec()).commit( + state.boards.active_frame_mut(), + state.history_limits.undo_stack_limit(), + ); + state.apply_edit_effects(measurer, effects); + state.needs_redraw = true; +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn multi_shape_resize_undo_and_redo_are_one_gesture() { + let mut state = crate::input::state::test_support::TestInputStateBuilder::default().build(); + let measurer = crate::draw::TextMeasurer::default(); + let ids: Vec<_> = [10, 50] + .into_iter() + .map(|x| { + state.boards.active_frame_mut().add_shape(Shape::Rect { + x, + y: 10, + w: 20, + h: 20, + fill: false, + color: crate::draw::WHITE, + thick: 2.0, + }) + }) + .collect(); + state.set_selection(ids.clone()); + let snapshots = state.capture_resize_selection_snapshots(); + let bounds = state.selection_bounds_with(&measurer).unwrap(); + let before: Vec<_> = snapshots + .iter() + .map(|(_, snapshot)| snapshot.shape.clone()) + .collect(); + state.apply_selection_resize_with( + &measurer, + crate::input::state::SelectionHandle::BottomRight, + &bounds, + bounds.width, + bounds.height, + &snapshots, + ); + let after: Vec<_> = ids + .iter() + .map(|id| { + state + .boards + .active_frame() + .shape(*id) + .unwrap() + .shape + .clone() + }) + .collect(); + assert_ne!(before, after); + let history = state.boards.active_frame().undo_stack_len(); + finish_selection_resize(&mut state, &measurer, &snapshots); + let frame = state.boards.active_frame_mut(); + assert_eq!(frame.undo_stack_len(), history + 1); + assert!(frame.undo_last().is_some()); + for (id, before) in ids.iter().zip(before) { + assert_eq!(frame.shape(*id).unwrap().shape, before); + } + assert!(frame.redo_last().is_some()); + for (id, after) in ids.iter().zip(after) { + assert_eq!(frame.shape(*id).unwrap().shape, after); } } - if has_changes { - state.mark_session_dirty(); - } - state.needs_redraw = true; } diff --git a/src/toolbar_gtk/view/top_bar.rs b/src/toolbar_gtk/view/top_bar.rs index 53a68db50..e58e2b0e1 100644 --- a/src/toolbar_gtk/view/top_bar.rs +++ b/src/toolbar_gtk/view/top_bar.rs @@ -17,7 +17,9 @@ mod controls; mod drag; +mod popover_owner; mod popovers; +use popover_owner::{PopoverOwner, PopoverResources}; mod strip; mod style_pill; #[cfg(test)] @@ -137,7 +139,7 @@ type OverflowContentKey = (Tool, Option, bool, bool, bool, bool, bool, boo /// deliberately NOT in this key: each slider emits continuously during a drag, /// so keying the content on its value would rebuild the popover subtree on the /// first backend echo, destroying the live gesture and resetting the scroll -/// position. They ride the persistent `canvas_updaters` instead (like the +/// position. They ride the persistent popover value updaters instead (like the /// strip's thickness/opacity/font-size sliders), which set the value in place /// and are a no-op while the slider is mid-drag. The step *counts* stay in the /// key: they change through discrete −/+ clicks, never a drag, so a rebuild is @@ -378,39 +380,11 @@ pub(in crate::toolbar_gtk) struct TopBar { capture_surface: CaptureSurfaceContent, structure: Option, updaters: Rc>>, - /// Persistent value-updaters for the open Canvas popover's content. The - /// continuously-dragged delay sliders ride these updaters so a drag never - /// triggers a subtree rebuild. Repopulated whenever the popover content is - /// (re)built; run every `apply`. - canvas_updaters: Rc>>, - /// Persistent value-updaters for the open Settings popover. Checkbox - /// values and their next events change in place; only structural menu - /// changes replace the subtree. - settings_updaters: Vec, - shapes_popover: Option, - shapes_capture_surface: Option, - overflow_popover: Option, - overflow_capture_surface: Option, - canvas_popover: Option, - canvas_capture_surface: Option, - session_popover: Option, - session_capture_surface: Option, - settings_popover: Option, - settings_capture_surface: Option, - /// Popover open state as last driven by the snapshot; lets the - /// `closed` handlers distinguish user dismissal from state sync. - shapes_expected_open: Rc>, - overflow_expected_open: Rc>, - canvas_expected_open: Rc>, - session_expected_open: Rc>, - settings_expected_open: Rc>, - /// Discriminants of the currently built popover contents; skips the - /// per-snapshot rebuild that would reset hover and in-flight presses. - shapes_content_key: Cell>, - overflow_content_key: Cell>, - canvas_content_key: RefCell>, - session_content_key: RefCell>, - settings_content_key: RefCell>, + shapes: PopoverOwner, + overflow: PopoverOwner, + canvas: PopoverOwner, + session: PopoverOwner, + settings: PopoverOwner, drag_active: Rc>, drag_blocked: Rc>, move_drag: Option, @@ -490,28 +464,11 @@ impl TopBar { capture_surface, structure: None, updaters: Rc::new(RefCell::new(Vec::new())), - canvas_updaters: Rc::new(RefCell::new(Vec::new())), - settings_updaters: Vec::new(), - shapes_popover: None, - shapes_capture_surface: None, - overflow_popover: None, - overflow_capture_surface: None, - canvas_popover: None, - canvas_capture_surface: None, - session_popover: None, - session_capture_surface: None, - settings_popover: None, - settings_capture_surface: None, - shapes_expected_open: Rc::new(Cell::new(false)), - overflow_expected_open: Rc::new(Cell::new(false)), - canvas_expected_open: Rc::new(Cell::new(false)), - session_expected_open: Rc::new(Cell::new(false)), - settings_expected_open: Rc::new(Cell::new(false)), - shapes_content_key: Cell::new(None), - overflow_content_key: Cell::new(None), - canvas_content_key: RefCell::new(None), - session_content_key: RefCell::new(None), - settings_content_key: RefCell::new(None), + shapes: PopoverOwner::default(), + overflow: PopoverOwner::default(), + canvas: PopoverOwner::default(), + session: PopoverOwner::default(), + settings: PopoverOwner::default(), drag_active: Rc::new(Cell::new(false)), drag_blocked: Rc::new(Cell::new(false)), move_drag: None, @@ -605,11 +562,11 @@ impl TopBar { // a subtree rebuild (set_value is a no-op mid-drag, so an in-flight // gesture survives). Run after `sync_popovers` so a fresh rebuild's // updaters are the ones invoked. - for updater in self.canvas_updaters.borrow().iter() { + for updater in &self.canvas.updaters { updater(snapshot); } if snapshot.settings_popover_open { - for updater in &self.settings_updaters { + for updater in &self.settings.updaters { updater(snapshot); } } @@ -676,46 +633,15 @@ impl TopBar { fn rebuild(&mut self, snapshot: &ToolbarSnapshot, plan: &TopStripPlan) { // Popovers are parented to bar buttons; unparent them before the // buttons go away or GTK complains and leaks the popover widgets. - self.shapes_expected_open.set(false); - self.overflow_expected_open.set(false); - self.canvas_expected_open.set(false); - self.session_expected_open.set(false); - self.settings_expected_open.set(false); - if let Some(popover) = self.shapes_popover.take() { - popover.unparent(); - } - self.shapes_capture_surface = None; - if let Some(popover) = self.overflow_popover.take() { - popover.unparent(); - } - self.overflow_capture_surface = None; - if let Some(popover) = self.canvas_popover.take() { - popover.unparent(); - } - self.canvas_capture_surface = None; - if let Some(popover) = self.session_popover.take() { - popover.unparent(); - } - self.session_capture_surface = None; - if let Some(popover) = self.settings_popover.take() { - popover.unparent(); - } - self.settings_capture_surface = None; - self.shapes_content_key.set(None); - self.overflow_content_key.set(None); - *self.canvas_content_key.borrow_mut() = None; - *self.session_content_key.borrow_mut() = None; - *self.settings_content_key.borrow_mut() = None; + self.shapes.clear(); + self.overflow.clear(); + self.canvas.clear(); + self.session.clear(); + self.settings.clear(); while let Some(child) = self.root.first_child() { self.root.remove(&child); } self.updaters.borrow_mut().clear(); - // The Canvas popover was just unparented above, so its persistent - // value-updaters (which capture those now-dead widgets) must go too; - // a fresh open repopulates them. - self.canvas_updaters.borrow_mut().clear(); - self.settings_updaters.clear(); - if snapshot.top_minimized { self.build_minimized(snapshot, plan); } else if snapshot.top_micro_active() { diff --git a/src/toolbar_gtk/view/top_bar/controls.rs b/src/toolbar_gtk/view/top_bar/controls.rs index 2b0af6a37..5af5c8fe6 100644 --- a/src/toolbar_gtk/view/top_bar/controls.rs +++ b/src/toolbar_gtk/view/top_bar/controls.rs @@ -79,7 +79,7 @@ impl TopBar { let accessible_label = control.accessible_label(snapshot); button.update_property(&[gtk4::accessible::Property::Label(&accessible_label)]); let sender = self.feedback.clone(); - let expected = self.shapes_expected_open.clone(); + let expected = self.shapes.expected_open.clone(); button.connect_clicked(move |_| { send_event(&sender, event_for_toggle_state(control, !expected.get())); }); @@ -102,7 +102,7 @@ impl TopBar { event_for_toggle_state(control, false), ); let sender = self.feedback.clone(); - let expected = self.shapes_expected_open.clone(); + let expected = self.shapes.expected_open.clone(); popover.connect_closed(move |_| { if expected.get() { send_event(&sender, event_for_toggle_state(control, false)); @@ -110,8 +110,7 @@ impl TopBar { }); let capture_surface = CaptureSurfaceContent::empty(); popover.set_child(Some(capture_surface.widget())); - self.shapes_popover = Some(popover); - self.shapes_capture_surface = Some(capture_surface); + self.shapes.install(popover, capture_surface); button } @@ -385,7 +384,7 @@ impl TopBar { ); button.set_child(Some(&icon.area)); let sender = self.feedback.clone(); - let expected = self.overflow_expected_open.clone(); + let expected = self.overflow.expected_open.clone(); button.connect_clicked(move |_| { send_event(&sender, event_for_toggle_state(control, !expected.get())); }); @@ -404,7 +403,7 @@ impl TopBar { event_for_toggle_state(control, false), ); let sender = self.feedback.clone(); - let expected = self.overflow_expected_open.clone(); + let expected = self.overflow.expected_open.clone(); popover.connect_closed(move |_| { if expected.get() { send_event(&sender, event_for_toggle_state(control, false)); @@ -412,33 +411,29 @@ impl TopBar { }); let capture_surface = CaptureSurfaceContent::empty(); popover.set_child(Some(capture_surface.widget())); - self.overflow_popover = Some(popover); - self.overflow_capture_surface = Some(capture_surface); + self.overflow.install(popover, capture_surface); // The Canvas/Session/Settings popovers anchor to the same ⋯ toggle // their menu entries live in; the entries themselves render inside // the overflow popover from the shared spec. let (canvas_popover, canvas_capture) = self.menu_popover( &button, - self.canvas_expected_open.clone(), + self.canvas.expected_open.clone(), ToolbarEvent::ToggleCanvasPopover(false), ); - self.canvas_popover = Some(canvas_popover); - self.canvas_capture_surface = Some(canvas_capture); + self.canvas.install(canvas_popover, canvas_capture); let (session_popover, session_capture) = self.menu_popover( &button, - self.session_expected_open.clone(), + self.session.expected_open.clone(), ToolbarEvent::ToggleSessionPopover(false), ); - self.session_popover = Some(session_popover); - self.session_capture_surface = Some(session_capture); + self.session.install(session_popover, session_capture); let (settings_popover, settings_capture) = self.menu_popover( &button, - self.settings_expected_open.clone(), + self.settings.expected_open.clone(), ToolbarEvent::ToggleSettingsPopover(false), ); - self.settings_popover = Some(settings_popover); - self.settings_capture_surface = Some(settings_capture); + self.settings.install(settings_popover, settings_capture); button } diff --git a/src/toolbar_gtk/view/top_bar/popover_owner.rs b/src/toolbar_gtk/view/top_bar/popover_owner.rs new file mode 100644 index 000000000..59a2bf9ff --- /dev/null +++ b/src/toolbar_gtk/view/top_bar/popover_owner.rs @@ -0,0 +1,75 @@ +//! A popover and its capture resources have one lifecycle. +use super::*; + +#[derive(Clone)] +pub(super) struct PopoverResources { + pub(super) popover: gtk4::Popover, + pub(super) capture_surface: CaptureSurfaceContent, +} + +pub(super) struct PopoverOwner { + pub(super) mounted: Option, + pub(super) expected_open: Rc>, + pub(super) content_key: Option, + pub(super) updaters: Vec, +} + +impl Default for PopoverOwner { + fn default() -> Self { + Self { + mounted: None, + expected_open: Rc::new(Cell::new(false)), + content_key: None, + updaters: Vec::new(), + } + } +} + +impl PopoverOwner { + pub(super) fn install( + &mut self, + popover: gtk4::Popover, + capture_surface: CaptureSurfaceContent, + ) { + self.clear(); + self.mounted = Some(PopoverResources { + popover, + capture_surface, + }); + } + + pub(super) fn clear(&mut self) { + self.expected_open.set(false); + if let Some(resources) = self.mounted.take() { + resources.popover.unparent(); + } + self.content_key = None; + self.updaters.clear(); + } + + pub(super) fn set_open(&self, open: bool) { + self.expected_open.set(open && self.mounted.is_some()); + let Some(resources) = &self.mounted else { + return; + }; + if open && !resources.popover.is_visible() { + resources.popover.popup(); + } else if !open && resources.popover.is_visible() { + resources.popover.popdown(); + } + } +} + +impl PopoverResources { + pub(super) fn set_capture_transparent(&self, transparent: bool) { + if transparent && !self.popover.is_visible() { + return; + } + super::popovers::set_popover_capture_transparent( + &self.popover, + &self.capture_surface, + transparent, + !transparent, + ); + } +} diff --git a/src/toolbar_gtk/view/top_bar/popovers.rs b/src/toolbar_gtk/view/top_bar/popovers.rs index b42a83d9e..7343f640c 100644 --- a/src/toolbar_gtk/view/top_bar/popovers.rs +++ b/src/toolbar_gtk/view/top_bar/popovers.rs @@ -21,19 +21,6 @@ pub(super) fn set_popover_capture_transparent( set_popover_input_enabled(popover, input_enabled); } -fn paired_popover_surface<'a>( - popover: Option<&'a gtk4::Popover>, - surface: Option<&'a CaptureSurfaceContent>, -) -> Option<(&'a gtk4::Popover, &'a CaptureSurfaceContent)> { - if popover.is_none() && surface.is_none() { - return None; - } - Some(( - popover.expect("popover capture surface is paired at construction"), - surface.expect("capture surface is paired with its popover"), - )) -} - fn set_popover_input_enabled(popover: >k4::Popover, enabled: bool) { popover.set_can_target(enabled); if let Some(surface) = popover.surface() { @@ -48,115 +35,52 @@ fn set_popover_input_enabled(popover: >k4::Popover, enabled: bool) { } impl TopBar { - pub(super) fn hide_popovers_for_window_hide(&self) { - self.shapes_expected_open.set(false); - self.overflow_expected_open.set(false); - self.canvas_expected_open.set(false); - self.session_expected_open.set(false); - self.settings_expected_open.set(false); - for popover in [ - self.shapes_popover.as_ref(), - self.overflow_popover.as_ref(), - self.canvas_popover.as_ref(), - self.session_popover.as_ref(), - self.settings_popover.as_ref(), + fn popover_resources(&self) -> impl Iterator { + [ + ("top-shapes-popover", self.shapes.mounted.as_ref()), + ("top-overflow-popover", self.overflow.mounted.as_ref()), + ("top-canvas-popover", self.canvas.mounted.as_ref()), + ("top-session-popover", self.session.mounted.as_ref()), + ("top-settings-popover", self.settings.mounted.as_ref()), ] .into_iter() - .flatten() - { - if popover.is_visible() { - popover.popdown(); - } - } + .filter_map(|(name, resources)| resources.map(|resources| (name, resources))) + } + + pub(super) fn hide_popovers_for_window_hide(&self) { + self.shapes.set_open(false); + self.overflow.set_open(false); + self.canvas.set_open(false); + self.session.set_open(false); + self.settings.set_open(false); } - /// Popovers are independent native Wayland surfaces, so their parent's - /// opacity does not affect them. Keep any open popover mapped and replace - /// its content with the same transparent proof node as the toolbar rather - /// than starting a popup close animation during capture. + /// Each native stays mapped with transparent proof content during capture. pub(super) fn set_popovers_capture_transparent(&self, transparent: bool) { - for (popover, capture_surface) in [ - ( - self.shapes_popover.as_ref(), - self.shapes_capture_surface.as_ref(), - ), - ( - self.overflow_popover.as_ref(), - self.overflow_capture_surface.as_ref(), - ), - ( - self.canvas_popover.as_ref(), - self.canvas_capture_surface.as_ref(), - ), - ( - self.session_popover.as_ref(), - self.session_capture_surface.as_ref(), - ), - ( - self.settings_popover.as_ref(), - self.settings_capture_surface.as_ref(), - ), - ] { - let Some((popover, capture_surface)) = paired_popover_surface(popover, capture_surface) - else { - continue; - }; - if transparent && !popover.is_visible() { - continue; - } - set_popover_capture_transparent(popover, capture_surface, transparent, !transparent); + for (_, resources) in self.popover_resources() { + resources.set_capture_transparent(transparent); } } pub(in crate::toolbar_gtk::view) fn tooltip_roots(&self) -> Vec { - [ - self.shapes_popover.as_ref(), - self.overflow_popover.as_ref(), - self.canvas_popover.as_ref(), - self.session_popover.as_ref(), - self.settings_popover.as_ref(), - ] - .into_iter() - .flatten() - .map(|popover| popover.clone().upcast::()) - .collect() + self.popover_resources() + .map(|(_, resources)| resources.popover.clone().upcast()) + .collect() } pub(in crate::toolbar_gtk::view) fn capture_popover_targets(&self) -> Vec { - [ - ( - "top-shapes-popover", - self.shapes_popover.as_ref(), - self.shapes_capture_surface.as_ref(), - ), - ( - "top-overflow-popover", - self.overflow_popover.as_ref(), - self.overflow_capture_surface.as_ref(), - ), - ( - "top-canvas-popover", - self.canvas_popover.as_ref(), - self.canvas_capture_surface.as_ref(), - ), - ( - "top-session-popover", - self.session_popover.as_ref(), - self.session_capture_surface.as_ref(), - ), - ( - "top-settings-popover", - self.settings_popover.as_ref(), - self.settings_capture_surface.as_ref(), - ), - ] - .into_iter() - .filter_map(|(name, popover, capture_surface)| { - let (popover, capture_surface) = paired_popover_surface(popover, capture_surface)?; - (popover.is_visible() && popover.is_mapped()) - .then(|| CaptureProofTarget::new_withdrawable(name, popover, capture_surface)) - }) - .collect() + self.popover_resources() + .filter(|(_, resources)| { + resources.popover.is_visible() && resources.popover.is_mapped() + }) + .map(|(name, resources)| { + CaptureProofTarget::new_withdrawable( + name, + &resources.popover, + &resources.capture_surface, + ) + }) + .collect() } /// Keep the popovers' contents and open state in line with the snapshot. @@ -175,7 +99,7 @@ impl TopBar { let button_size = (btn_w * scale, btn_h * scale); let icon_size = ICON_SIZE * scale; - if let Some(popover) = self.shapes_popover.clone() { + if let Some(resources) = self.shapes.mounted.clone() { let open = snapshot.shape_picker_open; if open { // Only rebuild the grid when its inputs changed; a rebuild @@ -186,10 +110,9 @@ impl TopBar { snapshot.fill_enabled, snapshot.polygon_sides, ); - if self.shapes_content_key.get() != Some(content_key) { - self.shapes_capture_surface - .as_ref() - .expect("shapes popover capture surface is paired") + if self.shapes.content_key != Some(content_key) { + resources + .capture_surface .set_content(&self.build_shapes_popover_content( snapshot, button_size, @@ -197,18 +120,13 @@ impl TopBar { use_icons, scale, )); - self.shapes_content_key.set(Some(content_key)); + self.shapes.content_key = Some(content_key); } } - self.shapes_expected_open.set(open); - if open && !popover.is_visible() { - popover.popup(); - } else if !open && popover.is_visible() { - popover.popdown(); - } + self.shapes.set_open(open); } - if let Some(popover) = self.overflow_popover.clone() { + if let Some(resources) = self.overflow.mounted.clone() { let open = snapshot.top_overflow_open && model::TopToolbarSpec::overflow_control_count(snapshot, plan) > 0; if open { @@ -222,11 +140,10 @@ impl TopBar { snapshot.session_popover_open, snapshot.settings_popover_open, ); - if self.overflow_content_key.get() != Some(content_key) { + if self.overflow.content_key != Some(content_key) { let spec = model::TopToolbarSpec::build(snapshot, plan); - self.overflow_capture_surface - .as_ref() - .expect("overflow popover capture surface is paired") + resources + .capture_surface .set_content(&self.build_overflow_popover_content( snapshot, &spec, @@ -235,17 +152,12 @@ impl TopBar { use_icons, scale, )); - self.overflow_content_key.set(Some(content_key)); + self.overflow.content_key = Some(content_key); } } - self.overflow_expected_open.set(open); - if open && !popover.is_visible() { - popover.popup(); - } else if !open && popover.is_visible() { - popover.popdown(); - } + self.overflow.set_open(open); } else { - self.overflow_expected_open.set(false); + self.overflow.expected_open.set(false); } self.sync_menu_popovers(snapshot, scale); @@ -255,85 +167,63 @@ impl TopBar { /// line with the snapshot, mirroring the shapes/overflow pattern: content /// rebuilds only when the model's snapshot inputs change. fn sync_menu_popovers(&mut self, snapshot: &ToolbarSnapshot, scale: f64) { - if let Some(popover) = self.canvas_popover.clone() { + if let Some(resources) = self.canvas.mounted.clone() { let open = snapshot.canvas_popover_open; if open { let content_key = CanvasMenuContentKey::of(snapshot); - if self.canvas_content_key.borrow().as_ref() != Some(&content_key) { + if self.canvas.content_key.as_ref() != Some(&content_key) { let (content, updaters) = self.build_canvas_popover_content(snapshot, scale); - self.canvas_capture_surface - .as_ref() - .expect("canvas popover capture surface is paired") - .set_content(&content); + resources.capture_surface.set_content(&content); // Delay-slider values ride these persistent updaters (the // content key omits them), so a delay drag never rebuilds // the subtree. Replace the old set — it captured the now // dead widgets this rebuild just discarded. - *self.canvas_updaters.borrow_mut() = updaters; - *self.canvas_content_key.borrow_mut() = Some(content_key); + self.canvas.updaters = updaters; + self.canvas.content_key = Some(content_key); } } - self.canvas_expected_open.set(open); - if open && !popover.is_visible() { - popover.popup(); - } else if !open && popover.is_visible() { - popover.popdown(); - } + self.canvas.set_open(open); } else { - self.canvas_expected_open.set(false); + self.canvas.expected_open.set(false); } - if let Some(popover) = self.session_popover.clone() { + if let Some(resources) = self.session.mounted.clone() { let open = snapshot.session_popover_open; if open { let content_key = SessionMenuContentKey::of(snapshot); - if self.session_content_key.borrow().as_ref() != Some(&content_key) { - self.session_capture_surface - .as_ref() - .expect("session popover capture surface is paired") + if self.session.content_key.as_ref() != Some(&content_key) { + resources + .capture_surface .set_content(&self.build_session_popover_content(snapshot, scale)); - *self.session_content_key.borrow_mut() = Some(content_key); + self.session.content_key = Some(content_key); } } - self.session_expected_open.set(open); - if open && !popover.is_visible() { - popover.popup(); - } else if !open && popover.is_visible() { - popover.popdown(); - } + self.session.set_open(open); } else { - self.session_expected_open.set(false); + self.session.expected_open.set(false); } - if let Some(popover) = self.settings_popover.clone() { + if let Some(resources) = self.settings.mounted.clone() { let open = snapshot.settings_popover_open; if open { let content_key = SettingsMenuContentKey::of(snapshot); - if self.settings_content_key.borrow().as_ref() != Some(&content_key) { + if self.settings.content_key.as_ref() != Some(&content_key) { let (content, updaters) = self.build_settings_popover_content(snapshot, scale); - self.settings_capture_surface - .as_ref() - .expect("settings popover capture surface is paired") - .set_content(&content); - self.settings_updaters = updaters; - *self.settings_content_key.borrow_mut() = Some(content_key); + resources.capture_surface.set_content(&content); + self.settings.updaters = updaters; + self.settings.content_key = Some(content_key); } } - self.settings_expected_open.set(open); - if open && !popover.is_visible() { - popover.popup(); - } else if !open && popover.is_visible() { - popover.popdown(); - } + self.settings.set_open(open); } else { - self.settings_expected_open.set(false); + self.settings.expected_open.set(false); } } /// Canvas popover content: the Canvas pane's Boards / Pages / Advanced / /// Zoom command sections and the Step Undo/Redo configuration inside a /// scrollable viewport. Returns the built widget together with the - /// section's value-updaters, which the host keeps in `canvas_updaters` and + /// section's value-updaters, which the host keeps in the Canvas popover owner and /// runs every `apply` — the delay sliders read their live value from these /// (the content key omits the delay values) so a delay drag never rebuilds /// the subtree. Most state (sections, step counts, enabled/glyph faces) diff --git a/src/toolbar_gtk/view/top_bar/tests.rs b/src/toolbar_gtk/view/top_bar/tests.rs index 07257d884..535c82a46 100644 --- a/src/toolbar_gtk/view/top_bar/tests.rs +++ b/src/toolbar_gtk/view/top_bar/tests.rs @@ -321,7 +321,7 @@ fn canvas_popover_content_key_rebuilds_on_section_and_value_changes() { /// of the content key, the first backend echo would rebuild the whole popover /// subtree, destroying the live gesture and resetting the scroll. So a /// delay-value change must leave the content key stable — the values ride the -/// persistent `canvas_updaters` instead (set in place, a no-op mid-drag). +/// persistent Canvas popover value updaters instead (set in place, a no-op mid-drag). #[test] fn canvas_popover_content_key_ignores_delay_slider_values() { let state = make_test_input_state(); @@ -1129,26 +1129,11 @@ fn has_capture_phase_click_gesture(widget: >k4::Widget) -> bool { } fn detach_test_popovers(top: &mut TopBar) { - if let Some(popover) = top.shapes_popover.take() { - popover.unparent(); - } - top.shapes_capture_surface = None; - if let Some(popover) = top.overflow_popover.take() { - popover.unparent(); - } - top.overflow_capture_surface = None; - if let Some(popover) = top.canvas_popover.take() { - popover.unparent(); - } - top.canvas_capture_surface = None; - if let Some(popover) = top.session_popover.take() { - popover.unparent(); - } - top.session_capture_surface = None; - if let Some(popover) = top.settings_popover.take() { - popover.unparent(); - } - top.settings_capture_surface = None; + top.shapes.clear(); + top.overflow.clear(); + top.canvas.clear(); + top.session.clear(); + top.settings.clear(); } fn assert_builtin_node( @@ -2256,8 +2241,8 @@ fn assert_gtk_toggle_events(regular: &ToolbarSnapshot, highlighted: &ToolbarSnap active.any_highlight_active = true; active.top_pinned = true; active.top_overflow_open = true; - top.shapes_expected_open.set(true); - top.overflow_expected_open.set(true); + top.shapes.expected_open.set(true); + top.overflow.expected_open.set(true); for updater in top.updaters.borrow().iter() { updater(&active); } @@ -2270,8 +2255,8 @@ fn assert_gtk_toggle_events(regular: &ToolbarSnapshot, highlighted: &ToolbarSnap (&overflow, ToolbarEvent::ToggleTopOverflow(false)), ], ); - top.shapes_expected_open.set(false); - top.overflow_expected_open.set(false); + top.shapes.expected_open.set(false); + top.overflow.expected_open.set(false); detach_test_popovers(&mut top); assert_highlight_ring_event(&mut top, highlighted, &rx); } @@ -2279,12 +2264,28 @@ fn assert_gtk_toggle_events(regular: &ToolbarSnapshot, highlighted: &ToolbarSnap fn assert_test_popover_capture_surfaces(top: &TopBar) { for (popover, capture_surface) in [ ( - top.shapes_popover.as_ref().unwrap(), - top.shapes_capture_surface.as_ref().unwrap(), + top.shapes + .mounted + .as_ref() + .map(|resources| &resources.popover) + .unwrap(), + top.shapes + .mounted + .as_ref() + .map(|resources| &resources.capture_surface) + .unwrap(), ), ( - top.overflow_popover.as_ref().unwrap(), - top.overflow_capture_surface.as_ref().unwrap(), + top.overflow + .mounted + .as_ref() + .map(|resources| &resources.popover) + .unwrap(), + top.overflow + .mounted + .as_ref() + .map(|resources| &resources.capture_surface) + .unwrap(), ), ] { let content = gtk4::Box::new(gtk4::Orientation::Vertical, 0); @@ -2567,9 +2568,9 @@ fn assert_menu_popover_contracts(regular: &ToolbarSnapshot) { &session_snapshot, &plan_top_strip(&crate::ui_text::UiTextEngine::default(), &session_snapshot), ); - assert!(menu_top.session_popover.is_some(), "session popover exists"); + assert!(menu_top.session.mounted.is_some(), "session popover exists"); assert!( - menu_top.settings_popover.is_some(), + menu_top.settings.mounted.is_some(), "settings popover exists" ); @@ -2737,7 +2738,7 @@ fn assert_canvas_popover_contract( regular: &ToolbarSnapshot, rx: &std::sync::mpsc::Receiver, ) { - assert!(top.canvas_popover.is_some(), "canvas popover exists"); + assert!(top.canvas.mounted.is_some(), "canvas popover exists"); assert_canvas_command_sections(top, regular, rx); assert_canvas_delay_updates(top, regular); assert_empty_canvas_popover(top, regular); diff --git a/src/ui.rs b/src/ui.rs index ddc69c266..b02c69e9f 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -37,10 +37,10 @@ pub use color_picker_popup::{color_picker_popup_visual_geometry, render_color_pi pub(crate) use color_picker_popup::{ color_picker_popup_visual_geometry_with_engine, render_color_picker_popup_with_engine, }; -pub use command_palette::{command_palette_visual_geometry, render_command_palette}; pub(crate) use command_palette::{ - command_palette_visual_geometry_with_engine, render_command_palette_with_engine, + CommandPaletteView, command_palette_visual_geometry_with_engine, paint_command_palette, }; +pub use command_palette::{command_palette_visual_geometry, render_command_palette}; pub use context_menu::render_context_menu; pub(crate) use context_menu::render_context_menu_with_engine; pub(crate) use eyedropper_loupe::{compute_eyedropper_loupe_layout, render_eyedropper_loupe}; diff --git a/src/ui/command_palette.rs b/src/ui/command_palette.rs index c8fc5d2bd..1cb826bb2 100644 --- a/src/ui/command_palette.rs +++ b/src/ui/command_palette.rs @@ -66,6 +66,63 @@ const TOOLTIP_BG: Rgba = (0.04, 0.05, 0.07, 0.98); const SCROLL_TRACK: Rgba = (1.0, 1.0, 1.0, 0.1); const SCROLL_THUMB: Rgba = (1.0, 1.0, 1.0, 0.35); +/// Prepared application values consumed by the painter. +pub(crate) enum CommandPaletteView { + Closed, + Capture { + action: crate::config::Action, + bindings: Vec, + }, + List(PaletteListView), +} + +pub(crate) struct PaletteListView { + query: String, + rows: Vec, + geometry: (f64, f64, f64, f64), + scroll: usize, + selected: usize, + bindings: std::collections::HashMap>, + tooltip: Option<(String, i32, i32)>, +} + +impl CommandPaletteView { + pub(crate) fn prepare(state: &InputState, width: u32, height: u32) -> Self { + if !state.command_palette_is_engaged() { + return Self::Closed; + } + if let Some(action) = state.keybinding_capture_action() { + return Self::Capture { + action, + bindings: state.action_binding_labels(action), + }; + } + let rows = state.command_palette_rows(); + let geometry = state.command_palette_geometry_for_rows(width, height, &rows); + let bindings = rows + .iter() + .filter_map(|row| match row { + CommandPaletteListRow::Command { command, .. } => { + Some((command.action, state.action_binding_labels(command.action))) + } + _ => None, + }) + .collect(); + let tooltip = state + .command_palette_action_tooltip_for_layout(&rows, geometry) + .map(|(text, x, y)| (text.to_string(), x, y)); + Self::List(PaletteListView { + query: state.command_palette.query.clone(), + rows, + geometry: (geometry.x, geometry.y, geometry.width, geometry.height), + scroll: state.command_palette.scroll, + selected: state.command_palette.selected, + bindings, + tooltip, + }) + } +} + /// Render the command palette if open. pub fn render_command_palette( ctx: &cairo::Context, @@ -89,30 +146,27 @@ pub(crate) fn render_command_palette_with_engine( screen_width: u32, screen_height: u32, ) { - if !input_state.command_palette_is_engaged() { - return; - } - - if let Some(action) = input_state.keybinding_capture_action() { - render_keybinding_capture( - engine, - ctx, - input_state, - action, - screen_width, - screen_height, - ); - return; - } - - let rows = input_state.command_palette_rows(); - let geometry = - input_state.command_palette_geometry_for_rows(screen_width, screen_height, &rows); - let palette_width = geometry.width; - let height = geometry.height; + let view = CommandPaletteView::prepare(input_state, screen_width, screen_height); + paint_command_palette(engine, ctx, &view, screen_width, screen_height); +} - let x = geometry.x; - let y = geometry.y; +pub(crate) fn paint_command_palette( + engine: &UiTextEngine, + ctx: &cairo::Context, + view: &CommandPaletteView, + screen_width: u32, + screen_height: u32, +) { + let view = match view { + CommandPaletteView::Closed => return, + CommandPaletteView::Capture { action, bindings } => { + render_keybinding_capture(engine, ctx, bindings, *action, screen_width, screen_height); + return; + } + CommandPaletteView::List(view) => view, + }; + let rows = &view.rows; + let (x, y, palette_width, height) = view.geometry; draw_command_palette_frame( ctx, @@ -128,26 +182,11 @@ pub(crate) fn render_command_palette_with_engine( let inner_width = palette_width - COMMAND_PALETTE_PADDING * 2.0; let mut cursor_y = y + COMMAND_PALETTE_PADDING; - cursor_y = draw_command_palette_input( - engine, - ctx, - inner_x, - cursor_y, - inner_width, - &input_state.command_palette.query, - ); + cursor_y = draw_command_palette_input(engine, ctx, inner_x, cursor_y, inner_width, &view.query); - render_command_palette_rows( - engine, - ctx, - input_state, - &rows, - inner_x, - inner_width, - cursor_y, - ); + render_command_palette_rows(engine, ctx, view, rows, inner_x, inner_width, cursor_y); - if rows.is_empty() && !input_state.command_palette.query.is_empty() { + if rows.is_empty() && !view.query.is_empty() { draw_command_palette_empty_state( engine, ctx, @@ -164,18 +203,16 @@ pub(crate) fn render_command_palette_with_engine( palette_width, cursor_y, rows.len(), - input_state.command_palette.scroll, + view.scroll, ); - if let Some((tooltip, pointer_x, pointer_y)) = - input_state.command_palette_action_tooltip_for_layout(&rows, geometry) - { + if let Some((tooltip, pointer_x, pointer_y)) = view.tooltip.as_ref() { draw_command_palette_action_tooltip( engine, ctx, tooltip, - pointer_x as f64, - pointer_y as f64, + *pointer_x as f64, + *pointer_y as f64, screen_width as f64, screen_height as f64, ); @@ -367,7 +404,7 @@ fn draw_command_palette_input( fn render_command_palette_rows( engine: &UiTextEngine, ctx: &cairo::Context, - input_state: &InputState, + view: &PaletteListView, rows: &[CommandPaletteListRow], inner_x: f64, inner_width: f64, @@ -375,7 +412,7 @@ fn render_command_palette_rows( ) { let styles = command_palette_row_styles(); - let scroll = input_state.command_palette.scroll; + let scroll = view.scroll; for (visible_idx, row) in rows .iter() .skip(scroll) @@ -391,11 +428,15 @@ fn render_command_palette_rows( command, command_index, } => { - let is_selected = *command_index == input_state.command_palette.selected; + let is_selected = *command_index == view.selected; render_command_row( engine, ctx, - input_state, + &view.query, + view.bindings + .get(&command.action) + .map(Vec::as_slice) + .unwrap_or_default(), command, &styles, inner_x, diff --git a/src/ui/command_palette/command_palette_row.rs b/src/ui/command_palette/command_palette_row.rs index 07bd6252d..1dcfa5553 100644 --- a/src/ui/command_palette/command_palette_row.rs +++ b/src/ui/command_palette/command_palette_row.rs @@ -1,6 +1,5 @@ use crate::config::KeybindingsConfig; use crate::config::action_meta::ActionMeta; -use crate::input::InputState; use crate::input::state::COMMAND_PALETTE_ITEM_HEIGHT; use crate::input::state::query_tokens; use crate::input::state::{ @@ -49,7 +48,8 @@ pub(super) fn command_palette_row_styles() -> CommandPaletteRowStyle { pub(super) fn render_command_row( engine: &UiTextEngine, ctx: &cairo::Context, - input_state: &InputState, + query: &str, + shortcut_labels: &[String], cmd: &ActionMeta, styles: &CommandPaletteRowStyle, inner_x: f64, @@ -93,7 +93,7 @@ pub(super) fn render_command_row( draw_label_match_highlights( engine, ctx, - &input_state.command_palette.query, + query, cmd.label, label_x, label_y, @@ -124,7 +124,6 @@ pub(super) fn render_command_row( }; let content_right = inner_x + inner_width - 8.0 - actions_width; - let shortcut_labels = input_state.action_binding_labels(cmd.action); let badge_left_edge = render_command_row_shortcut_badge( engine, ctx, @@ -132,7 +131,7 @@ pub(super) fn render_command_row( content_right, desc_x, is_selected, - &shortcut_labels, + shortcut_labels, &styles.shortcut, ); diff --git a/src/ui/command_palette/modals.rs b/src/ui/command_palette/modals.rs index 684648663..deffb31f0 100644 --- a/src/ui/command_palette/modals.rs +++ b/src/ui/command_palette/modals.rs @@ -79,7 +79,7 @@ pub(super) fn keybinding_capture_geometry( pub(super) fn render_keybinding_capture( engine: &UiTextEngine, ctx: &cairo::Context, - input_state: &InputState, + current: &[String], action: crate::config::Action, screen_width: u32, screen_height: u32, @@ -108,7 +108,6 @@ pub(super) fn render_keybinding_capture( y + 38.0, None, ); - let current = input_state.action_binding_labels(action); constants::set_color(ctx, TEXT_DESCRIPTION); engine.draw_baseline( ctx, diff --git a/src/ui/command_palette/tests/engine.rs b/src/ui/command_palette/tests/engine.rs index 646490b79..e0eeac596 100644 --- a/src/ui/command_palette/tests/engine.rs +++ b/src/ui/command_palette/tests/engine.rs @@ -113,3 +113,29 @@ fn retained_palette_tooltip_geometry_contains_pixels_and_unicode_highlight_is_vi assert!(invalid.iter().all(|byte| *byte == 0)); } } + +#[test] +fn prepared_palette_paints_without_application_state() { + let engine = UiTextEngine::default(); + let closed = pixels(1, |ctx| { + paint_command_palette(&engine, ctx, &CommandPaletteView::Closed, 800, 600) + }); + assert!(closed.iter().all(|byte| *byte == 0)); + let view = CommandPaletteView::List(PaletteListView { + query: "No matching action".into(), + rows: Vec::new(), + geometry: (100.0, 100.0, 520.0, 180.0), + scroll: 0, + selected: 0, + bindings: Default::default(), + tooltip: None, + }); + let painted = pixels(1, |ctx| { + paint_command_palette(&engine, ctx, &view, 800, 600) + }); + assert!(painted.iter().any(|byte| *byte != 0)); + // The dimmer and the panel have different opacity, so this checks the + // prepared geometry actually places a panel, beyond merely clearing Cairo. + let alpha = |x: usize, y: usize| painted[(y * 800 + x) * 4 + 3]; + assert!(alpha(200, 150) > alpha(20, 20)); +} diff --git a/src/ui/properties_panel.rs b/src/ui/properties_panel.rs index dbf7b05cd..657d9739e 100644 --- a/src/ui/properties_panel.rs +++ b/src/ui/properties_panel.rs @@ -86,10 +86,10 @@ pub(crate) fn render_properties_panel_with_engine( ); constants::set_color(ctx, DIVIDER); - ctx.move_to(layout.label_x, layout.title_baseline_y + 4.0); + ctx.move_to(layout.label_x, layout.divider_y); ctx.line_to( layout.origin_x + layout.width - layout.padding_x, - layout.title_baseline_y + 4.0, + layout.divider_y, ); let _ = ctx.stroke();