-
Notifications
You must be signed in to change notification settings - Fork 320
feat: add custom error and success states to txn button #1460
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
Changes from 6 commits
cb5314c
8260e9c
eb0f419
7c1347b
67b204e
268f0d2
6345ac9
0c375fc
932bca5
c06526a
7c42166
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@coinbase/onchainkit': patch | ||
--- | ||
|
||
- **feat**: Added ability to customize error and success states for TransactionButton. By @abcrane123 #1460 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,11 @@ import type { | |
|
||
export type Call = { to: Hex; data?: Hex; value?: bigint }; | ||
|
||
type TransactionButtonOverride = { | ||
text?: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are stuck with the text prop name, but any reason not to allow a ReactNode here? this would allow someone to use their own custom loading icon for the pending state, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah i agree i'll update type but keep |
||
onClick?: (receipt?: TransactionReceipt) => void; | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Define in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is used in type for |
||
|
||
/** | ||
* List of transaction lifecycle statuses. | ||
* The order of the statuses loosely follows the transaction lifecycle. | ||
|
@@ -71,6 +76,8 @@ export type TransactionButtonReact = { | |
className?: string; // An optional CSS class name for styling the button component. | ||
disabled?: boolean; // A optional prop to disable the submit button | ||
text?: string; // An optional text to be displayed in the button component. | ||
errorOverride?: TransactionButtonOverride; // Optional overrides for text and onClick handler in error state (default is resubmit txn) | ||
successOverride?: TransactionButtonOverride; // Optional overrides for text and onClick handler in success state (default is view txn on block explorer) | ||
}; | ||
|
||
export type TransactionContextType = { | ||
|
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.
I also feel like this might be cleaner if the top-level prop is not necessarily an override but the state itself, then we can set the default behavior using a default value:
e.g.
but more of a nit
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.
then we can eliminate lines 55-87 (around 30~ LOC)
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.
the issue is the default success state can be either
showCallsStatus({ id: transactionId });
orwindow.open(...)
depending on wallet type