-
Notifications
You must be signed in to change notification settings - Fork 125
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
add TS examples for equivalent SDK methods #177
Conversation
Preview is available here: |
docs/building-apps/wallet/sep24.mdx
Outdated
accountAddress: accountKp.publicKey(), | ||
assetCode, | ||
authToken, | ||
fundsAccountAddress: recipientAccount |
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 believe fundsAccountAddress
is the equivalent key for destinationAccount
above, but lmk if I'm wrong
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 think so. It should be optional, is it like this right now? (If not, making it optional is part of WAL-889 IIRC)
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.
fundsAccountAddress
is optional. It gets set to accountAddress
if it isn't explicitly set and then sent to the /interactive
endpoint as account
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.
Got it. Small thing: for withdrawal it should be originAccount
Preview is available here: |
1 similar comment
Preview is available here: |
Preview is available here: |
Preview is available here: |
docs/building-apps/wallet/intro.mdx
Outdated
@@ -60,6 +66,19 @@ val walletCustom = Wallet( | |||
) | |||
``` | |||
|
|||
```typescript | |||
const customClient: AxiosInstance = axios.create({ | |||
baseURL: "https://some-url.com/api", |
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.
Why do we even need to set baseUrl? I don't think it's good to show it in the example
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.
You don't necessarily, it's just an example of the params axios.create
takes. I can rm
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.
Think it's better to remove it, cause in the SDK it doesn't make any sense to do so
docs/building-apps/wallet/intro.mdx
Outdated
timeout: 1000, | ||
headers: { "X-Custom-Header": "foobar" }, | ||
}); | ||
let appConfig = new ApplicationConfiguration(DefaultSigner, customClient); |
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.
Shouldn't DefaultSigner be there by default? (I.e. you don't need to set it manually)
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.
You don't, I can just pass null
here
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 you just omit it or it's not possible?
docs/building-apps/wallet/intro.mdx
Outdated
@@ -60,6 +66,19 @@ val walletCustom = Wallet( | |||
) | |||
``` | |||
|
|||
```typescript |
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 think all this code belongs to the "Configuring the Client" section
docs/building-apps/wallet/sep24.mdx
Outdated
@@ -28,6 +28,10 @@ Let's start with getting an instance of `Interactive` class, responsible for all | |||
val sep24 = anchor.sep24() | |||
``` | |||
|
|||
```typescript | |||
let sep24 = await anchor.getInfo(); |
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.
anchor.interactive()
?
@@ -143,6 +189,18 @@ suspend fun depositDifferentAccount(): InteractiveFlowResponse { | |||
} | |||
``` | |||
|
|||
```typescript | |||
const recipientAccount = "G..."; |
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 you add TODO here to modify code when memo is supported?
docs/building-apps/wallet/sep24.mdx
Outdated
@@ -236,6 +324,13 @@ It's also possible to fetch transaction by the asset | |||
val transactions = sep24.getTransactionsForAsset(asset, token) | |||
``` | |||
|
|||
```typescript | |||
const transactions = await anchor.getHistory({ |
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.
getHistory
should be deprecated... It has some extra filtering logic that shouldn't be there.
getTransactionsForAsset
is more flexible than getHistory
Preview is available here: |
Preview is available here: |
1 similar comment
Preview is available here: |
Preview is available here: |
No description provided.