fix: close batch flow when removing last draft #2453
Conversation
Branch preview✅ Deploy successful! https://close_batch_flow--walletweb.review-wallet-web.5afe.dev |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
| setTxFlow(undefined) | ||
| } | ||
| }, | ||
| [deleteTx, batchTxs, setTxFlow], |
There was a problem hiding this comment.
I know React recommends using event handlers but I would do this in an effect instead.
There was a problem hiding this comment.
Is there a benefit of doing this in an effect over adding it to the onDelete handler?
There was a problem hiding this comment.
On delete is probably more robust. But at the same time deleting a tx and closing a modal are two completely different actions. Closing the modal is a side effect of having nothing to display. A matter of taste in the end, imperative vs declarative.
| const shouldExitFlow = !!txFlow && batchTxs.length === 0 | ||
| useEffect(() => { | ||
| if (shouldExitFlow) { | ||
| setTxFlow(undefined) |
There was a problem hiding this comment.
Can we somehow couple this to the user action i.e. add it to the deleteTx call instead of having it in a useEffect?
There was a problem hiding this comment.
Btw can there be a case when there’s some other tx flow open, not a batch modal?
There was a problem hiding this comment.
Btw can there be a case when there’s some other tx flow open, not a batch modal?
I've updated this accordingly.
|
Looks good to me I created a batched tx with a couple of tx inside. Clicked "Confirm batch" to go the "batch tx creation", deleted all the transaction and as I deleted the last one the form closed and the page was redirected to the dashboard |
What it solves
Resolves #2428
How this PR fixes it
If the batch confirmation flow is open, it is now closed when the last draft is removed via the sidebar.
How to test it
Batch transaction(s) and enter the confirmation flow. Whilst in the flow, remove the last draft from the sidebar and observe the confirmation flow close. It should not close other flows that may be open under the sidebar.
Screenshots
Checklist