-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Revise payment tutorials, add/remove screenshots #3091
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
base: master
Are you sure you want to change the base?
Conversation
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.
PR looks pretty good, but there are a couple points I have that apply across all functions.
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 we also add a button to fund an existing account with additional XRP?
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 don't know of an easy way to do that using the faucet. If I want to add XRP to an account, I can just create another account and send some XRP. Accounts that run out of XRP should try sending smaller amounts when testing.
let net = getNet() | ||
const client = new xrpl.Client(net) | ||
await client.connect() | ||
let results = `\nConnected to ${net}.` |
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 results = `\nConnected to ${net}.` | |
let results += `===Getting Account===\nConnected to ${net}.` |
This suggestion applies to all instances that add to the result field. I think it's worth keeping a log of all actions that take place, so ensuring all results are +=
and not overridden with =
. It'd also like to see a more pronounced headers to identify the type of function that's being called to help differentiate info in the logs. So for an offer create transaction, it starts with something like ===Creating Offer===
and then has a line break for subsequent metadata.
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 made these changes.
docs/tutorials/javascript/send-payments/send-and-cash-checks.md
Outdated
Show resolved
Hide resolved
docs/tutorials/javascript/send-payments/create-trust-line-send-currency.md
Outdated
Show resolved
Hide resolved
Question about how we reference code snippets, should we not use this format now?
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
refactored code, new CSS, only one active account at a time, but radio button allows switching between two accounts in the active fields.