Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds native clipboard actions end-to-end: a new ClipboardButtons UI, ControlBar props for onCopy/onPaste, route handlers that send clipboard messages, and server-side InputHandler support for platform-aware copy/paste; also moves Changes
Sequence DiagramsequenceDiagram
participant User
participant ClipboardButtons
participant ControlBar
participant TrackpadRoute
participant WebSocket
participant Remote
User->>ClipboardButtons: pointer down/up on Copy or Paste
ClipboardButtons->>ClipboardButtons: manage firedRef to avoid duplicate trigger
ClipboardButtons->>ControlBar: onCopy() / onPaste()
ControlBar->>TrackpadRoute: handleCopy() / handlePaste()
TrackpadRoute->>WebSocket: send({ type: "clipboard", clipboardAction: "copy"/"paste" })
WebSocket->>Remote: deliver clipboard combo to remote host
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
package.json (2)
42-42: Alpha dependency pinned with caret range.
nitroat^3.0.1-alpha.2is a pre-release version with a caret range, which could resolve to newer alpha/beta versions with breaking changes. Consider pinning it exactly (e.g.,3.0.1-alpha.2without^) to avoid surprises, or leave a comment explaining why the alpha track is needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 42, The package.json currently pins the pre-release dependency "nitro" with a caret range "^3.0.1-alpha.2", which allows npm to install newer pre-release versions; change the version string to an exact pin "3.0.1-alpha.2" (remove the caret) to prevent accidental upgrades to other alpha/beta releases, or if you intentionally track the alpha channel, add a clear inline comment in package.json explaining why the caret-range is required for this project.
42-47: Unrelated dependency changes bundled with a feature PR.Moving nitro and bumping vite are orthogonal to the copy/paste feature. Keeping them in a separate commit or PR makes bisecting and rollback easier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 42 - 47, This PR mixes unrelated dependency bumps with the feature work; revert the dependency changes to keep the feature PR focused: in package.json undo the version changes for "nitro" and "vite" (and any other non-feature-related bumps you unintentionally changed), restore prior versions for those entries, regenerate the lockfile (npm/yarn/pnpm install) so it matches, commit that revert to this branch, and then create a separate branch/PR that contains only the dependency upgrades for "nitro" and "vite" (and related lockfile changes) so they can be reviewed and rolled back independently.src/components/Trackpad/actions/ClipboardButtons.tsx (2)
1-1: UnusedReactimport.With React 19's JSX transform, you don't need to import
Reactunless you use it explicitly (e.g.,React.FC). SinceReact.FCis used on line 8, this is fine — but consider droppingReact.FCin favor of a plain function signature, which is the more modern React/TS idiom and would let you remove the import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Trackpad/actions/ClipboardButtons.tsx` at line 1, Remove the unused React import by converting the component from a React.FC to a plain function component: replace the React.FC signature (used in ClipboardButtons) with a standard function declaration (e.g., function ClipboardButtons(props: ClipboardButtonsProps) { ... }) and drop the top-level import of React from "react"; keep importing only hooks/types you use (like useRef) to avoid unused imports.
12-28: SharedfiredRefbetween Copy and Paste could cause subtle cross-button interference.A single ref guards both buttons. If a pointer interaction starts on one button but the
pointerUpsomehow dispatches against the other (e.g., drag across), the guard state leaks between them. Consider using separate refs or keying the ref by action identity.Separate refs approach
- const firedRef = useRef(false); + const copyFiredRef = useRef(false); + const pasteFiredRef = useRef(false);Then pass the corresponding ref to each handler, or create a small helper that closes over the correct ref.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Trackpad/actions/ClipboardButtons.tsx` around lines 12 - 28, The shared firedRef used by handlePointerDown and handlePointerUp causes cross-button interference between the Copy and Paste buttons; change to use a separate ref per button (e.g., copyFiredRef and pasteFiredRef) or a ref keyed by action identity, then wire the correct ref into the handlers (update calls to handlePointerDown and handlePointerUp so each button passes its own ref or use a small helper factory that closes over the proper ref) so fired state cannot leak between buttons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Trackpad/actions/ClipboardButtons.tsx`:
- Around line 32-45: In the ClipboardButtons component, the two buttons (the
Copy and Paste buttons that call handlePointerDown/handlePointerUp with onCopy
and onPaste) need an explicit type attribute to avoid defaulting to
type="submit"; update both button elements to include type="button" so they
won’t trigger form submission if placed inside a form.
In `@src/routes/trackpad.tsx`:
- Around line 49-50: The handleCopy and handlePaste handlers currently call
sendCombo(['control', 'c']) and sendCombo(['control', 'v']) which always map to
Ctrl; update them to use a platform-aware modifier: determine whether to use
'meta' (Cmd on macOS) or 'control' (Ctrl on others) and call sendCombo with that
modifier, or alternatively always send 'meta' on macOS by checking
navigator.platform or a shared platform-detection helper used elsewhere; adjust
handleCopy and handlePaste to compute const modifier = isMac ? 'meta' :
'control' and call sendCombo([modifier, 'c']) / sendCombo([modifier, 'v']),
ensuring InputHandler.ts/KeyMap.ts mapping picks up the correct Key.
---
Nitpick comments:
In `@package.json`:
- Line 42: The package.json currently pins the pre-release dependency "nitro"
with a caret range "^3.0.1-alpha.2", which allows npm to install newer
pre-release versions; change the version string to an exact pin "3.0.1-alpha.2"
(remove the caret) to prevent accidental upgrades to other alpha/beta releases,
or if you intentionally track the alpha channel, add a clear inline comment in
package.json explaining why the caret-range is required for this project.
- Around line 42-47: This PR mixes unrelated dependency bumps with the feature
work; revert the dependency changes to keep the feature PR focused: in
package.json undo the version changes for "nitro" and "vite" (and any other
non-feature-related bumps you unintentionally changed), restore prior versions
for those entries, regenerate the lockfile (npm/yarn/pnpm install) so it
matches, commit that revert to this branch, and then create a separate branch/PR
that contains only the dependency upgrades for "nitro" and "vite" (and related
lockfile changes) so they can be reviewed and rolled back independently.
In `@src/components/Trackpad/actions/ClipboardButtons.tsx`:
- Line 1: Remove the unused React import by converting the component from a
React.FC to a plain function component: replace the React.FC signature (used in
ClipboardButtons) with a standard function declaration (e.g., function
ClipboardButtons(props: ClipboardButtonsProps) { ... }) and drop the top-level
import of React from "react"; keep importing only hooks/types you use (like
useRef) to avoid unused imports.
- Around line 12-28: The shared firedRef used by handlePointerDown and
handlePointerUp causes cross-button interference between the Copy and Paste
buttons; change to use a separate ref per button (e.g., copyFiredRef and
pasteFiredRef) or a ref keyed by action identity, then wire the correct ref into
the handlers (update calls to handlePointerDown and handlePointerUp so each
button passes its own ref or use a small helper factory that closes over the
proper ref) so fired state cannot leak between buttons.
Comment on lines
32
to
45
| <button | ||
| className="btn btn-sm btn-outline" | ||
| onPointerDown={(e) => handlePointerDown(e, onCopy)} | ||
| onPointerUp={(e) => handlePointerUp(e, onCopy)} | ||
| > | ||
| Copy | ||
| </button> | ||
| <button | ||
| className="btn btn-sm btn-outline" | ||
| onPointerDown={(e) => handlePointerDown(e, onPaste)} | ||
| onPointerUp={(e) => handlePointerUp(e, onPaste)} | ||
| > | ||
| Paste | ||
| </button> |
Contributor
There was a problem hiding this comment.
Add explicit type="button" to both buttons.
As flagged by Biome, buttons default to type="submit", which can cause unintended form submission if these buttons ever end up inside a <form>. Add type="button" to both.
Proposed fix
<button
className="btn btn-sm btn-outline"
+ type="button"
onPointerDown={(e) => handlePointerDown(e, onCopy)}
onPointerUp={(e) => handlePointerUp(e, onCopy)}
>
Copy
</button>
<button
className="btn btn-sm btn-outline"
+ type="button"
onPointerDown={(e) => handlePointerDown(e, onPaste)}
onPointerUp={(e) => handlePointerUp(e, onPaste)}
>
Paste
</button>📝 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.
Suggested change
| <button | |
| className="btn btn-sm btn-outline" | |
| onPointerDown={(e) => handlePointerDown(e, onCopy)} | |
| onPointerUp={(e) => handlePointerUp(e, onCopy)} | |
| > | |
| Copy | |
| </button> | |
| <button | |
| className="btn btn-sm btn-outline" | |
| onPointerDown={(e) => handlePointerDown(e, onPaste)} | |
| onPointerUp={(e) => handlePointerUp(e, onPaste)} | |
| > | |
| Paste | |
| </button> | |
| <button | |
| className="btn btn-sm btn-outline" | |
| type="button" | |
| onPointerDown={(e) => handlePointerDown(e, onCopy)} | |
| onPointerUp={(e) => handlePointerUp(e, onCopy)} | |
| > | |
| Copy | |
| </button> | |
| <button | |
| className="btn btn-sm btn-outline" | |
| type="button" | |
| onPointerDown={(e) => handlePointerDown(e, onPaste)} | |
| onPointerUp={(e) => handlePointerUp(e, onPaste)} | |
| > | |
| Paste | |
| </button> |
🧰 Tools
🪛 Biome (2.3.14)
[error] 32-36: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 39-43: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Trackpad/actions/ClipboardButtons.tsx` around lines 32 - 45,
In the ClipboardButtons component, the two buttons (the Copy and Paste buttons
that call handlePointerDown/handlePointerUp with onCopy and onPaste) need an
explicit type attribute to avoid defaulting to type="submit"; update both button
elements to include type="button" so they won’t trigger form submission if
placed inside a form.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/Trackpad/actions/ClipboardButtons.tsx (1)
12-12: SharedfiredRefbetween Copy and Paste could cause subtle edge-case bugs.A single ref guards both buttons. If
onPointerDownfires on Copy (settingfiredRef = true) and the user somehow triggersonPointerUpon the Paste button instead, the guard will incorrectly suppress the Paste fallback and then reset the ref. While unlikely with normal touch interactions, using separate refs (or keying the ref by action label) would be more robust.Proposed fix — separate refs or key-based guard
- const firedRef = useRef(false); + const firedRef = useRef<string | null>(null); - const handlePointerDown = (e: React.PointerEvent, action: () => void, label: string) => { + const handlePointerDown = (e: React.PointerEvent, action: () => void, label: string) => { e.preventDefault(); e.stopPropagation(); console.log(`[UI] ${label} Button - PointerDown triggered`); - firedRef.current = true; + firedRef.current = label; action(); }; - const handlePointerUp = (e: React.PointerEvent, action: () => void, label: string) => { + const handlePointerUp = (e: React.PointerEvent, action: () => void, label: string) => { e.preventDefault(); e.stopPropagation(); - if (!firedRef.current) { + if (firedRef.current !== label) { console.log(`[UI] ${label} Button - PointerUp fallback triggered`); - action(); + if (firedRef.current === null) action(); } - firedRef.current = false; + firedRef.current = null; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Trackpad/actions/ClipboardButtons.tsx` at line 12, The shared firedRef used by the Copy and Paste button handlers (in ClipboardButtons.tsx) can cause cross-action suppression when onPointerDown for one button and onPointerUp for the other occurs; replace the single firedRef with two distinct refs (e.g., firedRefCopy and firedRefPaste) or a keyed ref map and use the matching ref in each button's onPointerDown/onPointerUp/fallback logic so each action guards and resets only its own ref (update the Copy and Paste handler functions to reference their respective ref names).src/server/InputHandler.ts (1)
76-78: Wrapconstdeclaration in a block to avoid switch-clause scope leakage.Biome correctly flags that
const promisesdeclared in acasewithout braces is accessible from other cases.Proposed fix
- case 'scroll': - // Fixed type from Promise<void>[] to Promise<any>[] to handle nut-js return types - const promises: Promise<any>[] = []; + case 'scroll': { + // Fixed type from Promise<void>[] to Promise<any>[] to handle nut-js return types + const promises: Promise<any>[] = []; // ... existing scroll logic ... - break; + break; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/InputHandler.ts` around lines 76 - 78, The const declaration "promises" inside the switch case 'scroll' can leak into other cases; wrap the entire case 'scroll' body in a block (add { ... } around the case's statements) so Promise<any>[] promises is scoped to that block. Locate the switch handling in InputHandler (case 'scroll') and enclose the case's code—including the const promises and any related logic—within braces to prevent scope leakage.src/components/Trackpad/ControlBar.tsx (1)
63-89: Addtype="button"to all buttons in ControlBar.Biome flags all four buttons as missing an explicit
typeprop. Same issue as inClipboardButtons.Proposed fix
<button className={`btn btn-sm ${scrollMode ? "btn-primary" : "btn-outline"}`} + type="button" onPointerDown={(e) => handleInteraction(e, onToggleScroll, "ToggleScroll")} > ... <button className="btn btn-sm btn-outline" + type="button" onPointerDown={(e) => handleInteraction(e, onRightClick, "RightClick")} > ... <button className={`btn btn-sm ${getModifierButtonClass()}`} + type="button" onPointerDown={(e) => handleInteraction(e, onModifierToggle, "ModifierToggle")} > ... <button className="btn btn-sm btn-secondary" + type="button" onPointerDown={(e) => handleInteraction(e, onKeyboardToggle, "KeyboardToggle")} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Trackpad/ControlBar.tsx` around lines 63 - 89, Add explicit type="button" to every HTML button in the ControlBar UI to prevent them from defaulting to "submit": update the button elements in the ControlBar component (the buttons that call handleInteraction with onToggleScroll/"ToggleScroll", onRightClick/"RightClick", onModifierToggle/"ModifierToggle", and onKeyboardToggle/"KeyboardToggle") to include type="button", and also ensure the ClipboardButtons component implementation uses type="button" for its internal buttons; locate these via the ControlBar component, ClipboardButtons usage, and helper functions getModifierButtonClass/getModifierLabel to verify all buttons are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Trackpad/ControlBar.tsx`:
- Around line 62-70: The ControlBar grid currently uses "grid-cols-5" but
renders six interactive elements because ClipboardButtons returns two <button>s;
update the layout so items don't wrap by either changing the container class
from "grid-cols-5" to "grid-cols-6" in ControlBar.tsx, or wrap the
ClipboardButtons usage in a single container element (e.g., a div) so it
occupies one grid cell; locate the ControlBar component and the ClipboardButtons
invocation and apply one of these two fixes to ensure the "Keyboard" button
stays on the same row.
In `@src/server/InputHandler.ts`:
- Around line 40-42: The debug call to clipboard.getContent() in InputHandler
may throw; wrap that call in its own try/catch (inside the same block where
clipboard.getContent() is used) and only log when it succeeds, e.g., call
clipboard.getContent() inside try, check the returned string length before doing
content.substring(0,30) (or use content.slice(0,30)) to avoid another error, and
on catch either skip logging or log a safe fallback message; update references
around the existing console.log(`[Clipboard] Grabbed from screen: ...`) to use
the value returned by the try block.
- Around line 32-53: The clipboard case in InputHandler.ts hardcodes
Key.LeftControl for copy/paste which breaks on macOS; update the clipboard
branch handling msg.clipboardAction to use the same modifier selection as the
combo logic by calling translateKey('control') or checking isMac to pick
Key.LeftMeta on macOS (instead of Key.LeftControl) before calling
keyboard.pressKey / keyboard.releaseKey; reference the clipboard case handling
of msg.clipboardAction, the translateKey helper and the isMac flag, and replace
usages of Key.LeftControl in that block with the platform-aware modifier.
---
Duplicate comments:
In `@src/components/Trackpad/actions/ClipboardButtons.tsx`:
- Around line 34-47: The two buttons in ClipboardButtons.tsx (the Copy and Paste
elements that call handlePointerDown/handlePointerUp with onCopy and onPaste)
are missing an explicit type and default to "submit"; update both button
elements to include type="button" so they do not submit enclosing forms when
clicked while preserving the existing onPointerDown/onPointerUp handlers.
---
Nitpick comments:
In `@src/components/Trackpad/actions/ClipboardButtons.tsx`:
- Line 12: The shared firedRef used by the Copy and Paste button handlers (in
ClipboardButtons.tsx) can cause cross-action suppression when onPointerDown for
one button and onPointerUp for the other occurs; replace the single firedRef
with two distinct refs (e.g., firedRefCopy and firedRefPaste) or a keyed ref map
and use the matching ref in each button's onPointerDown/onPointerUp/fallback
logic so each action guards and resets only its own ref (update the Copy and
Paste handler functions to reference their respective ref names).
In `@src/components/Trackpad/ControlBar.tsx`:
- Around line 63-89: Add explicit type="button" to every HTML button in the
ControlBar UI to prevent them from defaulting to "submit": update the button
elements in the ControlBar component (the buttons that call handleInteraction
with onToggleScroll/"ToggleScroll", onRightClick/"RightClick",
onModifierToggle/"ModifierToggle", and onKeyboardToggle/"KeyboardToggle") to
include type="button", and also ensure the ClipboardButtons component
implementation uses type="button" for its internal buttons; locate these via the
ControlBar component, ClipboardButtons usage, and helper functions
getModifierButtonClass/getModifierLabel to verify all buttons are covered.
In `@src/server/InputHandler.ts`:
- Around line 76-78: The const declaration "promises" inside the switch case
'scroll' can leak into other cases; wrap the entire case 'scroll' body in a
block (add { ... } around the case's statements) so Promise<any>[] promises is
scoped to that block. Locate the switch handling in InputHandler (case 'scroll')
and enclose the case's code—including the const promises and any related
logic—within braces to prevent scope leakage.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/server/InputHandler.ts`:
- Around line 41-46: The current debug log in InputHandler.ts exposes up to 30
chars of clipboard content (variable content from clipboard.getContent()), which
is a privacy risk; update the try/catch around clipboard.getContent() to stop
printing content.substring(0, 30) — either remove the debug console.log entirely
or replace it with a non-sensitive message such as logging only the content
length (e.g., content.length) and preserve the existing catch behavior that logs
a generic warning; ensure you update the console.log call that references
clipboard content so no actual clipboard data is written to logs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/Trackpad/ControlBar.tsx (2)
63-89: Buttons missing explicittype="button"attribute.All
<button>elements here (and inClipboardButtons.tsx) lack an explicittypeprop. The default is"submit", which can cause unintended form submissions if these components are ever rendered inside a<form>. Addingtype="button"is a quick fix.Example fix for one button
<button className={`btn btn-sm ${scrollMode ? "btn-primary" : "btn-outline"}`} + type="button" onPointerDown={(e) => handleInteraction(e, onToggleScroll, "ToggleScroll")} >Apply similarly to all buttons in this file and in
ClipboardButtons.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Trackpad/ControlBar.tsx` around lines 63 - 89, The buttons in ControlBar.tsx (the Scroll/Cursor toggle, R-Click, modifier toggle using getModifierButtonClass()/getModifierLabel(), and Keyboard button that call handleInteraction) are missing explicit type attributes and default to "submit"; update each <button> to include type="button" to prevent accidental form submission, and apply the same change to all buttons rendered by the ClipboardButtons component in ClipboardButtons.tsx so none of those buttons default to type="submit".
30-34: Console logging on every interaction may be noisy in production.
handleInteractionlogs every button click. Consider gating this behind a debug flag or removing it before release to keep the console clean for end users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Trackpad/ControlBar.tsx` around lines 30 - 34, The console.log in handleInteraction is noisy for production; update handleInteraction (in ControlBar / function handleInteraction) to remove the unconditional console.log or gate it behind a debug flag/dev check (e.g., a prop like debug or NODE_ENV !== 'production') so clicks are only logged when debugging is enabled; ensure the chosen flag is evaluated before logging and keep the action() call unchanged.src/server/InputHandler.ts (1)
82-82:Promise<any>[]could use a more specific type.
Promise<any>[]loses type safety. If the nut-js scroll methods returnPromise<void>, usePromise<void>[]instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/InputHandler.ts` at line 82, The array declaration uses a broad Promise<any>[] which loses type safety; update the type of the promises array (variable name: promises) to Promise<void>[] in InputHandler.ts (e.g., inside the method where promises is declared) to match the nut-js scroll methods' return type, and ensure all pushed values are Promise<void> (or cast/adjust return types where necessary) so the compiler enforces correct usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/server/InputHandler.ts`:
- Around line 17-22: translateKey currently only maps the string 'control' to
'meta' on macOS, but clients may send the abbreviation 'ctrl'; update the
translateKey function (and its isMac check) to treat both 'control' and 'ctrl'
equivalently by checking key.toLowerCase() against both values (e.g., key ===
'control' || key === 'ctrl') and returning 'meta' on macOS so Ctrl inputs from
the client map to the Mac Command (Meta) key.
---
Duplicate comments:
In `@src/server/InputHandler.ts`:
- Around line 41-46: The clipboard logging change is fine — only the content
length is logged — but update the try block in InputHandler (where
clipboard.getContent() is called) to robustly handle undefined/null content and
use the app's logger API instead of console.log: call clipboard.getContent(),
compute const len = content ? content.length : 0, and log a single safe message
via the project's logger (e.g., processLogger or the existing logging utility)
like "[Clipboard] Copied content length: <len>"; keep the catch handler but
change its message to use the same logger.
- Around line 31-58: No code changes required — the clipboard handling in the
'clipboard' case correctly selects modKey (isMac ? Key.LeftSuper :
Key.LeftControl), handles 'copy' and 'paste' branches, wraps
clipboard.getContent() in its own try/catch, and only logs content length; leave
the logic around msg.clipboardAction, keyboard.pressKey/releaseKey, and
clipboard.getContent as-is.
---
Nitpick comments:
In `@src/components/Trackpad/ControlBar.tsx`:
- Around line 63-89: The buttons in ControlBar.tsx (the Scroll/Cursor toggle,
R-Click, modifier toggle using getModifierButtonClass()/getModifierLabel(), and
Keyboard button that call handleInteraction) are missing explicit type
attributes and default to "submit"; update each <button> to include
type="button" to prevent accidental form submission, and apply the same change
to all buttons rendered by the ClipboardButtons component in
ClipboardButtons.tsx so none of those buttons default to type="submit".
- Around line 30-34: The console.log in handleInteraction is noisy for
production; update handleInteraction (in ControlBar / function
handleInteraction) to remove the unconditional console.log or gate it behind a
debug flag/dev check (e.g., a prop like debug or NODE_ENV !== 'production') so
clicks are only logged when debugging is enabled; ensure the chosen flag is
evaluated before logging and keep the action() call unchanged.
In `@src/server/InputHandler.ts`:
- Line 82: The array declaration uses a broad Promise<any>[] which loses type
safety; update the type of the promises array (variable name: promises) to
Promise<void>[] in InputHandler.ts (e.g., inside the method where promises is
declared) to match the nut-js scroll methods' return type, and ensure all pushed
values are Promise<void> (or cast/adjust return types where necessary) so the
compiler enforces correct usage.
Comment on lines
+17
to
+22
| const isMac = process.platform === 'darwin'; | ||
|
|
||
| const translateKey = (key: string): string => { | ||
| if (isMac && key.toLowerCase() === 'control') return 'meta'; | ||
| return key; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -i "ctrl|control" --type=ts -C2Repository: AOSSIE-Org/Rein
Length of output: 6366
🏁 Script executed:
rg -n "translateKey" --type=ts -B3 -A3Repository: AOSSIE-Org/Rein
Length of output: 933
🏁 Script executed:
rg -n "KEY_MAP|nutKeys\|translatedKeys" --type=ts -B2 -A2 src/server/InputHandler.tsRepository: AOSSIE-Org/Rein
Length of output: 803
translateKey misses 'ctrl' abbreviation — fix translation for macOS compatibility.
The client sends 'ctrl' as the key name (ExtraKeys.tsx line 12), but translateKey only translates 'control' to 'meta' on macOS. Since 'ctrl'.toLowerCase() === 'control' is false, the 'ctrl' abbreviation passes through untranslated, causing macOS users to send LeftControl instead of LeftSuper (Cmd). Update the check to handle both 'control' and 'ctrl'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/InputHandler.ts` around lines 17 - 22, translateKey currently only
maps the string 'control' to 'meta' on macOS, but clients may send the
abbreviation 'ctrl'; update the translateKey function (and its isMac check) to
treat both 'control' and 'ctrl' equivalently by checking key.toLowerCase()
against both values (e.g., key === 'control' || key === 'ctrl') and returning
'meta' on macOS so Ctrl inputs from the client map to the Mac Command (Meta)
key.
Contributor
|
@b-u-g-g I’ve reviewed your PR and the functionality works as expected However, your branch is currently several commits behind the base branch. Please update your branch and resolve any conflicts that arise and resolve all the code rabbit comments. Additionally, there’s no need to push changes to package.json or the lock file unless your PR specifically introduces or modifies dependencies. Please remove those changes if they’re not required. Also, since there are two PRs addressing the same issue, please coordinate with @Nakshatra480 so we can consolidate the work. We only need one final PR for this feature. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Copy and Paste buttons in
ControlBar.tsxwere non-functional. The initial proposed solution of using['control', 'c/v']key-combos was identified as non-scalable, specifically because it risks killing active processes in terminal environments (SIGINT).Solution
Implemented a native clipboard pipeline that moves logic from simulated keyboard input to direct OS clipboard interaction via
nut-js.InputHandler.tsto include a newclipboardmessage type. This usesclipboard.getContent()to read the OS buffer andkeyboard.type()for pasting, ensuring the OS handles the context (like text selection) rather than a raw interrupt signal.Cross-Platform Compatibility
onPointerDownfor immediate response.useRefguard inClipboardButtons.tsx. It fires ononPointerDownand usesonPointerUpas a fallback if the initial event is swallowed, preventing double-firing while ensuring reliability on mobile browsers.File Structure
The clipboard logic remains isolated in the
actions/folder to keepControlBar.tsxclean and focused on layout.