fix(signing/views): allow discarding signing popup when tx in progress #10388
Conversation
Pull Request Checklist
|
| [button/button {:type :secondary | ||
| :container-style {:padding-horizontal 24} | ||
| :label (i18n/label :t/cancel) | ||
| :on-press #(re-frame/dispatch [:signing.ui/cancel-is-pressed])}]]) |
There was a problem hiding this comment.
Notice that transactions can't actually be cancelled, so hitting Cancel while the tx is in-flight really just discards the popup. Might be worth considering a different label depending on context.
0x-r4bbit
changed the title
As discussed in status-im#9977, there's an issue that the signing view popup cannot be cancelled/discarded when a transaction is in progress. This is because the `cancel-is-pressed` event is only conditionally applied to the cancel button, rendering it non-functional for the time-being. This commit changes that behaviour to always attach the event handler to the cancel button, so that the popup can be closed even when a transaction has been sent. Fixes status-im#9977
|
Let me preface with, I don't have a deep understanding of the issue. It seems to me that indeed I expect two things will happen:
In the future I expect an aggregated tx history and inapp notifications (include a tx pending state @errorists) to make it easier to recover from this glitch. wdyt @errorists ? |
|
oh I'm not familiar with the issue either, I've read the
I think so, yes |
|
i don't think its correct, because you can't cancel tx when its in progress, so let's say user entered password and pressed send, now he is waiting and looking at the loading indicator, and he decides oh I want to cancel this tx, and make a new one instead, but its not possible to cancel tx because its already sent to the blockchain, so probably cancel should be replaced by close |
|
100% agree it's weird. I've had someone in a shop hit Cancel on my crypto payment asking me to pay again 😠, because it seemed Cancel was an option. Changing the copy on the button however doesn't seem right either. A button is not something that should change without user action. An alternative would be to replace the loading state with an error state. Something like Again, though, feels like an edge case. And I'm happy with this as intermediary solution because the pending state is visible on the stickerpack and at least the user is no longer stuck having to close the app. I suggest creating another issue to describe desired behavior and put some more thought into that. |
|
Thanks for all the feedback @flexsurfer @hesterbruikman and @errorists !
So just to clarify, we're not necessarily even talking about an error state but rather a "pending". I think the issue can be broken down to: When a transaction is signed, it's in-flight (can't be undone) and until it completes (which might take a while) the I wonder always renaming that |
|
|
@flexsurfer I can update the label from I assume this PR should update labels for |
Summary
As discussed in #9977, there's an issue that the signing view popup cannot
be cancelled/discarded when a transaction is in progress.
This is because the
cancel-is-pressedevent is only conditionally applied tothe cancel button, rendering it non-functional for the time-being.
This commit changes that behaviour to always attach the event handler to
the cancel button, so that the popup can be closed even when a transaction
has been sent.
Fixes #9977
Review notes
One thing to keep in mind:
Once the transaction has been signed and is in progress, there's no way to "cancel" it, so hitting the "Cancel" button in the UI really just "discards" it. One could consider changing the label to "Discard", but it probably won't work well because the UI is used in a context where cancelling makes sense.
Testing notes
Best way to test this is to turn off the
signing/transaction-completedevent so the app thinks the transaction is in progress forever.Platforms