feat: add real-time latency indicator and websocket heartbeat#84
feat: add real-time latency indicator and websocket heartbeat#84Nakshatra480 wants to merge 4 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds an application-level ping/pong heartbeat: the frontend hook sends periodic pings and measures RTT, the server replies with pong, and the Trackpad UI displays latency (ms) with color-coded indicator in the ControlBar. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client Browser
participant Hook as useRemoteConnection Hook
participant WS as WebSocket Server
participant UI as ControlBar
Browser->>Hook: Render TrackpadPage
Hook->>WS: Open WebSocket
Note over Hook: Start ping timer (every 3s)
loop Heartbeat interval
Hook->>WS: {"type":"ping","timestamp":T0}
WS->>Hook: {"type":"pong","timestamp":T0}
Hook->>Hook: RTT = now() - T0
Hook->>UI: update latency prop
UI->>UI: render "Ping: Xms" with color
end
Browser->>Hook: Unmount / Disconnect
Hook->>Hook: clear timer, close ws, reset latency
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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
🤖 Fix all issues with AI agents
In `@src/components/Trackpad/ControlBar.tsx`:
- Around line 73-116: The grid is defined as "grid-cols-5" but there are six
active buttons in ControlBar (the Scroll/Cursor button using onToggleScroll,
Copy, Paste, R-Click using onRightClick, Modifier using
getModifierButtonClass/getModifierLabel and onModifierToggle, and Keyboard using
onKeyboardToggle), causing wrapping; update the grid to "grid-cols-6" (or reduce
buttons to five) so all buttons fit on one row—adjust the className on the
container div accordingly.
- Around line 73-116: Every <button> in ControlBar.tsx lacks an explicit type
and therefore defaults to type="submit"; update each button element (the
Scroll/Cursor toggle that calls handleInteraction with onToggleScroll, the Copy
and Paste buttons, the R-Click button that calls handleInteraction with
onRightClick, the modifier button that uses
getModifierButtonClass/getModifierLabel and onModifierToggle, and the Keyboard
button that calls onKeyboardToggle) to include type="button" to prevent
accidental form submission.
In `@src/hooks/useRemoteConnection.ts`:
- Around line 55-59: The cleanup closure currently captures the stale ws state
(initially null) so ws?.close() is never called; change the WebSocket storage to
a ref (e.g., wsRef via useRef<WebSocket | null>) and in connect() assign
wsRef.current = socket (and update any other ws usages to read wsRef.current),
then in the cleanup return call wsRef.current?.close() alongside clearing
reconnectTimer and heartbeatTimer (and null out wsRef.current on close) so the
real socket is reliably closed on unmount.
🧹 Nitpick comments (2)
src/hooks/useRemoteConnection.ts (1)
21-28: Heartbeat fires immediately after reconnect — consider a brief delay.When the WebSocket reconnects (after the 3s reconnect timeout), the first heartbeat ping fires immediately in
onopen, then every 3 seconds viasetInterval. This is fine functionally, but if you want the first latency reading sooner after connect, you could send an initial ping immediately inonopenbefore starting the interval.This is a minor UX observation — the first latency display won't appear until up to 3 seconds after connection.
Optional: send initial ping immediately
socket.onopen = () => { setStatus('connected'); + // Send first ping immediately for quick latency reading + socket.send(JSON.stringify({ type: 'ping', timestamp: Date.now() })); heartbeatTimer = setInterval(() => { if (socket.readyState === WebSocket.OPEN) { socket.send(JSON.stringify({ type: 'ping', timestamp: Date.now() })); } }, 3000); };src/components/Trackpad/ControlBar.tsx (1)
80-97: Non-functional Copy/Paste buttons and commented-out L-Click code.The Copy and Paste buttons (lines 80–89) have no event handlers — they render but do nothing on interaction. The L-Click button (lines 90–97) is fully commented out. If these are WIP placeholders, consider removing them from this PR to keep the diff focused on the latency feature, or add TODO comments indicating they're planned.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/Trackpad/ControlBar.tsx`:
- Around line 81-92: The Copy and Paste buttons in the ControlBar component are
rendered without handlers so they are non-functional; either wire them up or
mark them disabled for now. Locate the ControlBar component (buttons with text
"Copy" and "Paste") and either add onClick handlers that call the appropriate
functions (e.g., copySelection, pasteClipboard or existing methods in the same
component) or set the buttons to non-interactive by adding the disabled
attribute and a CSS class for reduced opacity/aria-disabled for accessibility.
Ensure any new handlers use existing clipboard utilities or component methods
and keep ARIA attributes consistent.
In `@src/hooks/useRemoteConnection.ts`:
- Around line 4-5: The ws state (useState WebSocket | null) is dead — remove the
unused state and its updater to avoid needless re-renders: delete the const [ws,
setWs] = useState<WebSocket | null>(null); declaration in the
useRemoteConnection hook and remove the setWs(socket) call where the WebSocket
is assigned (leaving wsRef.current = socket and all send/sendCombo/cleanup logic
using wsRef intact).
🧹 Nitpick comments (1)
src/hooks/useRemoteConnection.ts (1)
31-41: RTT calculation assumesmsg.timestampis echoed back as a number.If the server-side handler ever coerces the timestamp to a string (e.g., via JSON key handling),
Date.now() - msg.timestampwould still work due to JS implicit coercion, but a negative or nonsensical RTT could surface if the field is missing or malformed. Consider a guard:if (msg.type === 'pong') { - const rtt = Date.now() - msg.timestamp; - setLatency(rtt); + const rtt = Date.now() - msg.timestamp; + if (Number.isFinite(rtt) && rtt >= 0) { + setLatency(rtt); + } }
|
Hi @imxade i have added a latency indicator and websocket heartbeat feature, can you review this PR. |
1 participant
Description
This PR adds a real-time connection quality indicator to the trackpad interface. It implements a lightweight heartbeat mechanism to measure the Round-Trip Time (RTT) between the client and the server, helping users diagnose lag or connection issues.
Changes
pongresponse handler inwebsocket.tsto return the client's timestamp.useRemoteConnectionto initiate a heartbeat every 3 seconds and calculate latency.ControlBar:Verification
---when disconnected.Closes #83
Summary by CodeRabbit