[SDK] fix hiddenWallets prop #7183
Conversation
🦋 Changeset detectedLatest commit: a925fc1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ConnectButton
participant ConnectModal
participant ConnectModalContent
App->>ConnectButton: Render with hiddenWallets
ConnectButton->>ConnectModal: Pass hiddenWallets
ConnectModal->>ConnectModalContent: Pass walletIdsToHide = hiddenWallets
ConnectModalContent-->>App: Render wallet list (filtered)
sequenceDiagram
participant App
participant ConnectEmbed
participant ConnectEmbedContent
participant ConnectModalContent
App->>ConnectEmbed: Render with hiddenWallets
ConnectEmbed->>ConnectEmbedContent: Pass hiddenWallets
ConnectEmbedContent->>ConnectModalContent: Pass walletIdsToHide = hiddenWallets
ConnectModalContent-->>App: Render wallet list (filtered)
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx (1)
303-304: LGTM! Good unification of hiddenWallets sources.The logic correctly creates a unified
hiddenWalletsvariable from multiple potential sources, giving priority to the direct prop over the details modal configuration.Note: The
||operator will treat an empty array[]as falsy, falling back toprops.detailsModal?.hiddenWallets. If this behavior is unintended and an empty array should take precedence, consider using nullish coalescing:-const hiddenWallets = props.hiddenWallets || props.detailsModal?.hiddenWallets; +const hiddenWallets = props.hiddenWallets ?? props.detailsModal?.hiddenWallets;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/hidden-wallets-prop.md(1 hunks)packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts(1 hunks)packages/thirdweb/src/react/core/hooks/connection/ConnectEmbedProps.ts(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx(4 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx(4 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModal.tsx(3 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/useConnectModal.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/thirdweb/src/react/web/ui/ConnectWallet/useConnectModal.tsx (1)
packages/thirdweb/src/exports/wallets.ts (1)
WalletId(54-54)
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModal.tsx (1)
packages/thirdweb/src/exports/wallets.ts (1)
WalletId(54-54)
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx (1)
packages/thirdweb/src/exports/wallets.ts (1)
WalletId(54-54)
packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts (1)
packages/thirdweb/src/exports/wallets.ts (1)
WalletId(54-54)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts (2)
15-15: LGTM: Proper import of WalletId typeThe import statement correctly references the WalletId type from the wallet-types module, which is consistent with the other files in this PR.
984-987: LGTM: Clean implementation of hiddenWallets propThe addition of the
hiddenWalletsproperty is well-implemented:
- Properly typed as optional
WalletId[]- Clear and descriptive JSDoc documentation
- Maintains backward compatibility
- Follows the existing code style patterns
packages/thirdweb/src/react/core/hooks/connection/ConnectEmbedProps.ts (2)
6-6: LGTM: Consistent import patternThe WalletId import follows the same pattern as used in ConnectButtonProps.ts, maintaining consistency across the codebase.
276-279: LGTM: Consistent hiddenWallets implementationThe
hiddenWalletsproperty implementation is identical to the one in ConnectButtonProps, which ensures consistency across components. The documentation clearly explains the purpose and usage..changeset/hidden-wallets-prop.md (1)
1-6: LGTM: Properly formatted changesetThe changeset correctly:
- Marks this as a "patch" change (appropriate for adding optional props)
- Provides a clear description of the new feature
- Follows standard changeset formatting conventions
packages/thirdweb/src/react/web/ui/ConnectWallet/useConnectModal.tsx (3)
8-8: LGTM: Consistent import statementThe WalletId import maintains consistency with the other files in this PR.
369-372: LGTM: Well-documented hiddenWallets propThe
hiddenWalletsproperty follows the same pattern as the other files, maintaining consistency across the codebase with clear documentation.
146-146: LGTM: Proper prop propagationThe
hiddenWalletsprop is correctly passed down to theConnectModalcomponent, ensuring the feature works throughout the component hierarchy. This demonstrates the fix mentioned in the PR objectives where the prop needs to be passed down properly.packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx (1)
8-8: LGTM! Clean implementation of hiddenWallets prop.The implementation correctly:
- Imports the
WalletIdtype- Adds the optional
hiddenWalletsprop to theConnectEmbedContentinterface- Passes the prop down the component hierarchy
- Maps it to
walletIdsToHidefor theConnectModalContentcomponentThe typing and prop propagation are consistent and maintain backwards compatibility.
Also applies to: 342-342, 299-299, 421-421
packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModal.tsx (1)
7-7: LGTM! Consistent implementation of hiddenWallets in modal options.The changes properly:
- Import the
WalletIdtype for consistency- Extend the
ConnectModalOptionstype with the optionalhiddenWalletsproperty- Pass the prop to
ConnectModalContentaswalletIdsToHideThe implementation follows the same pattern as other components in the PR.
Also applies to: 46-46, 156-156
packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx (1)
398-398: Excellent prop propagation throughout the component hierarchy.The
hiddenWalletsprop is correctly passed to all relevant components:
ConnectModalfor wallet selection filteringConnectedWalletDetailsvia thedetailsModalprop- The
connectOptionsobject for consistencyThis ensures the hidden wallets configuration is consistently applied across all parts of the connect button UI.
Also applies to: 563-566, 591-591
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (53.84%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7183 +/- ##
=======================================
Coverage 55.66% 55.66%
=======================================
Files 904 904
Lines 58391 58400 +9
Branches 4114 4116 +2
=======================================
+ Hits 32504 32510 +6
- Misses 25781 25784 +3
Partials 106 106
🚀 New features to boost your workflow:
|
joaquim-verges
deleted the
yfkmnp-codex/add-hiddenwallets-prop-to-filter-wallets
branch
Fixes TOOL-4607
Summary
Checklist
PR-Codex overview
This PR introduces a new
hiddenWalletsprop across several components to allow specific wallets to be hidden from the wallet selection list, enhancing user experience by decluttering the options presented during wallet connection.Detailed summary
hiddenWalletsprop toConnectEmbed,ConnectButton, anduseConnectModal.ConnectEmbedPropsandConnectButtonPropsto includehiddenWallets?: WalletId[].ConnectModalandConnectEmbedto utilize thehiddenWalletsprop for wallet selection.hiddenWalletsprop appropriately.Summary by CodeRabbit