Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check before opening hamburger config menu while vehicle is armed #784

Merged

Conversation

JoaoMario109
Copy link
Contributor

Add a check before opening hamburger configuration menu while vehicle is armed to avoid users changing configurations while vehicle still armed preventing potential dangerous actions
Closes #730

@JoaoMario109
Copy link
Contributor Author

Currently no alert popup is shown when the joystick configuration view is opened and vehicle is automatically disabled, we could add a Swal alert but when the user either interact with it or press ESC it will close the configuration menu itself due to it being listen to these events, so in this case we would need to change this behavior, or maybe auto close the popup

src/App.vue Outdated Show resolved Hide resolved
src/App.vue Outdated Show resolved Hide resolved
src/App.vue Show resolved Hide resolved
@rafaellehmkuhl
Copy link
Member

Currently no alert popup is shown when the joystick configuration view is opened and vehicle is automatically disabled, we could add a Swal alert but when the user either interact with it or press ESC it will close the configuration menu itself due to it being listen to these events, so in this case we would need to change this behavior, or maybe auto close the popup

I would say that auto-closing the popup would be enough for now, but the reality is that if the popup appears, there's a high risk of the user clicking outside it to close it and (I think) that will close the config menu also, so I would say for us to do the right way and put a close button. Can you open an issue to track that for a following PR?

* Add a check layer to open the hamburger menu when the vehicle is armed
Comment on lines 329 to 333
// Automatically disarms the vehicle if it is armed and entering joystick configuration
if (isArmed) {
disarm()
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this auto-disarm we should actually remove. It's a too dangerous operation to be done automatically.

I think it's better for us to relegate that decision to the user than to simply disarm his vehicle without further notice.

@rafaellehmkuhl rafaellehmkuhl merged commit 7b3e900 into bluerobotics:master Feb 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

we shouldn't allow configuring the joystick when armed.
2 participants