feat: consolidate overlay into desktop app and add compact orb demo#450
feat: consolidate overlay into desktop app and add compact orb demo#450senamakel merged 13 commits intotinyhumansai:mainfrom
Conversation
… integration - Added a new OverlayApp component to handle overlay-specific UI and functionality. - Introduced an overlay window configuration in tauri.conf.json, allowing for a transparent, always-on-top overlay. - Implemented parent RPC communication for the overlay to interact with the main application. - Updated main.tsx to conditionally render the OverlayApp based on the current window context. - Enhanced CSS styles to support the overlay's visual requirements. This commit establishes the foundation for overlay functionality, improving user experience with a dedicated interface for specific tasks.
…tions - Deleted the overlay module and its associated files, including process management and configuration settings. - Removed environment variable checks and overlay-related logic from the core process and configuration schema. - Updated documentation to reflect the removal of overlay features, simplifying the codebase and improving maintainability. This commit streamlines the application by eliminating unused overlay components, enhancing overall performance.
- Added a new CSS animation for overlay bubble appearance, improving visual feedback. - Introduced an OverlayBubble interface to manage bubble properties such as tone and text. - Updated OverlayApp component to include a new OverlayBubbleChip for displaying messages with dynamic styling based on tone. - Adjusted overlay window dimensions in tauri.conf.json for a more compact design. This commit improves the user experience by providing visually distinct overlay messages and a refined interface.
…anvas component - Introduced an optional `inverted` prop to the `RotatingTetrahedronCanvas` component, allowing for dynamic color changes based on the prop value. - Updated fill and edge materials to reflect the inverted state, enhancing visual customization. - Refactored the component to improve readability and maintainability by utilizing the new prop in the rendering logic. - Adjusted the effect dependencies to include the `inverted` prop for proper reactivity. This commit enhances the user experience by providing a more flexible and visually appealing tetrahedron display.
…inverted colors - Updated the opacity of the fill material in the RotatingTetrahedronCanvas component to enhance visual clarity when the inverted prop is true. - Reduced the emissive intensity for the inverted state to improve the overall appearance of the tetrahedron. This commit refines the visual representation of the rotating tetrahedron, ensuring better contrast and aesthetics based on user preferences.
…mance - Implemented useRef hooks for fill and edge materials in the RotatingTetrahedronCanvas component to optimize rendering performance. - Updated the useEffect hook to adjust material properties based on the inverted state, improving visual consistency. - Refactored animation speed handling to utilize a reference for smoother updates. - Cleaned up resource management by ensuring materials are disposed of correctly when the component unmounts. This commit enhances the visual fidelity and performance of the rotating tetrahedron, providing a more responsive and visually appealing experience.
- Changed the text alignment of the OverlayBubbleChip component from left to right for improved readability. - Updated the vertical positioning logic in the Tauri overlay to account for a right margin, ensuring consistent placement of the overlay window. These adjustments enhance the visual presentation and positioning of overlay elements, contributing to a better user experience.
- Adjusted the overlay dimensions for a more refined appearance. - Modified the bubble tone classes for improved color consistency and readability. - Enhanced the text size and line height in the OverlayBubbleChip component for better visual clarity. - Updated the orb button size and styling to enhance user interaction. These changes contribute to a more polished and user-friendly overlay experience.
- Introduced a new scenario management system in the OverlayApp component, allowing for dynamic cycling between different overlay states. - Added a new text display feature for scenario two, providing real-time feedback as the text is typed out. - Refactored the bubble rendering logic to accommodate the new scenario structure, improving the overall user interaction experience. These changes enhance the functionality and interactivity of the overlay, making it more engaging for users.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRemoves the backend overlay spawn and config toggle, embeds an overlay Tauri window and capabilities, adds overlay UI and JSON-RPC client, exposes parent RPC URL via a Tauri command, and implements macOS-specific overlay positioning and always-on-top behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Overlay as Overlay Window (Renderer)
participant Tauri as Tauri Command Layer
participant Core as Desktop Core (HTTP JSON-RPC)
User->>Overlay: interact / trigger scenario
Overlay->>Tauri: invoke overlay_parent_rpc_url()
Tauri->>Core: read OPENHUMAN_CORE_RPC_URL (env)
Tauri-->>Overlay: RPC URL or None
alt RPC URL present
Overlay->>Core: HTTP POST JSON-RPC (callParentCoreRpc)
Core-->>Overlay: JSON-RPC result / error
Overlay->>Overlay: parse/unpack result, update UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
app/src-tauri/src/lib.rs (1)
66-85: Overlay positioning logic is correct.The bottom-right pinning calculation correctly accounts for monitor position, monitor size, and window size. Error handling is appropriate with warn-level logs on failure.
Minor naming nit:
right_marginis used for both the right and bottom offsets. Considermarginoredge_marginfor clarity since it applies to both edges.💡 Optional naming improvement
- let right_margin = 20i32; - let x = monitor.position().x + monitor.size().width as i32 - size.width as i32 - right_margin; - let y = monitor.position().y + monitor.size().height as i32 - size.height as i32 - right_margin; + let margin = 20i32; + let x = monitor.position().x + monitor.size().width as i32 - size.width as i32 - margin; + let y = monitor.position().y + monitor.size().height as i32 - size.height as i32 - margin;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/lib.rs` around lines 66 - 85, In pin_overlay_bottom_right change the misleading variable name right_margin to a clearer name (e.g., margin or edge_margin) and update its two usages so the identifier reflects that it applies to both the right and bottom edges; modify the declaration currently "let right_margin = 20i32;" to "let margin = 20i32;" (or edge_margin) and replace references to right_margin in the x and y calculations and any log messages so the function pin_overlay_bottom_right's intent is clearer.app/src-tauri/capabilities/default.json (1)
4-6: Consider updating the description to reflect overlay window.The description still says "Capability for the main window" but now also applies to the overlay window. Consider updating it for accuracy:
- "description": "Capability for the main window (desktop only)", + "description": "Capability for the main and overlay windows (desktop only)",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/capabilities/default.json` around lines 4 - 6, Update the "description" value to accurately reflect that this capability applies to both the main and overlay windows; locate the JSON object containing the "description" key and the "windows" array (which lists "main" and "overlay") and change the text from "Capability for the main window (desktop only)" to wording that mentions both windows (e.g., "Capability for the main and overlay windows (desktop only)").app/src/components/RotatingTetrahedronCanvas.tsx (1)
60-176: Missinginvertedin dependency array may trigger ESLint warning.The setup effect uses
inverted(lines 106-111, 116, 151) but has an empty dependency array. This is intentional to avoid recreating the Three.js scene, but ESLint'sreact-hooks/exhaustive-depsrule may flag it.If the linter complains, suppress it with a comment explaining the pattern:
// eslint-disable-next-line react-hooks/exhaustive-deps -- Scene created once; inverted changes handled by separate effect }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/RotatingTetrahedronCanvas.tsx` around lines 60 - 176, The useEffect in RotatingTetrahedronCanvas references the prop/state variable inverted (used to set fillMaterial, edgeMaterial and speedRef) but has an empty dependency array which will trigger react-hooks/exhaustive-deps; to fix, explicitly suppress the linter for that effect by adding an eslint-disable-next-line react-hooks/exhaustive-deps comment immediately before the closing bracket of the useEffect and include a brief inline justification like "Scene created once; inverted changes handled by separate effect" so reviewers understand the intentional omission.app/src-tauri/tauri.conf.json (1)
24-38: Minor dimension mismatch between config and OverlayApp.The overlay window is configured with
width: 240, height: 220here, butapp/src/overlay/OverlayApp.tsxsetsOVERLAY_WIDTH = 248andOVERLAY_HEIGHT = 228viasetSize(). The React component will resize the window on mount, so this works, but the mismatch could cause a brief visual flash during startup.Consider aligning the initial dimensions to match:
💡 Suggested alignment
{ "label": "overlay", "title": "OpenHuman Overlay", - "width": 240, - "height": 220, - "minWidth": 240, - "minHeight": 220, + "width": 248, + "height": 228, + "minWidth": 248, + "minHeight": 228, "transparent": true,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/tauri.conf.json` around lines 24 - 38, The overlay window dimensions in tauri.conf.json (the "overlay" window entry) don't match the sizes used by the React overlay (OVERLAY_WIDTH and OVERLAY_HEIGHT set via setSize() in OverlayApp.tsx), which can cause a startup flash; update the overlay config's width, height, minWidth and minHeight to match the values used in OverlayApp.tsx (or alternatively change OVERLAY_WIDTH and OVERLAY_HEIGHT in OverlayApp.tsx to match the config) so both the tauri window definition ("label": "overlay") and the OverlayApp.tsx setSize() values are identical.app/src/overlay/parentCoreRpc.ts (1)
8-13: Useconstarrow functions for module functions to match repo standard.These declarations should use arrow-function constants for consistency with the TS/JS rule set.
Suggested refactor
-export function normalizeLegacyMethod(method: string): string { +export const normalizeLegacyMethod = (method: string): string => { if (method.startsWith('openhuman.accessibility_')) { return method.replace('openhuman.accessibility_', 'openhuman.screen_intelligence_'); } return method; -} +}; /** RpcOutcome with non-empty logs serializes as `{ result, logs }` in the core. */ -function unwrapCliCompatibleJson<T>(raw: unknown): T { +const unwrapCliCompatibleJson = <T,>(raw: unknown): T => { if ( raw !== null && typeof raw === 'object' && 'result' in raw && 'logs' in raw && Array.isArray((raw as { logs: unknown }).logs) ) { return (raw as { result: T }).result; } return raw as T; -} +}; -export async function callParentCoreRpc<T>( +export const callParentCoreRpc = async <T>( rpcUrl: string, method: string, params: Record<string, unknown> = {}, timeoutMs: number = DEFAULT_RPC_TIMEOUT_MS -): Promise<T> { +): Promise<T> => { const normalizedMethod = normalizeLegacyMethod(method); const payload = { jsonrpc: '2.0' as const, id: nextJsonRpcId++, method: normalizedMethod, params, }; @@ - return unwrapCliCompatibleJson<T>(json.result); -} + return unwrapCliCompatibleJson<T>(json.result); +};As per coding guidelines, "
**/*.{js,jsx,ts,tsx}: Prefer arrow functions over function declarations".Also applies to: 16-27, 44-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/overlay/parentCoreRpc.ts` around lines 8 - 13, The file uses function declarations instead of the project-standard const arrow-function pattern; replace the exported function declaration normalizeLegacyMethod(method: string) with an exported constant arrow function (e.g., export const normalizeLegacyMethod = (method: string): string => { ... }) and apply the same change to the other module-level functions noted in the review (the ones covering the other ranges), preserving names and signatures, return types, and existing logic so only the declaration style changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/overlay/OverlayApp.tsx`:
- Line 41: The render stores a fresh WebviewWindow each render by calling
getCurrentWindow() into appWindow and then uses it as a dependency, causing the
resize effect in OverlayApp to re-run unnecessarily; move the getCurrentWindow()
call inside the effect (or store it once in a ref) so the effect depends on
stable values only—locate the usage of appWindow and the resize effect in
OverlayApp (the effect that attaches the resize listener and calls
appWindow.on/resize or similar) and replace the render-time getCurrentWindow()
with either a local const inside that effect or a useRef that is initialized
once so the effect no longer re-triggers on every render.
- Around line 58-77: The effect currently calls setTypedText('') synchronously
which violates the react-hooks/set-state-in-effect rule; change the effect to
use a ref for the typing index (e.g., indexRef via useRef) instead of a local
index variable, remove the immediate setTypedText('') calls in the effect body,
and only call setTypedText inside the interval callback; on cleanup (inside the
returned function) clear the intervalId and call setTypedText('') to reset when
leaving the scenario; update references to index -> indexRef.current and keep
SCENARIO_TWO_TEXT, intervalId, scenario, and setTypedText as the points of
modification.
In `@app/src/overlay/parentCoreRpc.ts`:
- Around line 16-27: The current unwrapCliCompatibleJson function can mistakenly
strip fields from legitimate business objects because it treats any object with
both "result" and "logs" properties as the CLI wrapper; tighten the shape check
in unwrapCliCompatibleJson by only unwrapping when the object exactly matches
the CLI wrapper shape (e.g., "result" and "logs" are present and there are no
extra enumerable keys beyond those two, and "logs" is an array), otherwise
return the original object unchanged; update the conditional in
unwrapCliCompatibleJson to verify Object.keys(raw).length === 2 (or an
equivalent exact-key check) along with Array.isArray(logs) before returning (raw
as { result: T }).result.
- Around line 86-93: The code reads the body into json via response.json() and
then directly inspects json.error and json.result; first add a defensive
envelope-shape check that ensures the parsed value is a non-null object (e.g.
typeof json === 'object' && json !== null) and that it conforms to the expected
JsonRpcResponse shape before accessing properties, and if that check fails throw
a deterministic protocol error like "Invalid JSON-RPC envelope" (replace the
current checks to run only after this guard); update the logic around the json
variable/JsonRpcResponse handling so the subsequent json.error and
hasOwnProperty('result') accesses are safe.
- Around line 69-75: The timeout catch block currently only treats the error as
an AbortError when err instanceof DOMException; update the check in the catch
for the RPC helper (the catch handling around normalizedMethod/timeoutMs and
clearTimeout(timer)) to also treat plain Error objects that have name ===
'AbortError' as timeouts (e.g., use a defensive condition like (err instanceof
DOMException && err.name === 'AbortError') || (err && err.name ===
'AbortError')), and apply the identical change to the timeout handling in the
api client (the catch near app/src/services/apiClient.ts) so both places throw
the same timeout Error message when an abort is signaled.
---
Nitpick comments:
In `@app/src-tauri/capabilities/default.json`:
- Around line 4-6: Update the "description" value to accurately reflect that
this capability applies to both the main and overlay windows; locate the JSON
object containing the "description" key and the "windows" array (which lists
"main" and "overlay") and change the text from "Capability for the main window
(desktop only)" to wording that mentions both windows (e.g., "Capability for the
main and overlay windows (desktop only)").
In `@app/src-tauri/src/lib.rs`:
- Around line 66-85: In pin_overlay_bottom_right change the misleading variable
name right_margin to a clearer name (e.g., margin or edge_margin) and update its
two usages so the identifier reflects that it applies to both the right and
bottom edges; modify the declaration currently "let right_margin = 20i32;" to
"let margin = 20i32;" (or edge_margin) and replace references to right_margin in
the x and y calculations and any log messages so the function
pin_overlay_bottom_right's intent is clearer.
In `@app/src-tauri/tauri.conf.json`:
- Around line 24-38: The overlay window dimensions in tauri.conf.json (the
"overlay" window entry) don't match the sizes used by the React overlay
(OVERLAY_WIDTH and OVERLAY_HEIGHT set via setSize() in OverlayApp.tsx), which
can cause a startup flash; update the overlay config's width, height, minWidth
and minHeight to match the values used in OverlayApp.tsx (or alternatively
change OVERLAY_WIDTH and OVERLAY_HEIGHT in OverlayApp.tsx to match the config)
so both the tauri window definition ("label": "overlay") and the OverlayApp.tsx
setSize() values are identical.
In `@app/src/components/RotatingTetrahedronCanvas.tsx`:
- Around line 60-176: The useEffect in RotatingTetrahedronCanvas references the
prop/state variable inverted (used to set fillMaterial, edgeMaterial and
speedRef) but has an empty dependency array which will trigger
react-hooks/exhaustive-deps; to fix, explicitly suppress the linter for that
effect by adding an eslint-disable-next-line react-hooks/exhaustive-deps comment
immediately before the closing bracket of the useEffect and include a brief
inline justification like "Scene created once; inverted changes handled by
separate effect" so reviewers understand the intentional omission.
In `@app/src/overlay/parentCoreRpc.ts`:
- Around line 8-13: The file uses function declarations instead of the
project-standard const arrow-function pattern; replace the exported function
declaration normalizeLegacyMethod(method: string) with an exported constant
arrow function (e.g., export const normalizeLegacyMethod = (method: string):
string => { ... }) and apply the same change to the other module-level functions
noted in the review (the ones covering the other ranges), preserving names and
signatures, return types, and existing logic so only the declaration style
changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d54045b-a76e-4455-ab3f-e6c3257e063d
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
app/src-tauri/capabilities/default.jsonapp/src-tauri/src/lib.rsapp/src-tauri/tauri.conf.jsonapp/src/components/RotatingTetrahedronCanvas.tsxapp/src/index.cssapp/src/main.tsxapp/src/overlay/OverlayApp.tsxapp/src/overlay/parentCoreRpc.tssrc/core/jsonrpc.rssrc/openhuman/config/schema/load.rssrc/openhuman/config/schema/mod.rssrc/openhuman/config/schema/types.rssrc/openhuman/mod.rssrc/openhuman/overlay/mod.rssrc/openhuman/overlay/process.rs
💤 Files with no reviewable changes (6)
- src/openhuman/mod.rs
- src/openhuman/config/schema/mod.rs
- src/openhuman/overlay/mod.rs
- src/openhuman/config/schema/types.rs
- src/openhuman/config/schema/load.rs
- src/openhuman/overlay/process.rs
| } catch (err) { | ||
| clearTimeout(timer); | ||
| if (err instanceof DOMException && err.name === 'AbortError') { | ||
| throw new Error( | ||
| `Core RPC request timed out after ${timeoutMs}ms (method: ${normalizedMethod})` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to verify the code snippet
cat -n app/src/overlay/parentCoreRpc.ts | head -80Repository: tinyhumansai/openhuman
Length of output: 2657
🏁 Script executed:
# Search for AbortError handling patterns in the codebase
rg "AbortError" --type ts --type tsx -B 2 -A 2Repository: tinyhumansai/openhuman
Length of output: 93
🏁 Script executed:
# Check the complete context around the error handling to understand the try-catch structure
sed -n '50,85p' app/src/overlay/parentCoreRpc.tsRepository: tinyhumansai/openhuman
Length of output: 1033
🏁 Script executed:
# Check if this overlay code runs in browser or other contexts
rg "parentCoreRpc" --type ts --type js -lRepository: tinyhumansai/openhuman
Length of output: 85
🏁 Script executed:
# Search for AbortController usage patterns in the codebase
rg "AbortController" --type ts --type js -B 1 -A 3Repository: tinyhumansai/openhuman
Length of output: 3482
🏁 Script executed:
# Check the TypeScript configuration to understand the target
cat tsconfig.json 2>/dev/null || find . -name "tsconfig.json" -type f | head -3Repository: tinyhumansai/openhuman
Length of output: 109
🏁 Script executed:
# Look for any type definitions or error handling for AbortError
rg "AbortError\|DOMException" --type ts --type js --type jsonRepository: tinyhumansai/openhuman
Length of output: 48
🏁 Script executed:
# Check the apiClient.ts to see how it handles AbortError
sed -n '1,100p' app/src/services/apiClient.ts | grep -A 10 -B 5 "AbortError\|abort\|catch"Repository: tinyhumansai/openhuman
Length of output: 706
🏁 Script executed:
# Check if there's a different version in overlay/src
cat app/src/overlay/parentCoreRpc.ts | head -1 && echo "---" && cat overlay/src/parentCoreRpc.ts 2>/dev/null | head -1Repository: tinyhumansai/openhuman
Length of output: 77
🏁 Script executed:
# Look at the app tsconfig to understand the runtime target
cat app/tsconfig.json | grep -A 5 -B 5 "lib\|target\|dom"Repository: tinyhumansai/openhuman
Length of output: 568
🏁 Script executed:
# Search for any error type checks or assertions in the codebase
rg "instanceof Error|instanceof DOMException" --type ts --type jsRepository: tinyhumansai/openhuman
Length of output: 17812
🏁 Script executed:
# Check if there are tests for the parentCoreRpc error handling
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "parentCoreRpc\|callParentCoreRpc" 2>/dev/nullRepository: tinyhumansai/openhuman
Length of output: 50
🏁 Script executed:
# Look at how AbortError is being thrown - check actual fetch behavior and usage patterns
rg "controller.abort\|AbortError" --type ts --type js -B 3 -A 3 | head -100Repository: tinyhumansai/openhuman
Length of output: 48
🏁 Script executed:
# Check if there are any type definitions or comments about AbortError handling
rg "AbortError" --type ts -B 2 -A 2Repository: tinyhumansai/openhuman
Length of output: 1890
🏁 Script executed:
# Verify the actual runtime environment - is this overlay code or server code?
cat app/src/overlay/parentCoreRpc.tsRepository: tinyhumansai/openhuman
Length of output: 2596
Check for generic Error with AbortError name in timeout handling.
Line 71 only catches DOMException with name === 'AbortError'. Some runtimes surface abort signals as generic Error objects with the same name property (e.g., Node.js, custom polyfills), causing timeout errors to leak as uncaught exceptions. Use a more defensive check:
Suggested fix
- if (err instanceof DOMException && err.name === 'AbortError') {
+ if (err instanceof Error && err.name === 'AbortError') {
throw new Error(
`Core RPC request timed out after ${timeoutMs}ms (method: ${normalizedMethod})`
);
}The same issue exists in app/src/services/apiClient.ts (line 71).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/overlay/parentCoreRpc.ts` around lines 69 - 75, The timeout catch
block currently only treats the error as an AbortError when err instanceof
DOMException; update the check in the catch for the RPC helper (the catch
handling around normalizedMethod/timeoutMs and clearTimeout(timer)) to also
treat plain Error objects that have name === 'AbortError' as timeouts (e.g., use
a defensive condition like (err instanceof DOMException && err.name ===
'AbortError') || (err && err.name === 'AbortError')), and apply the identical
change to the timeout handling in the api client (the catch near
app/src/services/apiClient.ts) so both places throw the same timeout Error
message when an abort is signaled.
- Adjusted overlay dimensions for better visual consistency. - Enhanced text display in OverlayBubbleChip to show text progressively based on bubble content. - Refactored the handling of scenario text in OverlayApp to streamline the display logic. These changes contribute to a more polished and engaging user experience in the overlay.
- Removed unnecessary generic type parameter from `unwrapCliCompatibleJson` and `callParentCoreRpc` functions for improved clarity and conciseness. - These changes enhance code readability and maintainability without altering functionality.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src-tauri/src/lib.rs (1)
90-115: Minor redundancy in window configuration—optional cleanup.Lines 92–97 use Tauri's cross-platform APIs (
set_always_on_top,set_visible_on_all_workspaces), while lines 99–109 use native macOS APIs that achieve the same effects (setLevel(NSStatusWindowLevel),CanJoinAllSpaces). Since this function is already macOS-only (#[cfg(target_os = "macos")]), the Tauri calls are technically redundant. Removing them would simplify the code without losing functionality, though keeping both provides a defensive belt-and-suspenders approach if desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/lib.rs` around lines 90 - 115, The configure_overlay_window_macos function contains redundant cross-platform Tauri calls (window.set_always_on_top and window.set_visible_on_all_workspaces) that duplicate behavior already achieved by the native NSWindow configuration; remove those two calls and their associated error branches to simplify the function, leaving the native NSWindow code (window.ns_window() match, collectionBehavior inserts, setCollectionBehavior, setLevel) and its logging intact so macOS-specific behavior is handled solely via the native API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src-tauri/src/lib.rs`:
- Around line 90-115: The configure_overlay_window_macos function contains
redundant cross-platform Tauri calls (window.set_always_on_top and
window.set_visible_on_all_workspaces) that duplicate behavior already achieved
by the native NSWindow configuration; remove those two calls and their
associated error branches to simplify the function, leaving the native NSWindow
code (window.ns_window() match, collectionBehavior inserts,
setCollectionBehavior, setLevel) and its logging intact so macOS-specific
behavior is handled solely via the native API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0d3153e-4f0f-4af7-b273-86569601de74
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
app/src-tauri/Cargo.tomlapp/src-tauri/capabilities/default.jsonapp/src-tauri/src/lib.rsapp/src-tauri/tauri.conf.jsonapp/src/components/RotatingTetrahedronCanvas.tsxapp/src/overlay/OverlayApp.tsxapp/src/overlay/parentCoreRpc.tsapp/src/services/apiClient.ts
✅ Files skipped from review due to trivial changes (1)
- app/src-tauri/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src-tauri/tauri.conf.json
- app/src/overlay/OverlayApp.tsx
- app/src/components/RotatingTetrahedronCanvas.tsx
- app/src/overlay/parentCoreRpc.ts
- app/src-tauri/capabilities/default.json
Summary
overlaywindow instead of a separate overlay processProblem
Solution
Submission Checklist
app/) and/orcargo test(core) for logic you add or changeapp/test/e2e, mock backend,tests/json_rpc_e2e.rsas appropriate)(Any feature related checklist can go in here)
Impact
react-hooks/set-state-in-effectwarnings, including one in the current overlay demo component.Related
Summary by CodeRabbit