Conversation
WalkthroughA draggable floating control panel overlay was introduced in the desktop app at the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ControlPanel (React)
participant JS Bindings
participant Tauri Backend
participant OverlayState
User->>ControlPanel (React): Drag or interact with overlay
ControlPanel (React)->>JS Bindings: windowSetOverlayBounds(name, bounds)
JS Bindings->>Tauri Backend: plugin:windows|window_set_overlay_bounds
Tauri Backend->>OverlayState: Update overlay bounds for window/name
Note over OverlayState: Overlay bounds updated
User->>ControlPanel (React): Close/Hide overlay
ControlPanel (React)->>JS Bindings: windowRemoveOverlayBounds(name)
JS Bindings->>Tauri Backend: plugin:windows|window_remove_overlay_bounds
Tauri Backend->>OverlayState: Remove overlay bounds for window/name
User->>ControlPanel (React): Request window close
ControlPanel (React)->>JS Bindings: windowClose(window)
JS Bindings->>Tauri Backend: plugin:windows|window_close
Tauri Backend->>HyprWindow: Close window logic executed
Note over plugins/listener/src/fsm.rs: FSM triggers Control window visibility
plugins/listener/src/fsm.rs->>Tauri Backend: windowShow(Control) on running_active
plugins/listener/src/fsm.rs->>Tauri Backend: windowHide(Control) on inactive
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (3)
plugins/windows/js/bindings.gen.ts (1)
71-71: OverlayBound type well-defined.The OverlayBound type appropriately represents rectangular bounds with numeric coordinates and dimensions.
Consider adding JSDoc comments to improve developer experience:
+/** Represents the bounding rectangle for an overlay window */ export type OverlayBound = { x: number; y: number; width: number; height: number }apps/desktop/src/routes/app.control.tsx (1)
76-90: Remove unnecessary setTimeout calls in button handlers.The
setTimeout(updateOverlayBounds, 0)calls appear unnecessary since the overlay bounds shouldn't change when buttons are clicked. These calls were likely added for debugging or as a workaround.const captureScreenshot = () => { console.log("Capture screenshot"); - setTimeout(updateOverlayBounds, 0); }; const toggleRecording = () => { setIsRecording(!isRecording); console.log(isRecording ? "Stop recording" : "Start recording"); - setTimeout(updateOverlayBounds, 0); }; const openSettings = () => { console.log("Open settings"); - setTimeout(updateOverlayBounds, 0); };plugins/windows/src/overlay.rs (1)
24-25: Consider optimizing the polling frequency.The 20 Hz polling rate (50ms intervals) might be resource-intensive for a background task. Consider if this frequency is necessary or if it could be reduced to 10 Hz (100ms) without affecting user experience.
- sleep(Duration::from_millis(1000 / 20)).await; + sleep(Duration::from_millis(1000 / 10)).await; // 10 Hz instead of 20 Hz
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
apps/desktop/src/routeTree.gen.ts(13 hunks)apps/desktop/src/routes/app.control.tsx(1 hunks)plugins/windows/Cargo.toml(1 hunks)plugins/windows/build.rs(1 hunks)plugins/windows/js/bindings.gen.ts(2 hunks)plugins/windows/permissions/autogenerated/commands/window_remove_overlay_bounds.toml(1 hunks)plugins/windows/permissions/autogenerated/commands/window_set_overlay_bounds.toml(1 hunks)plugins/windows/permissions/autogenerated/reference.md(2 hunks)plugins/windows/permissions/schemas/schema.json(2 hunks)plugins/windows/src/commands.rs(1 hunks)plugins/windows/src/ext.rs(7 hunks)plugins/windows/src/lib.rs(3 hunks)plugins/windows/src/overlay.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugins/windows/src/lib.rs (3)
plugins/windows/js/bindings.gen.ts (1)
commands(9-46)plugins/windows/src/commands.rs (2)
window_set_overlay_bounds(114-125)window_remove_overlay_bounds(129-146)plugins/windows/src/overlay.rs (1)
app(22-22)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (29)
plugins/windows/Cargo.toml (1)
31-31: LGTM! Appropriate dependency for async overlay functionality.The addition of
tokiowith thetimefeature is well-suited for the new asynchronous overlay bounds commands and any time-based operations they may require.plugins/windows/build.rs (1)
12-13: LGTM! Well-named commands following established conventions.The new overlay bounds commands follow the existing naming pattern and provide a clear set/remove pair for managing overlay functionality.
plugins/windows/permissions/autogenerated/commands/window_set_overlay_bounds.toml (1)
1-14: LGTM! Standard auto-generated permission configuration.The permission file follows the established pattern for Tauri command permissions with proper allow/deny entries and clear descriptions.
plugins/windows/permissions/autogenerated/commands/window_remove_overlay_bounds.toml (1)
1-14: LGTM! Consistent permission configuration for the remove command.The permission file properly complements the set overlay bounds permissions with the same standard structure and appropriate command references.
plugins/windows/permissions/autogenerated/reference.md (2)
212-233: Documentation follows established patterns.The new overlay bounds permission documentation is consistent with existing entries and clearly describes the allow/deny permissions for
window_remove_overlay_bounds.
290-311: Documentation follows established patterns.The new overlay bounds permission documentation is consistent with existing entries and clearly describes the allow/deny permissions for
window_set_overlay_bounds.plugins/windows/src/lib.rs (3)
5-10: Proper module integration.The overlay module is correctly added and imported following Rust module conventions.
61-69:Details
✅ Verification successful
Clean state management integration.
The OverlayState is properly managed alongside the existing ManagedState. The block structure improves code organization and readability.
Verify that the OverlayState type is properly defined:
🏁 Script executed:
#!/bin/bash # Verify OverlayState implementation ast-grep --pattern 'pub struct OverlayState { $$$ }' cat plugins/windows/src/overlay.rsLength of output: 2690
Clean state management integration verified
- Confirmed
OverlayStateis defined with#[derive(Default)]and contains the expectedbounds: Arc<RwLock<HashMap<String, HashMap<String, OverlayBound>>>>inplugins/windows/src/overlay.rs(lines 14–16).- Integration alongside
ManagedStateinplugins/windows/src/lib.rsis correct and readable.No further changes needed.
47-48:Details
✅ Verification successful
Commands properly registered.
The new overlay bounds commands are correctly added to the specta builder's command collection.
Verify that the command implementations exist and match these registrations:
🏁 Script executed:
#!/bin/bash # Verify the overlay bounds commands are implemented ast-grep --pattern 'pub async fn window_set_overlay_bounds($_) -> $_ { $$$ }' ast-grep --pattern 'pub async fn window_remove_overlay_bounds($_) -> $_ { $$$ }'Length of output: 162
🏁 Script executed:
#!/bin/bash # Search for the overlay bounds command implementations across the repository rg -n 'window_set_overlay_bounds' . rg -n 'window_remove_overlay_bounds' .Length of output: 3972
Overlay bounds commands implemented and registered correctly.
window_set_overlay_bounds(line 114) andwindow_remove_overlay_bounds(line 129) are implemented inplugins/windows/src/commands.rs.- Both commands are registered in the Specta builder at lines 47–48 of
plugins/windows/src/lib.rs.Approving these changes.
plugins/windows/permissions/schemas/schema.json (2)
381-392: Schema entries properly structured.The permission schema entries for
window_remove_overlay_boundsfollow the established pattern with consistent descriptions and proper JSON structure.
417-428: Schema entries properly structured.The permission schema entries for
window_set_overlay_boundsfollow the established pattern with consistent descriptions and proper JSON structure.plugins/windows/js/bindings.gen.ts (1)
67-67: HyprWindow type properly extended.The "control" variant is correctly added to the HyprWindow union type, enabling support for the new control window type.
plugins/windows/src/commands.rs (2)
112-125: LGTM! Well-structured overlay bounds management.The implementation correctly handles the nested HashMap structure and properly acquires write locks. The logic for setting overlay bounds is straightforward and effective.
127-146: Good cleanup logic with proper empty map removal.The implementation correctly handles the case where overlay bounds are removed and cleans up empty maps. The use of
let Some(map) = state.get_mut(window.label()) else { return Ok(()); }pattern is clean and efficient.apps/desktop/src/routes/app.control.tsx (4)
1-10: LGTM! Clean imports and route setup.The route setup and imports are well-organized and follow React Router conventions properly.
12-22: Good state management for draggable control panel.The state initialization logic is sound, with proper calculation of initial positioning to center the control panel horizontally near the bottom of the viewport.
23-33: Clean overlay bounds update logic.The
updateOverlayBoundsfunction correctly retrieves the DOM element's bounding rectangle and calls the appropriate Windows plugin command.
137-152: Well-designed reusable IconButton component.The IconButton component is well-structured with proper TypeScript types, optional props, and consistent styling. The tooltip functionality adds good UX.
plugins/windows/src/overlay.rs (4)
5-11: Well-structured OverlayBound definition.The
OverlayBoundstruct is properly defined with appropriate traits for serialization and type generation.
13-16: Good thread-safe state management design.The
OverlayStatewith nested HashMap and RwLock provides appropriate thread-safe access to overlay bounds per window and overlay name.
45-59: Verify coordinate calculation logic.The coordinate transformation logic looks correct, properly accounting for window position, overlay bounds, and scale factor. The algorithm correctly transforms overlay bounds from local coordinates to screen coordinates for cursor position comparison.
61-71: Good cursor event and focus management.The logic for managing cursor event ignoring and window focus based on overlay bounds is well-implemented. The conditional focus management ensures the window gets focus when cursor is over overlay areas.
plugins/windows/src/ext.rs (6)
28-29: Properly added Control variant to HyprWindow enum.The new Control variant is correctly added with appropriate serde rename attribute, consistent with other enum variants.
43-43: Updated Display implementation correctly.The Display trait implementation is properly updated to handle the Control variant.
149-149: Appropriate title for Control window.The title "Control" is simple and appropriate for this window type.
168-168: Suitable sizing for floating control panel.The 100x100 logical pixel size for both default and minimum size is appropriate for a small floating control panel, ensuring it remains compact and unobtrusive.
Also applies to: 182-182
280-280: Correct URL path mapping.The "/app/control" path correctly maps to the new React route component.
381-392: Consistent window initialization for Control type.The Control window initialization follows the same pattern as other non-main windows: hidden initially, disabled maximize/minimize, proper sizing, and centered positioning. This is appropriate for a floating control panel.
apps/desktop/src/routeTree.gen.ts (1)
20-20: Auto-generated route additions look correct and complete.The new
AppControlRoutehas been properly generated and integrated across all necessary type interfaces and route definitions. The pattern is consistent with existing app subroutes and follows TanStack Router conventions correctly.Also applies to: 65-69, 126-132, 196-196, 209-209, 226-226, 240-240, 256-256, 273-273, 286-286, 300-300, 340-340, 358-361
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/windows/src/ext.rs (1)
168-168: Clarify the sizing strategy for Control windows.The Control window has a default/min size of 100x100, but in the
show()method (lines 366-369), it's actually sized to the full monitor dimensions. This could be confusing for maintainers.Consider either:
- Using more descriptive size values that indicate this is a placeholder
- Adding a comment explaining the sizing strategy
- Using a separate method like
get_overlay_size()for overlay windows- Self::Control => LogicalSize::new(100.0, 100.0), + Self::Control => LogicalSize::new(100.0, 100.0), // Placeholder - actual size set to monitor dimensionsAlso applies to: 182-182
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
plugins/windows/build.rs(2 hunks)plugins/windows/js/bindings.gen.ts(3 hunks)plugins/windows/permissions/autogenerated/commands/window_close.toml(1 hunks)plugins/windows/permissions/autogenerated/reference.md(3 hunks)plugins/windows/permissions/schemas/schema.json(3 hunks)plugins/windows/src/commands.rs(2 hunks)plugins/windows/src/ext.rs(9 hunks)plugins/windows/src/lib.rs(4 hunks)plugins/windows/src/overlay.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- plugins/windows/permissions/autogenerated/commands/window_close.toml
- plugins/windows/permissions/autogenerated/reference.md
🚧 Files skipped from review as they are similar to previous changes (6)
- plugins/windows/build.rs
- plugins/windows/src/commands.rs
- plugins/windows/permissions/schemas/schema.json
- plugins/windows/js/bindings.gen.ts
- plugins/windows/src/lib.rs
- plugins/windows/src/overlay.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugins/windows/src/ext.rs (3)
plugins/windows/src/overlay.rs (2)
app(22-22)spawn_overlay_listener(18-73)plugins/windows/js/bindings.gen.ts (1)
HyprWindow(70-70)plugins/windows/src/commands.rs (2)
window_show(5-11)window_close(15-21)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (7)
plugins/windows/src/ext.rs (7)
28-29: LGTM! Clean addition of Control variant.The new
Controlvariant follows the existing enum pattern with proper serde rename attribute.
43-43: LGTM! Consistent Display implementation.The Display implementation for the Control variant follows the established pattern.
149-149: LGTM! Appropriate title for Control window.The title implementation is consistent with other window types.
294-298: LGTM! Improved window reuse logic.The early return when an existing window is found is a good optimization that avoids unnecessary window creation.
300-406: Excellent refactor of window creation logic.The refactored
show()method is much cleaner with the builder pattern approach. The Control window configuration is comprehensive for overlay functionality:
- Full monitor size with transparency for overlay behavior
- Always on top and visible on all workspaces
- Skip taskbar and accept first mouse
- Proper macOS-specific panel configuration
The integration with
spawn_overlay_listeneris also appropriate.
425-425: Good platform-specific window decoration handling.The conditional decoration settings are appropriate:
- macOS: Decorations enabled with overlay title bar style
- Windows: Decorations disabled (likely for custom window chrome)
This follows platform conventions well.
Also applies to: 432-436
443-443: LGTM! Consistent trait extension.The
window_closemethod addition to the trait and its implementation properly delegates to theclose()method, maintaining consistency with other window operations.Also applies to: 471-473
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/listener/src/fsm.rs (2)
473-479: Consider improving error handling for window operations.The new entry action correctly shows the Control window when entering the running_active state. However, the error handling could be improved.
Consider logging window operation failures instead of silently ignoring them:
#[action] async fn enter_running_active(&mut self) { { use tauri_plugin_windows::{HyprWindow, WindowsPluginExt}; - let _ = self.app.window_show(HyprWindow::Control); + if let Err(e) = self.app.window_show(HyprWindow::Control) { + tracing::warn!("Failed to show Control window: {:?}", e); + } } }
488-491: Maintain consistency with error handling patterns.The window hiding logic is implemented correctly and maintains symmetry with the show operation. For consistency with any changes made to the
enter_running_activefunction, consider applying the same error handling pattern here.If you decide to improve error logging in
enter_running_active, apply the same pattern here:{ use tauri_plugin_windows::{HyprWindow, WindowsPluginExt}; - let _ = self.app.window_hide(HyprWindow::Control); + if let Err(e) = self.app.window_hide(HyprWindow::Control) { + tracing::warn!("Failed to hide Control window: {:?}", e); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/routes/app.control.tsx(1 hunks)plugins/listener/src/fsm.rs(2 hunks)plugins/windows/src/ext.rs(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/routes/app.control.tsx
- plugins/windows/src/ext.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (2)
plugins/listener/src/fsm.rs (2)
419-419: LGTM: Entry action properly integrated with FSM.The addition of the
entry_action = "enter_running_active"attribute correctly integrates the new window control functionality with the state machine's lifecycle.
473-479:Details
✅ Verification successful
Verify that window_show is synchronous or add await if needed.
The
enter_running_activefunction is marked asasyncbut doesn't await thewindow_showcall. Please verify whether this is intentional.
🏁 Script executed:
#!/bin/bash # Description: Check if window_show method is async and requires await # Expected: Find the method signature to determine if it's async ast-grep --pattern $'impl $_ { $$$ fn window_show($$$) -> $_ { $$$ } $$$ }' # Also check for async version ast-grep --pattern $'impl $_ { $$$ async fn window_show($$$) -> $_ { $$$ } $$$ }' # Check the trait definition rg -A 5 "trait.*WindowsPluginExt"Length of output: 843
No changes needed:
window_showis synchronous
Thewindow_showmethod is defined as a regularfnreturningResult<…, …>, not anasync fn, so there is nothing toawait.
|
Related: #653 |
1 participant
Summary by CodeRabbit
/app/controlroute in the desktop app, featuring screenshot, recording, and settings buttons.