-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update screens for deposit and withdrawal flows #821
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for acre-dapp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for acre-dapp-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
eec8832
to
3952c51
Compare
The status will tell us when the wallet window opens and when the transaction is completed.
The "Skeleton" loading screen from both Deposit and Withdraw flows is no longer needed. The new flow adds two steps: - "Opening your wallet for signature" Screen - "Awaiting Transaction" Screen
d7b5be7
to
9e6fbe6
Compare
We want to know when the transaction data are built. We add callback that is triggered after building data.
730ce12
to
51a3965
Compare
@@ -24,6 +25,7 @@ export function useDepositBTCTransaction( | |||
const [transactionHash, setTransactionHash] = useState<string | undefined>( | |||
undefined, | |||
) | |||
const [inProgress, setInProgress] = useState<boolean>(false) |
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.
Maybe it's a good opportunity to refactor this hook and use useMutation
hook from @tanstack/react-query
?
onDepositBTCSuccess, | ||
onDepositBTCError, | ||
) | ||
const { sendBitcoinTransaction, transactionHash, inProgress } = |
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 would rename inProgress
to isInProgress
to keep consistency with other boolean values. Or if you agree to refactor the useDepositBTCTransaction
hook as I mentioned above, we can use status
.
}: { | ||
step: WalletInteractionStep | ||
}) { | ||
const type = useActionFlowType() |
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.
const type = useActionFlowType() | |
const action = useActionFlowType() |
or
const type = useActionFlowType() | |
const actionType = useActionFlowType() |
@@ -25,6 +26,8 @@ export default class OrangeKitTbtcRedeemerProxy implements TbtcRedeemerProxy { | |||
|
|||
#sharesAmount: bigint | |||
|
|||
#builtDataStepCallback?: BuiltDataStepCallback |
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.
Can't we use onSignMessageStepCallback
?
@@ -25,6 +26,8 @@ export default class OrangeKitTbtcRedeemerProxy implements TbtcRedeemerProxy { | |||
|
|||
#sharesAmount: bigint | |||
|
|||
#builtDataStepCallback?: BuiltDataStepCallback |
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.
Maybe we should rename to 🤔 :
#builtDataStepCallback?: BuiltDataStepCallback | |
#buildDataStepCallback?: BuiltDataStepCallback |
or
#builtDataStepCallback?: BuiltDataStepCallback | |
#dataBuiltStepCallback?: BuiltDataStepCallback |
@@ -7,6 +7,7 @@ import { OrangeKitSdk } from "@orangekit/sdk" | |||
import { AcreContracts } from "./contracts" | |||
import { BitcoinProvider } from "./bitcoin" | |||
|
|||
export type BuiltDataStepCallback = (safeTxData: Hex) => Promise<void> |
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.
Same as above
isIndeterminate | ||
{...progressProps} | ||
/> | ||
<Image src={connector?.icon} p={2} bg="black" {...ICON_STYLES} /> |
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.
Let's add alt="Connector icon
.
Ref #811
Closes #814
Closes #813
This PR updates screens for deposit and withdrawal flows. The "Skeleton" loading screen from both Deposit and Withdraw flows is no longer needed. The new flow adds two steps:
Flow:
Screen.Recording.2024-10-31.at.13.12.28.mov