Refactor: a review screen for recovery attempts #4200
Conversation
Branch preview✅ Deploy successful! Storybook: |
katspaugh
force-pushed
the
review-recovery
branch
from
ceb2aba
to
1cb6dfd
Compare
ESLint Summary View Full Report
Report generated by eslint-plus-action |
📦 Next.js Bundle Analysis for safe-wallet-webThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
| Page | Size (compressed) |
|---|---|
global |
954.63 KB (🟡 +149 B) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Coverage report
Show files with reduced coverage 🔻
Test suite run success1484 tests passing in 203 suites. Report generated by 🧪jest coverage report action from a15acaf |
src/components/tx-flow/flows/RecoveryAttempt/RecoveryAttemptReview.tsx
Outdated
Show resolved
Hide resolved
src/components/tx-flow/flows/RecoveryAttempt/RecoveryAttemptReview.tsx
Outdated
Show resolved
Hide resolved
src/components/tx-flow/flows/RecoveryAttempt/RecoveryAttemptReview.tsx
Outdated
Show resolved
Hide resolved
| trackError(Errors._812, e) | ||
| setSubmitError(err) | ||
| } | ||
| setTxFlow(<RecoveryAttemptFlow params={recovery} />) |
There was a problem hiding this comment.
Its not part of this PR, but just to bring one idea if it wasn't noticed yet:
It would be nice if we could change the way how we set the current transaction flow component, like setting a enum on the state or any other primitive value instead of the component call would save us some undesired reconciliations and re-renders because react doesn't memoize non-primitive states.
ESLint Summary View Full Report
Report generated by eslint-plus-action |
katspaugh
force-pushed
the
review-recovery
branch
from
e07dbf2
to
9d40f6e
Compare
src/components/tx-flow/flows/RecoveryAttempt/RecoveryAttemptReview.tsx
Outdated
Show resolved
Hide resolved
| <FieldsGrid title="Initiated by"> | ||
| <EthHashInfo address={item.address} showAvatar hasExplorer showName /> | ||
| </FieldsGrid> |
There was a problem hiding this comment.
I would expect to see the recoverer address here (the wallet that initiated this recovery attempt) but item.address is the contract address of the delay modifier.
There was a problem hiding this comment.
item.executor seems to be the recoverer address
There was a problem hiding this comment.
We could even include the validity time since its part of item i.e. say Confirm or reject this transaction until 20.11.2024 or if it never expires i.e. item.expiresAt is null we could say Confirm or reject this transaction
|
I just noticed that it is possible to change the nonce of the transaction even though it is already queued. Can we make it a read-only field? |
|
@usame-algan good catch, I've added hideNonce as it's a module transaction. 
|
|
Minor issue. For comparison, you can see how in spending limits, the amount of confirmations go away when the option is selected. This is the same case |
|
Fixed ✅ 
|
katspaugh
force-pushed
the
review-recovery
branch
from
3da526e
to
a15acaf
Compare
|
Looks good! |
What it solves
Resolves #4188
How this PR fixes it
Adds a simple review screen similar to the Spening Limit review component.
How to test it