-
-
Notifications
You must be signed in to change notification settings - Fork 266
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
chore: refactoring of the TxDetailModal to split the BumpFee (rbf) flow #15977
Conversation
...rc/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ChangeFee.tsx
Show resolved
Hide resolved
...ponents/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ReplaceTxButton.tsx
Show resolved
Hide resolved
...rc/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ChangeFee.tsx
Show resolved
Hide resolved
...uite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModal.tsx
Show resolved
Hide resolved
...uite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModal.tsx
Show resolved
Hide resolved
.../src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModalBase.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing 🚀
I am delighted to witness such maximum despaghettification. The bottom content, heading, buttons callbacks, all that used to be such an iffy labyrinth of ifs and now it's neatly organised by concern 🙌 ❤️
Tested briefly with regtest, seems to be working fine, the bug with RBF fixed ✔️
EDIT: now I see that TS is broken in useRbfForm.test.tsx. That might be harder to update, as you'll need to render the context to test the <ReplaceTxButton />
, or it might be ez, idk..
...ponents/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ReplaceTxButton.tsx
Outdated
Show resolved
Hide resolved
c97c0ef
to
6f3d9fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good after updating the test, I'm very glad that in the end, the unit tests did verify the code is working correctly 👍
...uite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModal.tsx
Outdated
Show resolved
Hide resolved
6f3d9fe
to
1c5a5a9
Compare
🚀 Expo preview is ready!
|
1c5a5a9
to
0e622f0
Compare
In this PR I am separating the code for
TxDetailModal
andTxDetailModal
with bump-fee (RBF) form in it. The issue here is, thatuseRbf
is used twice, so the internal form state is initialized twice. User changes fee in one, but when they submit it, the different state is used (with just default values). This result in the bug, there the custom fee is not used when set.👉 For code-review, it will be best go commit by commit.
Resolves: #15598