Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks for adding the WalletConnect documentation; in ecosystem/ton-connect/wallet-connect.mdx: I’ve left a couple of suggestions, so please apply the inline suggestions to align the example configuration structure and wording with the docs style.
novusnota
left a comment
There was a problem hiding this comment.
Splendid! Left some comments that can improve this further.
Co-authored-by: Novus Nota <68142933+novusnota@users.noreply.github.com>
Co-authored-by: Novus Nota <68142933+novusnota@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Novus Nota <68142933+novusnota@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughAdded WalletConnect support documentation to the TON Connect ecosystem section, including a new navigation entry in docs.json and a dedicated documentation page covering WalletConnect compatibility, integration steps, and platform limitations. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
ecosystem/ton-connect/wallet-connect.mdx (2)
41-54: Consider extracting configuration values for clarity.The example inlines configuration values directly in the function call. While this works, extracting these as top-level constants would make them easier to identify and modify, improving the example's maintainability. This aligns with documentation best practices for runnable examples.
♻️ Suggested refactor
import { initializeWalletConnect } from '@tonconnect/sdk'; import { UniversalConnector } from '@reown/appkit-universal-connector'; const PROJECT_ID = 'YOUR_PROJECT_ID'; const APP_METADATA = { name: 'My DApp', description: 'Example DApp', url: 'https://mydapp.com', icons: ['https://mydapp.com/icon.png'] }; initializeWalletConnect(UniversalConnector, { projectId: PROJECT_ID, metadata: APP_METADATA });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ecosystem/ton-connect/wallet-connect.mdx` around lines 41 - 54, Extract the inline config values into top-level constants so the example is clearer and easier to modify: create constants (e.g., PROJECT_ID and APP_METADATA) and assign the projectId string and metadata object respectively, then call initializeWalletConnect(UniversalConnector, { projectId: PROJECT_ID, metadata: APP_METADATA }); this keeps the original functions/symbols (initializeWalletConnect, UniversalConnector) unchanged while improving readability and maintainability.
32-32: Clarify the conditional statement.The phrase "If required" is ambiguous. It's unclear whether this refers to requiring WalletConnect functionality or requiring wallet connection in general. Consider being more explicit.
📝 Suggested clarification
-If required, use [TON Connect](/ecosystem/ton-connect/overview) instead. +For wallet connection functionality in these environments, use [TON Connect](/ecosystem/ton-connect/overview) instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ecosystem/ton-connect/wallet-connect.mdx` at line 32, Replace the ambiguous sentence "If required, use [TON Connect](/ecosystem/ton-connect/overview) instead." with a specific condition that explains when to prefer TON Connect (e.g., "If your app needs a full TON-native wallet integration or features not supported by WalletConnect, use [TON Connect](/ecosystem/ton-connect/overview) instead.") — update the text near that exact sentence in wallet-connect.mdx so the reader clearly understands whether the requirement is about WalletConnect limitations, specific features, or full TON-native integration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ecosystem/ton-connect/wallet-connect.mdx`:
- Around line 34-54: Add a brief changelog/version note above the "How to
integrate" section stating which TON Connect SDK version introduced
WalletConnect support and include a link to the SDK changeset; update the
documentation near the initializeWalletConnect/UniversalConnector example to
read something like "WalletConnect support was added in TON Connect SDK version
X.Y.Z — see [changelog](URL) for details" so readers know version compatibility
and can follow the linked changeset.
- Line 30: Edit the sentence "Due to Telegram policies, WalletConnect is not
supported in applications within, such as [Telegram Mini
Apps](/ecosystem/tma/overview)." by removing the unnecessary word "within" so it
reads "Due to Telegram policies, WalletConnect is not supported in applications,
such as [Telegram Mini Apps](/ecosystem/tma/overview)." ensuring punctuation and
spacing remain correct.
- Line 25: The WalletKit link currently points to
'/ecosystem/ton-connect/walletkit/overview' which triggers a redirect; update
the link target in the text that reads "Most applications and wallets should
continue to rely on [TON Connect](/ecosystem/ton-connect/overview) and
[WalletKit](/ecosystem/ton-connect/walletkit/overview)" to use the direct path
'/ecosystem/walletkit/overview' so the WalletKit reference avoids the redirect.
---
Nitpick comments:
In `@ecosystem/ton-connect/wallet-connect.mdx`:
- Around line 41-54: Extract the inline config values into top-level constants
so the example is clearer and easier to modify: create constants (e.g.,
PROJECT_ID and APP_METADATA) and assign the projectId string and metadata object
respectively, then call initializeWalletConnect(UniversalConnector, { projectId:
PROJECT_ID, metadata: APP_METADATA }); this keeps the original functions/symbols
(initializeWalletConnect, UniversalConnector) unchanged while improving
readability and maintainability.
- Line 32: Replace the ambiguous sentence "If required, use [TON
Connect](/ecosystem/ton-connect/overview) instead." with a specific condition
that explains when to prefer TON Connect (e.g., "If your app needs a full
TON-native wallet integration or features not supported by WalletConnect, use
[TON Connect](/ecosystem/ton-connect/overview) instead.") — update the text near
that exact sentence in wallet-connect.mdx so the reader clearly understands
whether the requirement is about WalletConnect limitations, specific features,
or full TON-native integration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dec61925-f2ae-4c8e-b1a1-a38530aebeb3
⛔ Files ignored due to path filters (1)
resources/images/ton-connect/wallet-connect.pngis excluded by!**/*.png
📒 Files selected for processing (2)
docs.jsonecosystem/ton-connect/wallet-connect.mdx
closes #1698
Summary by CodeRabbit