Add hold to confirm functionality in action that need confirmation #822
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This issue will be good to be solved prior to finishing this PR, it will allow to use the same component from slide to confirm. |
|
@JoaoMario109 It appears that the new version 0.5.4 adds a position props, please check it out. |
JoaoMario109
force-pushed
the
762-hold-to-confirm
branch
2 times, most recently
from
80ec876
to
de24abc
Compare
|
The PR containing modification to allow position to call completed was merged Right now I'm only keeping this as draft because I need to await the npm release, as soon as the new release is available I'll update the lib and make ready for review |
JoaoMario109
force-pushed
the
762-hold-to-confirm
branch
from
de24abc
to
2dcd642
Compare
JoaoMario109
requested review from
rafaellehmkuhl,
patrickelectric and
ArturoManzoli
Just checked and the latest version includes it! Nice! |
rafaellehmkuhl
approved these changes
Apr 5, 2024
|
@ericjohnson97 I think a quick look from you would also be worth it! The idea here as basically to:
|
rafaellehmkuhl
requested changes
Apr 5, 2024
There was a problem hiding this comment.
After testing, I have found some bugs:
- The ArmerButton is not working (after you slide, nothing happens)
- The HoldToConfirm button is not working (I've mapped it to my joystick, but holding it do nothing)
Some improvements that I think would come very handy for the user:
- A popup/alert/something telling the user that they need to map the HoldToConfirm button after ticking the "require confirmation" checkbox for some action (this one you mentioned already that was going to be added, so I'm adding here just for remembering purposed)
- There should be some visual feedback while the user is holding the confirm button. From the code, this appears to be the case already. I'm adding here because I didn't receive it, but it's probably related to the HoldToConfirm button not being working at all
- There's no timeout on the slider, which causes it to be there indefinitely if the user decides not to confirm it
- There's no timeout on the slider reject alert, which causes it to be there indefinitely until the user clicks outside it with the mouse, breaking the purpose of the HoldToConfirm functionality
JoaoMario109
force-pushed
the
762-hold-to-confirm
branch
from
d527590
to
6e3a995
Compare
* Add hold to confirm action in cockpit action protocol
* Add a enable/disable hold to confirm option in mission store to allow users to control this functionality
JoaoMario109
force-pushed
the
762-hold-to-confirm
branch
2 times, most recently
from
07935e5
to
a93dd6e
Compare
* Add hold to confirm action to map action from controller to percentage of complete confirm action
* Remove slide to confirm from store and move it to the outside UI elements that use it, with this, th confirmation can be segregated by UI and controller actions
* Add confirmation per cockpit joystick action, so users can choose if they want confimation when certains actions are mapped in joystick
* Add extra tooltips where hold to confirm or any extra confirmation step can be required explained to the user where it will affect.
* Add default text fields generation to avoid needing repetitive texts on calls * Add a default 5 seconds timeout on slide to confirm, so if user does not confirm in this interval will be auto denied
* Adjust tooltip location on require confirmation info tooltip
JoaoMario109
force-pushed
the
762-hold-to-confirm
branch
from
a93dd6e
to
3b496d6
Compare
rafaellehmkuhl
approved these changes
Apr 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #762