Conversation
DonLakeFlyer
commented
Dec 5, 2025
DonLakeFlyer
commented
- More UI reorg
- Next/Prev buttons
- Delete/Hamburger at bottom
There was a problem hiding this comment.
Pull request overview
This PR reorganizes the mission item editor UI by moving controls to improve the user experience. The main changes include adding Previous/Next navigation buttons at the top of the mission item editor panel, relocating the Delete and Hamburger menu buttons to the bottom alongside the command description, and adjusting visual styling (transparency and opacity) for better panel visibility.
Key Changes
- Added navigation controls (Previous/Next buttons) for browsing mission items sequentially
- Relocated Delete and Hamburger menu buttons from top to bottom of the editor panel
- Updated panel backgrounds to use transparent color and increased right panel opacity from 0.2 to 0.85
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/QmlControls/SimpleItemEditor.qml | Changed background to transparent and removed top margin from height calculation |
| src/QmlControls/PlanView.qml | Added Previous/Next navigation buttons above MissionItemEditor and increased right panel opacity |
| src/QmlControls/MissionSettingsEditor.qml | Changed background color from windowShadeDark to transparent |
| src/QmlControls/MissionItemEditor.qml | Restructured layout moving Delete/Hamburger buttons to bottom row with command description |
| src/MissionManager/MissionSettingsItem.h | Updated command description from "Mission Start" to "Initial Mission Settings" |
| src/MissionManager/MissionController.h | Added documentation for the force parameter in setCurrentPlanViewSeqNum |
| QGCColoredImage { | ||
| Layout.fillHeight: true | ||
| Layout.preferredWidth: height | ||
| source: "/InstrumentValueIcons/backward.svg" | ||
| color: qgcPal.buttonText | ||
|
|
||
| MouseArea { | ||
| anchors.fill: parent | ||
| onClicked: { | ||
| if (_missionController.currentPlanViewVIIndex > 1) { | ||
| let prevItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex - 1) | ||
| _missionController.setCurrentPlanViewSeqNum(prevItem.sequenceNumber, false) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing accessibility support: The "Previous" navigation button lacks accessible text. Add Accessible.name property to describe the button's purpose for screen readers, e.g., Accessible.name: qsTr("Previous mission item").
| QGCColoredImage { | ||
| Layout.fillHeight: true | ||
| Layout.preferredWidth: height | ||
| source: "/InstrumentValueIcons/forward.svg" | ||
| color: qgcPal.buttonText | ||
|
|
||
| MouseArea { | ||
| anchors.fill: parent | ||
| onClicked: { | ||
| if (_missionController.currentPlanViewVIIndex < _missionController.visualItems.count - 1) { | ||
| let nextItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex + 1) | ||
| _missionController.setCurrentPlanViewSeqNum(nextItem.sequenceNumber, false) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing accessibility support: The "Next" navigation button lacks accessible text. Add Accessible.name property to describe the button's purpose for screen readers, e.g., Accessible.name: qsTr("Next mission item").
| QGCColoredImage { | ||
| id: deleteButton | ||
| Layout.preferredWidth: visible ? _hamburgerSize : 0 | ||
| Layout.preferredHeight: _hamburgerSize | ||
| Layout.fillHeight: true | ||
| source: "/res/TrashDelete.svg" | ||
| sourceSize.height: _hamburgerSize | ||
| fillMode: Image.PreserveAspectFit | ||
| mipmap: true | ||
| smooth: true | ||
| color: qgcPal.text | ||
| visible: missionItem.sequenceNumber !== 0 | ||
|
|
||
| QGCMouseArea { | ||
| fillItem: parent | ||
| onClicked: remove() | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing accessibility support: The delete button lacks accessible text. Add Accessible.name and Accessible.description properties to describe the button's purpose for screen readers, e.g., Accessible.name: qsTr("Delete") and Accessible.description: qsTr("Delete mission item").
| QGCColoredImage { | ||
| id: hamburger | ||
| Layout.alignment: Qt.AlignRight | ||
| Layout.preferredWidth: visible ? _hamburgerSize : 0 | ||
| Layout.preferredHeight: _hamburgerSize | ||
| sourceSize.height: _hamburgerSize | ||
| source: "qrc:/qmlimages/Hamburger.svg" | ||
| color: qgcPal.text | ||
| visible: missionItem.sequenceNumber !== 0 | ||
|
|
||
| QGCMouseArea { | ||
| fillItem: hamburger | ||
|
|
||
| onClicked: (position) => { | ||
| currentItemScope.focus = true | ||
| position = Qt.point(position.x, position.y) | ||
| // For some strange reason using mainWindow in mapToItem doesn't work, so we use globals.parent instead which also gets us mainWindow | ||
| position = mapToItem(globals.parent, position) | ||
| var dropPanel = hamburgerMenuDropPanelComponent.createObject(mainWindow, { clickRect: Qt.rect(position.x, position.y, 0, 0) }) | ||
| dropPanel.open() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing accessibility support: The hamburger menu button lacks accessible text. Add Accessible.name and Accessible.description properties to describe the button's purpose for screen readers, e.g., Accessible.name: qsTr("Menu") and Accessible.description: qsTr("Open mission item menu").
| anchors.fill: parent | ||
| onClicked: { | ||
| if (_missionController.currentPlanViewVIIndex > 1) { | ||
| let prevItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex - 1) | ||
| _missionController.setCurrentPlanViewSeqNum(prevItem.sequenceNumber, false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing keyboard navigation support: The Previous button should support keyboard interaction. Consider replacing MouseArea with Button or ToolButton, or add Keys.onReturnPressed and activeFocusOnTab: true to enable keyboard accessibility.
| anchors.fill: parent | |
| onClicked: { | |
| if (_missionController.currentPlanViewVIIndex > 1) { | |
| let prevItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex - 1) | |
| _missionController.setCurrentPlanViewSeqNum(prevItem.sequenceNumber, false) | |
| } | |
| } | |
| anchors.fill: parent | |
| activeFocusOnTab: true | |
| onClicked: { | |
| if (_missionController.currentPlanViewVIIndex > 1) { | |
| let prevItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex - 1) | |
| _missionController.setCurrentPlanViewSeqNum(prevItem.sequenceNumber, false) | |
| } | |
| } | |
| Keys.onReturnPressed: { | |
| if (_missionController.currentPlanViewVIIndex > 1) { | |
| let prevItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex - 1) | |
| _missionController.setCurrentPlanViewSeqNum(prevItem.sequenceNumber, false) | |
| } | |
| } | |
| Keys.onSpacePressed: { | |
| if (_missionController.currentPlanViewVIIndex > 1) { | |
| let prevItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex - 1) | |
| _missionController.setCurrentPlanViewSeqNum(prevItem.sequenceNumber, false) | |
| } | |
| } |
| anchors.rightMargin: _toolsMargin | ||
| width: _utmspEnabled ? _rightPanelWidth + ScreenTools.defaultFontPixelWidth * 21.667 : _rightPanelWidth | ||
| color: qgcPal.window | ||
| opacity: 0.85 |
There was a problem hiding this comment.
[nitpick] Significant opacity increase from 0.2 to 0.85 could impact map visibility. The right panel background is now much more opaque (85% vs 20%), which may obscure more of the underlying map. Consider whether this opacity level provides sufficient contrast for the panel contents while maintaining adequate map visibility, especially for users who need to see map details behind the panel.
| opacity: 0.85 | |
| opacity: 0.2 |
| QGCLabel { | ||
| Layout.fillWidth: true | ||
| horizontalAlignment: Text.AlignHCenter | ||
| text: _missionController.currentPlanViewItem.commandName |
There was a problem hiding this comment.
Potential null pointer access: _missionController.currentPlanViewItem is not checked for null before accessing .commandName. This could cause a runtime error if currentPlanViewItem is null when this component is displayed.
| text: _missionController.currentPlanViewItem.commandName | |
| text: _missionController.currentPlanViewItem ? _missionController.currentPlanViewItem.commandName : "" |
| QGCColoredImage { | ||
| Layout.fillHeight: true | ||
| Layout.preferredWidth: height | ||
| source: "/InstrumentValueIcons/backward.svg" | ||
| color: qgcPal.buttonText | ||
|
|
||
| MouseArea { | ||
| anchors.fill: parent | ||
| onClicked: { | ||
| if (_missionController.currentPlanViewVIIndex > 1) { | ||
| let prevItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex - 1) | ||
| _missionController.setCurrentPlanViewSeqNum(prevItem.sequenceNumber, false) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing visual feedback: The Previous navigation button should have visual indication when it's at the boundary (e.g., reduced opacity or disabled state) to inform users when they can't navigate further backward. Consider adding enabled: _missionController.currentPlanViewVIIndex > 1 and adjust the color or opacity based on the enabled state.
| QGCColoredImage { | ||
| Layout.fillHeight: true | ||
| Layout.preferredWidth: height | ||
| source: "/InstrumentValueIcons/forward.svg" | ||
| color: qgcPal.buttonText | ||
|
|
||
| MouseArea { | ||
| anchors.fill: parent | ||
| onClicked: { | ||
| if (_missionController.currentPlanViewVIIndex < _missionController.visualItems.count - 1) { | ||
| let nextItem = _missionController.visualItems.get(_missionController.currentPlanViewVIIndex + 1) | ||
| _missionController.setCurrentPlanViewSeqNum(nextItem.sequenceNumber, false) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing visual feedback: The Next navigation button should have visual indication when it's at the boundary (e.g., reduced opacity or disabled state) to inform users when they can't navigate further forward. Consider adding enabled: _missionController.currentPlanViewVIIndex < _missionController.visualItems.count - 1 and adjust the color or opacity based on the enabled state.
|
|
||
| signal clicked | ||
| signal remove | ||
| signal selectNextNotReadyItem |
There was a problem hiding this comment.
Removed signal still in use: The clicked signal declaration was removed, but the signal is still being emitted elsewhere in the file (line 58: _root.clicked()). This will cause a runtime error when the MouseArea's onClicked handler is triggered. Either restore the signal clicked declaration or ensure the emission at line 58 is also removed.
| signal selectNextNotReadyItem | |
| signal selectNextNotReadyItem | |
| signal clicked |
DonLakeFlyer
force-pushed
the
MorePlan
branch
from
8b00a6c to
1adcd47
Compare
