feat: improvements and bug fixes - #22
Conversation
… window positioning
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds onboarding and cursor highlight controls, centralizes theme defaults, updates Tauri capabilities and CSP settings, improves Windows window and capture lifecycle handling, serializes frontend persistence, and adds cleanup and validation safeguards. ChangesApplication runtime
Frontend experience
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPanel
participant AppShell
participant OnboardingModal
participant SettingsStore
User->>SettingsPanel: Select reopen onboarding
SettingsPanel->>AppShell: Emit open-onboarding
AppShell->>OnboardingModal: Open modal
OnboardingModal->>SettingsStore: Read shortcut overrides
OnboardingModal-->>AppShell: Emit close
AppShell->>SettingsStore: Persist onboardingCompleted
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/components/overlay/CursorHighlightShell.vue (1)
18-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicate hex-to-RGBA conversion logic.
Both components introduce identical logic to parse a hex color and convert it into an RGBA format. Consolidate this into a shared utility function (e.g., in
src/utils/color.ts) to improve maintainability and keep the code DRY.
src/components/overlay/CursorHighlightShell.vue#L18-L24: Replace this function definition with an import to the shared utility.src/components/app/panels/HomeModes.vue#L85-L89: Replace the inline hex-parsing logic with the shared utility.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/overlay/CursorHighlightShell.vue` around lines 18 - 24, The duplicate hex-to-RGBA conversion should be centralized in a shared color utility. In src/components/overlay/CursorHighlightShell.vue lines 18-24, extract the hexToRgba logic into the shared utility and replace the local function with an import; in src/components/app/panels/HomeModes.vue lines 85-89, replace the inline parsing with the same utility, preserving the existing fallback and alpha behavior.src-tauri/src/lib.rs (1)
25-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood fix: graceful fallback instead of panicking on malformed
zoom.json.Replacing the previous
expect(...)panic with a logged fallback toDEFAULT_ZOOM_BACKENDprevents a corrupt/missing compiled-in config from crashing the whole app at startup.One gap: this parse/fallback logic is pure and trivially testable but has no
#[test]coverage.✅ Suggested test
#[test] fn falls_back_to_default_backend_on_invalid_json() { let backend = serde_json::from_str::<ZoomConfig>("not json") .map(|c| c.backend) .unwrap_or_else(|_| DEFAULT_ZOOM_BACKEND.to_string()); assert_eq!(backend, DEFAULT_ZOOM_BACKEND); }As per coding guidelines, "Write standard Rust unit tests (#[test]) inside modules where logic resides" for
src-tauri/src/**/*.rs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/lib.rs` around lines 25 - 46, Add a standard Rust #[test] near init_zoom_backend_state that parses deliberately invalid JSON using the same serde_json fallback logic and asserts the result equals DEFAULT_ZOOM_BACKEND. Keep the test focused on the malformed-configuration fallback without changing production behavior.Source: Coding guidelines
src-tauri/src/zoom.rs (1)
58-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood defensive clamp; consider extracting the region math for unit testing.
Clamping the computed region to
MAX_ZOOM_REGIONbounds a JS-controllablesize/zoom_levelpair from driving an unboundedVec::with_capacity(region*region*4)allocation later incapture_region— a meaningful hardening of the JS→Rust boundary.The region/half/local_x/local_y math (lines 69-74) is pure and easily testable in isolation but currently has no
#[test]coverage.As per coding guidelines, "Write standard Rust unit tests (#[test]) inside modules where logic resides" for `src-tauri/src/**/*.rs`.✅ Suggested extraction for testability
fn compute_capture_region(size: u32, zoom_level: f32) -> (i32, i32) { let level = zoom_level.max(1.0); let region = ((size as f32) / level) .round() .clamp(1.0, MAX_ZOOM_REGION as f32) as i32; (region, region / 2) } #[test] fn clamps_region_to_max_zoom_region() { let (region, _) = compute_capture_region(u32::MAX, 1.0); assert!(region <= MAX_ZOOM_REGION); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/zoom.rs` around lines 58 - 87, Extract the pure region-size and half-size calculation from capture_zoom_region_raw_sync into a compute_capture_region helper, then use its result for the capture dimensions and local coordinates. Add standard #[test] coverage in the same module verifying that an extreme size at minimum zoom is clamped to MAX_ZOOM_REGION, while preserving the existing runtime behavior.Source: Coding guidelines
src-tauri/src/window.rs (1)
168-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate HWND-extraction boilerplate between
force_window_position_nativeandensure_window_on_monitor_native.Both functions repeat the same
window.window_handle()→RawWindowHandle::Win32extraction pattern (lines 196-203 here, lines 233-240 inensure_window_on_monitor_native). Extracting a sharedfn get_hwnd(window: &tauri::WebviewWindow) -> Option<HWND>helper would reduce duplication and keep future Win32 flag tweaks in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/window.rs` around lines 168 - 217, The HWND extraction logic is duplicated between force_window_position_native and ensure_window_on_monitor_native. Add a shared get_hwnd helper that converts a WebviewWindow handle to Option<HWND>, returning None for unavailable or non-Win32 handles, then update both functions to use it.src-tauri/src/types.rs (1)
46-77: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
with_mode_lockpanics if a futureModevariant is added without updating this list; no tests for the new lock.
mode_locksis pre-populated from a hardcoded variant list innew(), duplicating theModeenum. If a new variant is added later and this list isn't updated,with_mode_lockpanics via.expect(...). Lazily inserting locks on first use removes this footgun entirely. Also, no#[test]was added for this new synchronization behavior.♻️ Suggested lazy-lock refactor
pub struct WindowRegistry { pub mode_windows: RwLock<HashMap<Mode, Vec<String>>>, pub current_snapshot: RwLock<String>, - mode_locks: HashMap<Mode, Mutex<()>>, + mode_locks: Mutex<HashMap<Mode, Arc<Mutex<()>>>>, } impl WindowRegistry { pub fn new() -> Self { - let mut mode_locks = HashMap::new(); - for mode in [Mode::Overlay, Mode::Spotlight, Mode::Highlight, Mode::Zoom] { - mode_locks.insert(mode, Mutex::new(())); - } - Self { mode_windows: RwLock::new(HashMap::new()), current_snapshot: RwLock::new(String::new()), - mode_locks, + mode_locks: Mutex::new(HashMap::new()), } } pub fn with_mode_lock<T>(&self, mode: Mode, f: impl FnOnce() -> T) -> T { - let _guard = self - .mode_locks - .get(&mode) - .expect("mode_locks initialized for every Mode variant") - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let lock = { + let mut locks = self + .mode_locks + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + locks + .entry(mode) + .or_insert_with(|| Arc::new(Mutex::new(()))) + .clone() + }; + let _guard = lock.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); f() } }Add a
#[test]exercising serialization (e.g., two closures on the same mode can't interleave) and poisoned-lock recovery.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/types.rs` around lines 46 - 77, Refactor WindowRegistry::with_mode_lock to lazily create and store a Mutex for the requested Mode instead of relying on the hardcoded initialization list in WindowRegistry::new, while preserving poisoned-lock recovery. Add tests covering serialization for concurrent calls on the same mode and recovery after a poisoned lock.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/Cargo.toml`:
- Line 18: Update the tauri dependency declaration in Cargo.toml to remove the
unconditional devtools feature from the release dependency set. Add a dev-only
feature configuration that enables tauri’s devtools capability only for
local/debug builds, while preserving tray-icon for all builds and the existing
debug-only open_devtools behavior.
In `@src/components/overlay/OverlayShell.vue`:
- Around line 214-216: Update handleCloseOverlay to handle promise rejection
from the set_overlay_visible IPC invoke by adding a catch path that reports the
failure using the component’s existing error-handling or logging mechanism.
Preserve the current close behavior for successful calls, including both Escape
and dock-button triggers.
In `@src/utils/format-accelerator.ts`:
- Around line 20-22: Update formatAccelerator to accept the localized fallback
string as an argument and return it when accelerator is empty, removing the
hardcoded “Sin asignar” text. Update its callers to resolve and pass the
appropriate Vue I18n value.
---
Nitpick comments:
In `@src-tauri/src/lib.rs`:
- Around line 25-46: Add a standard Rust #[test] near init_zoom_backend_state
that parses deliberately invalid JSON using the same serde_json fallback logic
and asserts the result equals DEFAULT_ZOOM_BACKEND. Keep the test focused on the
malformed-configuration fallback without changing production behavior.
In `@src-tauri/src/types.rs`:
- Around line 46-77: Refactor WindowRegistry::with_mode_lock to lazily create
and store a Mutex for the requested Mode instead of relying on the hardcoded
initialization list in WindowRegistry::new, while preserving poisoned-lock
recovery. Add tests covering serialization for concurrent calls on the same mode
and recovery after a poisoned lock.
In `@src-tauri/src/window.rs`:
- Around line 168-217: The HWND extraction logic is duplicated between
force_window_position_native and ensure_window_on_monitor_native. Add a shared
get_hwnd helper that converts a WebviewWindow handle to Option<HWND>, returning
None for unavailable or non-Win32 handles, then update both functions to use it.
In `@src-tauri/src/zoom.rs`:
- Around line 58-87: Extract the pure region-size and half-size calculation from
capture_zoom_region_raw_sync into a compute_capture_region helper, then use its
result for the capture dimensions and local coordinates. Add standard #[test]
coverage in the same module verifying that an extreme size at minimum zoom is
clamped to MAX_ZOOM_REGION, while preserving the existing runtime behavior.
In `@src/components/overlay/CursorHighlightShell.vue`:
- Around line 18-24: The duplicate hex-to-RGBA conversion should be centralized
in a shared color utility. In src/components/overlay/CursorHighlightShell.vue
lines 18-24, extract the hexToRgba logic into the shared utility and replace the
local function with an import; in src/components/app/panels/HomeModes.vue lines
85-89, replace the inline parsing with the same utility, preserving the existing
fallback and alpha behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73db6ca7-5da6-4905-a7b0-ad7a102764d6
📒 Files selected for processing (41)
src-tauri/Cargo.tomlsrc-tauri/capabilities/default.jsonsrc-tauri/capabilities/mode-windows.jsonsrc-tauri/src/commands.rssrc-tauri/src/cursor.rssrc-tauri/src/dxgi_capture.rssrc-tauri/src/lib.rssrc-tauri/src/magnifier.rssrc-tauri/src/types.rssrc-tauri/src/window.rssrc-tauri/src/zoom.rssrc-tauri/tauri.conf.jsonsrc-tauri/tauri.microsoftstore.conf.jsonsrc/App.vuesrc/components/app/AppPanel.vuesrc/components/app/AppShell.vuesrc/components/app/panels/HomeModes.vuesrc/components/app/panels/HomeTools.vuesrc/components/app/panels/HotkeysPanel.vuesrc/components/app/panels/SettingsPanel.vuesrc/components/modals/ConfirmModal.vuesrc/components/modals/OnboardingModal.vuesrc/components/modals/StyleEditorModal.vuesrc/components/overlay/CursorHighlightShell.vuesrc/components/overlay/OverlayShell.vuesrc/components/overlay/SpotlightShell.vuesrc/components/overlay/ZoomShell.vuesrc/components/shared/CanvasStage.vuesrc/components/shared/FloatingDock.vuesrc/components/ui/UiCard.vuesrc/components/whiteboard/WhiteboardShell.vuesrc/composables/useCanvasDrawing.tssrc/composables/useMonitorContext.tssrc/locales/en.jsonsrc/locales/es.jsonsrc/stores/overlay.tssrc/stores/settings.tssrc/stores/tools.tssrc/theme/tokens.tssrc/types/settings.tssrc/utils/format-accelerator.ts
|
|
||
| [dependencies] | ||
| tauri = { version = "2", features = ["tray-icon"] } | ||
| tauri = { version = "2", features = ["tray-icon", "devtools"] } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n## Cargo.toml\n'
sed -n '1,80p' src-tauri/Cargo.toml
printf '\n## lib.rs\n'
sed -n '1,220p' src-tauri/src/lib.rs
printf '\n## Search for devtools usage\n'
rg -n "devtools|open_devtools|Inspect Element|debug_assertions" src-tauri -SRepository: daiv05/vynta
Length of output: 10441
🏁 Script executed:
set -euo pipefail
echo '## Cargo.toml'
sed -n '1,80p' src-tauri/Cargo.toml
echo
echo '## lib.rs'
sed -n '1,220p' src-tauri/src/lib.rs
echo
echo '## Search'
rg -n "devtools|open_devtools|Inspect Element|debug_assertions" src-tauri -SRepository: daiv05/vynta
Length of output: 10421
🌐 Web query:
Tauri 2 devtools feature release mode docs
💡 Result:
In Tauri 2.0, developer tools (DevTools) are enabled by default in development and debug builds, but they are disabled in production (release) builds by default [1][2][3]. To use DevTools in a release build, you must explicitly enable them. To enable DevTools for a release build, add the devtools Cargo feature to your project's src-tauri/Cargo.toml file [1][2]: [dependencies] tauri = { version = "2.0.0", features = ["devtools"] } Important considerations regarding this feature: 1. macOS Warning: The devtools API on macOS relies on private APIs [1]. Enabling this feature on macOS may cause your application to be rejected by the Apple App Store [1]. 2. Security and Best Practices: It is generally recommended not to ship applications with DevTools enabled in production [4]. The canonical approach for debugging production-like behavior without enabling DevTools in the final shipping binary is to run a debug build using the command: tauri build --debug This creates a build in src-tauri/target/debug/bundle that functions similarly to a release build but includes development features like the inspector [1][2]. If you find the --debug flag is not working as expected (e.g., due to specific environment configurations), ensure your package manager command is correctly passing the flag (e.g., npm run tauri build -- --debug) [5]. 3. Alternative Debugging: For production debugging, consider implementing dedicated logging solutions like the tauri-plugin-log, which allows you to capture logs to a file that users can share with you when issues occur [6]. Additionally, you can configure your bundler to generate source maps (e.g., in vite.config.ts) to make production errors easier to trace if you have access to the environment [6]. 4. Programmatic Control: You can also open or close DevTools programmatically within your Rust code using WebviewWindow::open_devtools and WebviewWindow::close_devtools, typically guarded by #[cfg(debug_assertions)] to ensure this code only runs in debug builds [1][2].
Citations:
- 1: https://github.com/tauri-apps/tauri-docs/blob/v2/src/content/docs/develop/Debug/index.mdx
- 2: https://v2.tauri.app/develop/debug/
- 3: https://v2.tauri.app/reference/javascript/api/namespacewebview/
- 4: DevTools in release mode tauri-apps/tauri#3059
- 5: [bug]
tauri buildignores--debugargument tauri-apps/tauri#11768 - 6: https://www.codegiz.com/blog/scukst8z/
Keep devtools out of the release dependency set
tauri’s devtools feature is enabled unconditionally in src-tauri/Cargo.toml:18, so release builds will ship DevTools support too. The #[cfg(debug_assertions)] guard only skips open_devtools(); it doesn’t remove the capability from the binary. Move this behind a dev-only feature and enable it only for local debugging.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/Cargo.toml` at line 18, Update the tauri dependency declaration in
Cargo.toml to remove the unconditional devtools feature from the release
dependency set. Add a dev-only feature configuration that enables tauri’s
devtools capability only for local/debug builds, while preserving tray-icon for
all builds and the existing debug-only open_devtools behavior.
| function handleCloseOverlay() { | ||
| invoke("set_overlay_visible", { visible: false }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add error handling to the overlay-close invoke.
invoke("set_overlay_visible", { visible: false }) has no .catch; if the IPC call fails, the overlay silently fails to close (both from Escape and the dock button now route through this).
🛡️ Proposed fix
function handleCloseOverlay() {
- invoke("set_overlay_visible", { visible: false });
+ invoke("set_overlay_visible", { visible: false }).catch(console.error);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function handleCloseOverlay() { | |
| invoke("set_overlay_visible", { visible: false }); | |
| } | |
| function handleCloseOverlay() { | |
| invoke("set_overlay_visible", { visible: false }).catch(console.error); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/overlay/OverlayShell.vue` around lines 214 - 216, Update
handleCloseOverlay to handle promise rejection from the set_overlay_visible IPC
invoke by adding a catch path that reports the failure using the component’s
existing error-handling or logging mechanism. Preserve the current close
behavior for successful calls, including both Escape and dock-button triggers.
| export function formatAccelerator(accelerator: string): string { | ||
| if (!accelerator) return "Sin asignar"; | ||
| return accelerator |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Internationalize the hardcoded user-facing string.
As per coding guidelines, all user-facing text in .ts files must be internationalized. The hardcoded Spanish string "Sin asignar" violates this rule. Please pass the localized fallback string as an argument so the components can resolve it via Vue I18n.
🌐 Proposed fix
-export function formatAccelerator(accelerator: string): string {
- if (!accelerator) return "Sin asignar";
+export function formatAccelerator(accelerator: string, emptyFallback: string = ""): string {
+ if (!accelerator) return emptyFallback;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function formatAccelerator(accelerator: string): string { | |
| if (!accelerator) return "Sin asignar"; | |
| return accelerator | |
| export function formatAccelerator(accelerator: string, emptyFallback: string = ""): string { | |
| if (!accelerator) return emptyFallback; | |
| return accelerator |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/format-accelerator.ts` around lines 20 - 22, Update
formatAccelerator to accept the localized fallback string as an argument and
return it when accelerator is empty, removing the hardcoded “Sin asignar” text.
Update its callers to resolve and pass the appropriate Vue I18n value.
Source: Coding guidelines
1 participant
Summary by CodeRabbit