Create waypoint queue widget #25
Conversation
There was a problem hiding this comment.
Pretty good so far! Try looking into doing some data validation with the inputFormatters attr of Textfield, it makes sure the input matches a regex.
| void _queueWaypoint() { | ||
| _getWaypointsFromInput(); | ||
| widget.queueWaypoints.queueWaypoint( | ||
| widget.systemId, widget.componentId, _latitude, _longitude, _altitude); |
There was a problem hiding this comment.
nit: add a , after _altitude
| void _queueWaypointWithoutQueue() { | ||
| _getWaypointsFromInput(); | ||
| widget.queueWaypoints.sendWaypointWithoutQueue( | ||
| widget.systemId, widget.componentId, _latitude, _longitude, _altitude); |
There was a problem hiding this comment.
nit: add a , after _altitude
| widget.queueWaypoints.queueWaypoint( | ||
| widget.systemId, widget.componentId, _latitude, _longitude, _altitude); | ||
| setState(() { | ||
| _widgetKey.currentState?.reassemble(); |
There was a problem hiding this comment.
reassemble only runs on debug not builds, you might want to test the widget without this
There was a problem hiding this comment.
without this, the widget does not update the table when a waypoint is added. what other method could I use to refresh the table so that it repopulates everything in the queue?
| ), | ||
| ), | ||
| const SizedBox(height: 16), | ||
| ElevatedButton( |
There was a problem hiding this comment.
Try putting the last two ElevatedButtons in an OverflowBar widget with a set spacing attr
| const SizedBox(height: 16), | ||
| ElevatedButton( | ||
| onPressed: _queueWaypoint, | ||
| child: const Text('Add Waypoint to Queue')), |
There was a problem hiding this comment.
nit: add , between ))
| const SizedBox(height: 16), | ||
| ElevatedButton( | ||
| onPressed: _queueWaypointWithoutQueue, | ||
| child: const Text('Send Waypoint Immediately')), |
There was a problem hiding this comment.
nit: add , between ))
| 'Waypoint Queue', | ||
| style: TextStyle(fontWeight: FontWeight.bold), | ||
| ), | ||
| Expanded( |
There was a problem hiding this comment.
I would caution against using Expanded, could cause some weirdness when putting this next to other widgets
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return Column( |
There was a problem hiding this comment.
Temporarily wrap this in a SizedBox with a fixed height and width for testing and showcase. It will be merged this way but we'll change it when integrating widget together (which is pretty far down the line)
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { |
There was a problem hiding this comment.
Try to center the elements
| child: Row( | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| SizedBox( |
There was a problem hiding this comment.
Instead of using SizedBox for the TextFields, try to make them dynamically fill the size they have. Only start implementing this change once you wrap everything in a SizedBox with fixed width though, otherwise it won't render.
| border: OutlineInputBorder(), | ||
| hintText: 'Latitude', | ||
| ), | ||
| controller: _latitudeInput, |
There was a problem hiding this comment.
consider adding inputFormatters to the 'TextFields'. These use Regex, so you'll need find or create one that only accepts valid rational numbers.
| widget.queueWaypoints.sendNextWaypointInQueue(); | ||
| setState(() {}); | ||
| } | ||
|
|
There was a problem hiding this comment.
I've confirmed this with Aaron, you can send all the waypoints at once and mavlink will just kinda figure it out. The best way to do this is to create a method in the queueWaypoints class to handle this, lmk if you want to try doing that or if I should do that for you.
No description provided.