feat: remove increase fee summary dialog, closes #5305 #5377
Conversation
WalkthroughThis update enhances the transaction submission process by adding new parameters and success messages. Key changes include the introduction of an Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
alter-eggo
force-pushed
the
fix/increase-fee
branch
from
f845cef
to
67058a1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- src/app/common/hooks/use-submit-stx-transaction.ts (3 hunks)
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx (1 hunks)
- src/app/pages/transaction-request/transaction-request.tsx (1 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review due to trivial changes (5)
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts
- src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx
- src/app/routes/app-routes.tsx
- src/shared/route-urls.ts
Additional comments not posted (13)
src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx (2)
Line range hint
14-14: LGTM! The function correctly retrieves state values.
28-31: LGTM! The integration ofuseStacksBroadcastTransactionwith the new parameter is correct.src/app/common/hooks/use-submit-stx-transaction.ts (4)
22-22: LGTM! The newisIncreaseFeeTransactionparameter is correctly added to the interface.
29-32: LGTM! The function signature is correctly updated to include the new parameter.
60-64: LGTM! The toast messages are correctly updated based on the new parameter.
86-86: LGTM! The new parameter is correctly included in the dependency array.src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (5)
26-30: LGTM! The newisIncreaseFeeTransactionparameter is correctly added to the interface.
32-36: LGTM! The function signature is correctly updated to include the new parameter.
47-47: LGTM! The new parameter is correctly passed to theuseSubmitTransactionCallbackhook.
63-66: LGTM! The navigation logic correctly handles the new parameter.
136-136: LGTM! The new parameter is correctly included in the dependency array.src/app/pages/transaction-request/transaction-request.tsx (1)
53-53: LGTM! The integration ofuseStacksBroadcastTransactionwith the new parameter is correct.src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx (1)
48-51: LGTM! The added parameterstoken: 'STX'andisIncreaseFeeTransaction: trueare consistent with the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- src/app/common/hooks/use-submit-stx-transaction.ts (3 hunks)
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx (1 hunks)
- src/app/pages/transaction-request/transaction-request.tsx (1 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review as they are similar to previous changes (10)
- src/app/common/hooks/use-submit-stx-transaction.ts
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts
- src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx
- src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx
- src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx
- src/app/pages/transaction-request/transaction-request.tsx
- src/app/routes/app-routes.tsx
- src/shared/route-urls.ts
| @@ -53,7 +57,11 @@ export function useSubmitTransactionCallback({ loadingKey }: UseSubmitTransactio | |||
| txId: safelyFormatHexTxid(response.txid), | |||
| }); | |||
| await delay(500); | |||
| toast.success('Transaction submitted!'); | |||
| const toastMessage = isIncreaseFeeTransaction | |||
There was a problem hiding this comment.
We could define this as a function outside the hook?
There was a problem hiding this comment.
and pass in as callback 👍🏼
There was a problem hiding this comment.
I believe better to put in onSuccess callback, that was good idea
| export function useSubmitTransactionCallback({ loadingKey }: UseSubmitTransactionArgs) { | ||
| export function useSubmitTransactionCallback({ | ||
| loadingKey, | ||
| isIncreaseFeeTransaction, |
There was a problem hiding this comment.
Not big on this API decision here. What if we want to show another toast for a contract call transaction? Do we add isContractCallTransaction:boolean as well?
By adding a callback, or a custom toast message, we don't couple useSubmitTransactionCallback to a specific use case of it.
IMO the toast should be outside this function and called within the onSuccess() handler. Or at least, isSuccessfulToastMessage param.
|
@alter-eggo mind attaching a screen recording of the new UX to this PR as part of the description? 🙏 Ideally for all flows affected (e.g. BTC and STX sends) |
|
Also, do have an issue to link to this PR? |
alter-eggo
force-pushed
the
fix/increase-fee
branch
from
67058a1
to
e72473f
Compare
| onCancel(): void; | ||
| } | ||
| export function IncreaseFeeActions(props: IncreaseFeeActionsProps) { | ||
| const { onCancel, isDisabled } = props; | ||
| const { onCancel, isDisabled, isBroadcasting } = props; |
There was a problem hiding this comment.
to avoid further bigger senseless refactoring, just have put it here. this component would be removed in scope of Approval UX
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (2)
src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (1)
26-30: Ensure the new interfaceUseStacksBroadcastTransactionArgsis well-documented.Adding comments to explain each property, especially
isIncreaseFeeTransaction, would improve code maintainability and clarity.src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx (1)
Line range hint
17-17: Remove unused import to clean up the code.- import { LoadingSpinner } from '@app/components/loading-spinner';The
LoadingSpinneris imported but not used in the component. Removing unused imports can help reduce confusion and maintain cleaner code.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- src/app/common/hooks/use-submit-stx-transaction.ts (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx (2 hunks)
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx (2 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (5 hunks)
- src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx (1 hunks)
- src/app/pages/swap/hooks/use-stacks-broadcast-swap.tsx (1 hunks)
- src/app/pages/transaction-request/transaction-request.tsx (1 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- src/app/pages/swap/hooks/use-stacks-broadcast-swap.tsx
Files skipped from review as they are similar to previous changes (8)
- src/app/common/hooks/use-submit-stx-transaction.ts
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts
- src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx
- src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx
- src/app/pages/transaction-request/transaction-request.tsx
- src/app/routes/app-routes.tsx
- src/shared/route-urls.ts
Additional Context Used
GitHub Check Runs (2)
typecheck failure (3)
src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx: [failure] 17-17:
'LoadingSpinner' is declared but its value is never read.lint-eslint failure (13)
src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx: [failure] 133-133:
React Hook useMemo has a missing dependency: 'showToast'. Either include it or remove the dependency array
Additional comments not posted (3)
src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (2)
49-55: TheshowToastfunction is well-implemented to handle different toast messages based on the transaction type.
70-73: The navigation logic after a fee increase transaction is clear and concise.src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx (1)
75-79: TheIncreaseFeeActionscomponent is correctly updated to handle the broadcasting state.
| @@ -79,6 +102,7 @@ | |||
| }, | |||
| onSuccess(txId) { | |||
| handlePreviewSuccess(signedTx, txId); | |||
| showToast(); | |||
There was a problem hiding this comment.
The showToast function is called here but is not included in the dependency array of useMemo.
- }, [isBroadcasting, requestToken, tabId, navigate, token, formSentSummaryTxState, decimals, toast, broadcastTransactionFn, signStacksTransaction, isIncreaseFeeTransaction]);
+ }, [isBroadcasting, requestToken, tabId, navigate, token, formSentSummaryTxState, decimals, toast, broadcastTransactionFn, signStacksTransaction, isIncreaseFeeTransaction, showToast]);Including showToast in the dependency array ensures that the memoized value updates correctly when showToast changes.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| showToast(); | |
| }, [isBroadcasting, requestToken, tabId, navigate, token, formSentSummaryTxState, decimals, toast, broadcastTransactionFn, signStacksTransaction, isIncreaseFeeTransaction, showToast]); |
alter-eggo
force-pushed
the
fix/increase-fee
branch
from
e72473f
to
1abf358
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- src/app/common/hooks/use-submit-stx-transaction.ts (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx (2 hunks)
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts (1 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx (3 hunks)
- src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx (1 hunks)
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx (4 hunks)
- src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx (1 hunks)
- src/app/pages/swap/hooks/use-stacks-broadcast-swap.tsx (1 hunks)
- src/app/pages/transaction-request/transaction-request.tsx (1 hunks)
- src/app/routes/app-routes.tsx (2 hunks)
- src/shared/route-urls.ts (1 hunks)
Files skipped from review as they are similar to previous changes (11)
- src/app/common/hooks/use-submit-stx-transaction.ts
- src/app/features/dialogs/increase-fee-dialog/components/increase-fee-actions.tsx
- src/app/features/dialogs/increase-fee-dialog/hooks/use-btc-increase-fee.ts
- src/app/features/dialogs/increase-fee-dialog/increase-btc-fee-dialog.tsx
- src/app/features/dialogs/increase-fee-dialog/increase-stx-fee-dialog.tsx
- src/app/features/stacks-transaction-request/hooks/use-stacks-broadcast-transaction.tsx
- src/app/pages/send/send-crypto-asset-form/form/stacks/stacks-send-form-confirmation.tsx
- src/app/pages/swap/hooks/use-stacks-broadcast-swap.tsx
- src/app/pages/transaction-request/transaction-request.tsx
- src/app/routes/app-routes.tsx
- src/shared/route-urls.ts
This pr removes increase fee summary step, and just shows toast on broadcasting RBF transaction both in btc and all stacks related send flows. discussed here
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Removals
IncreaseFeeSentroute and associated dialog, streamlining the transaction process.