Removed Graph's tab, added movable windows instead#1004
Removed Graph's tab, added movable windows instead#1004Matthew-Dobson merged 23 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request removes the static Graphs tab from the main GCS application and replaces it with a more flexible system of movable, floating graph windows. The change allows users to pop out up to 4 graph windows that remain always-on-top, providing better multitasking capabilities. Users can now select MAVLink fields to graph from a new "Graphs" menu in the toolbar, which opens dedicated Electron windows for each graph.
Changes:
- Removed the
/graphsroute and its associated components (graphs.jsx, messageSelector.jsx, graphPanel.jsx) - Added a new Graphs menu in the toolbar for selecting MAVLink fields to display
- Implemented Electron-based floating graph windows with IPC communication for real-time data updates
- Enhanced the socketMiddleware with better error handling and safety checks for graph data extraction
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| gcs/src/graphs.jsx | Removed entire graphs page component (143 lines) |
| gcs/src/graphWindow.jsx | New entry point for popup graph windows with IPC listeners |
| gcs/graphWindow.html | HTML template for graph popup windows |
| gcs/src/components/toolbar/toolbar.jsx | Added GraphsMenu component to toolbar |
| gcs/src/components/toolbar/menus/graphs.jsx | New menu for selecting up to 4 MAVLink fields to graph with search functionality |
| gcs/src/components/realtimeGraph.jsx | Updated to handle dynamic lineColor changes |
| gcs/src/components/navbar.jsx | Removed Graphs navigation link |
| gcs/src/components/mainContent.jsx | Removed /graphs route |
| gcs/src/components/graphs/messageSelector.jsx | Removed component (no longer needed) |
| gcs/src/components/graphs/graphPanel.jsx | Removed component (no longer needed) |
| gcs/src/components/graphWindow/graphWindow.jsx | Appears to be an unused stub component |
| gcs/src/redux/middleware/socketMiddleware.js | Refactored graph data extraction with better error handling and IPC forwarding |
| gcs/electron/preload.js | Added IPC channels for graph window communication and removeListener method |
| gcs/electron/modules/graphWindow.ts | New module managing graph window lifecycle and IPC handlers |
| gcs/electron/main.ts | Integrated graph window module registration and cleanup |
| gcs/.env_sample | Deleted (appears unintentional - contains important env var template) |
| gcs/src/components/dashboard/resizableInfoBox.jsx | Fixed trailing whitespace in comment |
| gcs/vite.test.config.ts | Added missing newline at end of file |
Comments suppressed due to low confidence (1)
gcs/src/redux/middleware/socketMiddleware.js:533
- Missing null safety check for window.ipcRenderer. The code directly accesses window.ipcRenderer without verifying its existence, which could cause a runtime error if the code runs outside an Electron context. Add a check such as "if (graphData && window.ipcRenderer)" before invoking the IPC call.
)
})
// I don't understand whatsoever why this doesn't work with the standard
// on method.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
….com/Avis-Drone-Labs/FGCS into 977-display-graphs-as-popout-windows
|
What are the changes to |
Thanks for flagging - I think those are just unintended formatting changes done automatically, no actual content changes, not sure why VS pushed them this time and not any other lol, can remove if you want :) |
1Blademaster
left a comment
1Blademaster
left a comment
There was a problem hiding this comment.
Just some comments after briefly reading the code.
1Blademaster
left a comment
1Blademaster
left a comment
There was a problem hiding this comment.
Can we make the graph resize to the size of the window?:
Checkbox ticked even though window is closed:
No data being plotted?:
…ed ie the graph never pops out without plotting anything
|
Fixed all of the comments requested. Still have those changes to the wierd files like CONTRIBUTING.md , no idea why but nothing has changed in them and the rest seems good on my end |
|
Works well. 
Can we have a time window of like 30s? We can make it configurable in the future. |
|
Also noticed when I click an item on the checkbox then the menu doesn't hide automatically; it stays visible? |
I don't think we need this title text here; it's available in the graph legend and the window title which I think is enough; also means we can make the graph larger which is good. |
No description provided.