-
Notifications
You must be signed in to change notification settings - Fork 31
feat(walletkit): update docs for version 0.0.4 #1697
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: main
Are you sure you want to change the base?
Conversation
|
Skipping AI review because this PR is from a fork. A maintainer can start the review by commenting /review in this PR. |
|
/review |
| networks: { | ||
| [CHAIN.MAINNET]: { // "-239" - Production network | ||
| apiClient: { | ||
| url: 'https://toncenter.com', | ||
| key: 'your-api-key', | ||
| }, | ||
| }, | ||
| [CHAIN.TESTNET]: { // "-3" - Testing network | ||
| apiClient: { | ||
| url: 'https://testnet.toncenter.com', | ||
| key: 'your-api-key-testnet', | ||
| }, | ||
| }, |
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.
[HIGH] API key placeholders do not use required <ANGLE_CASE> format
In the networks configuration example, the mainnet and testnet Toncenter API keys are shown as 'your-api-key' and 'your-api-key-testnet' at lines 131–132 and 137–138. These implicit placeholders do not follow the mandated <ANGLE_CASE> placeholder format and are not defined in the surrounding text, which makes it unclear that the values must be replaced and what they represent. This ambiguity can lead to copy‑paste misuse and violates the [HIGH] placeholder rule for code examples.
Out‑of‑diff evidence: https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L559-L571.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| `TonWalletKit.getWallet()` expects a wallet identifier in the `<chainId>:<address>` format. Build it via `createWalletId()` or rely on `getWalletByAddressAndNetwork()`: | ||
|
|
||
| ```ts title="TypeScript" | ||
| const wallet: IWallet = kit.getWallet(walletAdapter.getAddress()); | ||
| console.log('TON wallet address:', wallet.getAddress()); | ||
| import { createWalletId, Network, type Wallet } from '@ton/walletkit'; | ||
|
|
||
| const walletId = createWalletId(Network.mainnet(), walletAdapter.getAddress()); | ||
| const wallet: Wallet | undefined = kit.getWallet(walletId); | ||
|
|
||
| if (wallet) { | ||
| console.log('TON wallet address:', wallet.getAddress()); | ||
| } | ||
| ``` |
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.
[HIGH] Placeholder format and definition for wallet identifier
The description of TonWalletKit.getWallet() introduces the placeholder format <chainId>:<address> without using the prescribed <ANGLE_CASE> placeholder style or defining the placeholders on first use. As written, <chainId> and <address> look like concrete identifiers rather than template tokens and are not explained, which conflicts with the style rule requiring <ANGLE_CASE> placeholders and explicit definitions to avoid copy/paste confusion. The example code below continues to work, but the prose format and lack of definitions can mislead readers about what values to substitute. Updating the placeholders and defining them inline aligns the docs with the style guide while preserving semantics.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
|
Thanks for the updates to Per-comment submission: 3 posted, 1 failed. Unposted inline comments (raw text):
|
| `kit.removeWallet()` accepts either a `walletId` or the adapter instance itself. Reuse the same identifier as in `getWallet()`, or pass the adapter you used when adding the wallet: | ||
|
|
||
| ```ts title="TypeScript" | ||
| await kit.removeWallet(wallet.getAddress()); | ||
| const walletId = createWalletId(Network.mainnet(), walletAdapter.getAddress()); | ||
| await kit.removeWallet(walletId); | ||
| // or | ||
| await kit.removeWallet(walletAdapter); | ||
| ``` |
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.
[HIGH] Second-person pronoun in removeWallet description
The updated description for kit.removeWallet() includes the phrase “the adapter you used when adding the wallet,” which directly addresses the reader via the second-person pronoun “you.” The documentation style guide forbids using “you/your” to refer to the reader in normative prose, so this sentence violates that rule even though the surrounding example is otherwise correct. The rest of the section already describes behavior objectively, so a neutral rephrasing can preserve meaning without reader-directed language. Adjusting the sentence to describe the adapter instance in a passive, impersonal way resolves the style violation.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
Towards #146