Warn user about auto-arming vehicle#2065
Conversation
ArturoManzoli
force-pushed
the
608-warn-about-auto-arm-on-play
branch
3 times, most recently
from
6d97c75 to
ddcd025
Compare
rafaellehmkuhl
left a comment
rafaellehmkuhl
left a comment
There was a problem hiding this comment.
Working fine. Just some code minors to take a look into.
|
|
||
| // Allow executing missions | ||
| const executeMissionOnVehicle = async (): Promise<void> => { | ||
| if (missionStore.showChecklistBeforeArm === true) { |
There was a problem hiding this comment.
No need for the === true there.
if (missionStore.showChecklistBeforeArm) {
| try { | ||
| await vehicleStore.startMission() | ||
| } catch (error) { | ||
| openSnackbar({ message: 'Failed to start mission.', variant: 'error' }) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
This code is duplicating the one inside the handleMissionChecklistConfirmed method. Better to just do a handleMissionChecklistConfirmed(true) here.
Ideally, so it doesn't get out of context, "handleMissionChecklistConfirmed" should be renamed to "tryToStartMission", without any arguments, and in the @confirmed directive just use the following:
@confirmed="tryToStartMission"
as the @confirm will only be executed when the user clicks to go.
Them you can reuse tryToStartMission() here.
Signed-off-by: Arturo Manzoli <arturomanzoli@gmail.com>
ArturoManzoli
force-pushed
the
608-warn-about-auto-arm-on-play
branch
from
ddcd025 to
0eef9bc
Compare
rafaellehmkuhl
left a comment
rafaellehmkuhl
left a comment
There was a problem hiding this comment.
Working perfectly!
Nice feature!
3 participants
Relates to #22
Closes #608