[SDK] add hiddenWallets prop #7181
Conversation
🦋 Changeset detectedLatest commit: ddcc0f3 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 |
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. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughA new optional Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ConnectButton
participant ConnectModal
participant ConnectModalContent
App->>ConnectButton: Render with hiddenWallets
ConnectButton->>ConnectModal: Pass hiddenWallets as prop
ConnectModal->>ConnectModalContent: Pass hiddenWallets as walletIdsToHide
ConnectModalContent-->>App: Renders wallet list (excluding hiddenWallets)
sequenceDiagram
participant App
participant useConnectModal
participant ConnectModal
App->>useConnectModal: Call with hiddenWallets
useConnectModal->>ConnectModal: Render with hiddenWallets
ConnectModal-->>App: Renders wallet list (excluding hiddenWallets)
sequenceDiagram
participant App
participant ConnectEmbed
participant ConnectModalContent
App->>ConnectEmbed: Render with hiddenWallets
ConnectEmbed->>ConnectModalContent: Pass hiddenWallets as walletIdsToHide
ConnectModalContent-->>App: Renders wallet list (excluding hiddenWallets)
✨ 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: 1
🧹 Nitpick comments (1)
packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx (1)
303-304: Consider the precedence logic for hiddenWallets.The current logic uses
||operator which gives precedence toprops.hiddenWalletsoverprops.detailsModal?.hiddenWallets. This means if both are provided, onlyprops.hiddenWalletswill be used.Consider whether this is the intended behavior or if the arrays should be merged:
If merging is desired, consider this approach:
- const hiddenWallets = - props.hiddenWallets || props.detailsModal?.hiddenWallets; + const hiddenWallets = [ + ...(props.hiddenWallets || []), + ...(props.detailsModal?.hiddenWallets || []) + ];Or if precedence is intended, consider adding a comment to clarify the behavior.
📜 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(1 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 (3)
packages/thirdweb/src/react/core/hooks/connection/ConnectEmbedProps.ts (1)
packages/thirdweb/src/exports/wallets.ts (1)
WalletId(54-54)
packages/thirdweb/src/react/web/ui/ConnectWallet/useConnectModal.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: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (13)
.changeset/hidden-wallets-prop.md (1)
1-6: LGTM! Well-documented changeset.The changeset properly documents the new
hiddenWalletsprop addition as a minor version change, which is appropriate for a new feature. The description clearly explains the functionality and lists the affected components.packages/thirdweb/src/react/core/hooks/connection/ConnectButtonProps.ts (1)
984-987: LGTM! Well-defined type addition.The
hiddenWalletsprop is properly typed as an optionalWalletId[]array with clear documentation. The type import is correctly included and the JSDoc comment clearly explains the functionality.packages/thirdweb/src/react/core/hooks/connection/ConnectEmbedProps.ts (2)
6-6: LGTM! Proper type import.The
WalletIdimport is correctly added to support the newhiddenWalletsprop.
276-279: LGTM! Consistent type definition.The
hiddenWalletsprop is properly defined with clear documentation that matches theConnectButtonPropsimplementation. The type and documentation are consistent across components, which maintains a cohesive API.packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectModal.tsx (3)
7-7: LGTM! Clean import addition.The import of
WalletIdtype is correctly added to support the newhiddenWalletsfunctionality.
46-46: LGTM! Type definition follows established patterns.The
hiddenWalletsproperty is correctly defined with the appropriate typeWalletId[] | undefined, maintaining consistency with other optional properties in the interface.
156-156: LGTM! Proper prop forwarding.The change from passing
undefinedtoprops.hiddenWalletscorrectly forwards the new prop to theConnectModalContentcomponent aswalletIdsToHide.packages/thirdweb/src/react/web/ui/ConnectWallet/useConnectModal.tsx (3)
8-8: LGTM! Consistent import addition.The
WalletIdtype import maintains consistency with the other files in this PR.
146-146: LGTM! Correct prop forwarding.The
hiddenWalletsprop is properly passed from the hook options to the underlyingConnectModalcomponent.
369-372: LGTM! Excellent documentation and type definition.The new
hiddenWalletsproperty is well-documented with a clear description of its purpose. The type definition is consistent with the other files in this PR.packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx (3)
398-398: LGTM! Correct prop forwarding to ConnectModal.The
hiddenWalletsprop is properly passed to theConnectModalcomponent.
591-591: LGTM! hiddenWallets added to connectOptions.The
hiddenWalletsproperty is correctly included in theconnectOptionsobject passed toConnectedWalletDetails.
563-566:❓ Verification inconclusive
Review the prop spreading behavior.
The current implementation spreads
props.detailsModalfirst and then addshiddenWallets. This means ifprops.detailsModalcontains ahiddenWalletsproperty, it will be overridden by the computedhiddenWalletsvalue.Verify that this override behavior is intentional:
This behavior might be correct if the computed
hiddenWalletsshould take precedence over the one indetailsModal.
🏁 Script executed:
#!/bin/bash # Check if detailsModal props include hiddenWallets in type definitions ast-grep --pattern 'detailsModal?: { $$$ hiddenWallets$_ $$$ }'Length of output: 71
🏁 Script executed:
#!/bin/bash # Locate where ConnectButtonProps is defined and check detailsModal usage rg "ConnectButtonProps" -n packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx # Inspect all occurrences of detailsModal within this file rg "detailsModal" -n packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsxLength of output: 812
🏁 Script executed:
#!/bin/bash # List files in the connection hooks directory ls packages/thirdweb/src/core/hooks/connection # Search for ConnectButtonProps type definition and detailsModal within it rg -n "ConnectButtonProps" -n packages/thirdweb/src/core/hooks/connection/ConnectButtonProps* rg -n "detailsModal" -n packages/thirdweb/src/core/hooks/connection/ConnectButtonProps*Length of output: 533
#!/bin/bash # Locate the connection hooks directory under src/react echo "Searching for 'connection' directories under src/react..." find packages/thirdweb/src/react -type d | rg "/connection$" || echo "No 'connection' dirs found under src/react" # Find the ConnectButtonProps file anywhere in the repo echo "Locating ConnectButtonProps file..." props_file=$(find packages/thirdweb -type f | rg "ConnectButtonProps\.(js|ts)x?$") if [ -z "$props_file" ]; then echo "ConnectButtonProps file not found" exit 1 fi echo "Found ConnectButtonProps at: $props_file" # Show first 200 lines of that file echo "Printing first 200 lines of $props_file..." sed -n '1,200p' "$props_file" # Search for detailsModal in that file echo "Searching for detailsModal in $props_file..." rg -n "detailsModal" "$props_file" || echo "No detailsModal property in ConnectButtonProps"
#!/bin/bash # Find all ConnectButtonProps definition files in the repo files=$(find packages/thirdweb -type f | rg -i "ConnectButtonProps\.(ts|tsx|js|jsx)$") echo "ConnectButtonProps files found:" echo "$files" # For each file, print surrounding context of the detailsModal property for file in $files; do echo -e "\n---- Context in $file ----" rg -n "detailsModal" -A3 -B3 "$file" || echo "No detailsModal property in $file" done
#!/bin/bash # Search for ConnectButtonProps definition under src/react echo "Locating ConnectButtonProps files under src/react..." paths=$(find packages/thirdweb/src/react -type f | rg "ConnectButtonProps\.(ts|tsx|js|jsx)$") if [ -z "$paths" ]; then echo "No ConnectButtonProps files found under src/react" exit 1 fi echo "$paths" # For each found file, display header, the top of the file, and any detailsModal occurrences for file in $paths; do echo -e "\n=== File: $file ===" echo "First 200 lines of $file:" sed -n '1,200p' "$file" echo -e "\nOccurrences of detailsModal in $file:" rg -n "detailsModal" -A3 -B3 "$file" || echo "No detailsModal property found in $file" done
Confirm hiddenWallets override in detailsModal
- File:
packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButton.tsx(lines 563–566)- Spreading
...props.detailsModalbeforehiddenWalletswill always override anyhiddenWalletspassed in via props.If you intend to let consumers control which wallets to hide, swap the spread order:
detailsModal={{ - ...props.detailsModal, - hiddenWallets, + hiddenWallets, + ...props.detailsModal, }}
| wallets={props.wallets} | ||
| modalHeader={undefined} | ||
| walletIdsToHide={undefined} | ||
| walletIdsToHide={props.hiddenWallets} |
There was a problem hiding this comment.
Fix missing hiddenWallets prop in ConnectEmbedContent interface.
The code is accessing props.hiddenWallets but this prop is not defined in the ConnectEmbedContent props interface (lines 310-347) and is not being passed from the parent ConnectEmbed component (lines 278-298).
Apply these fixes:
- Add
hiddenWalletsto theConnectEmbedContentprops interface:
const ConnectEmbedContent = (props: {
modalSize?: "compact" | "wide";
className?: string;
style?: React.CSSProperties;
+ hiddenWallets?: WalletId[];
// ---
accountAbstraction: SmartWalletOptions | undefined;- Pass the prop from parent
ConnectEmbedcomponent around line 298:
<WalletUIStatesProvider theme={props.theme} isOpen={true}>
<ConnectEmbedContent
auth={props.auth}
accountAbstraction={props.accountAbstraction}
+ hiddenWallets={props.hiddenWallets}
chain={preferredChain}🤖 Prompt for AI Agents
In packages/thirdweb/src/react/web/ui/ConnectWallet/Modal/ConnectEmbed.tsx
around lines 278-298 and 310-347, the prop hiddenWallets is used in
ConnectEmbedContent but is missing from its props interface and not passed from
the parent ConnectEmbed component. Fix this by adding hiddenWallets to the
ConnectEmbedContent props interface definition and then pass hiddenWallets as a
prop from the ConnectEmbed component to ConnectEmbedContent around line 298.
Fixes TOOL-4607
Summary
Checklist
PR-Codex overview
This PR introduces a new
hiddenWalletsprop across several components to allow specific wallets to be excluded from the wallet selection list.Detailed summary
hiddenWalletsprop toConnectEmbed,ConnectButton, anduseConnectModal.ConnectEmbedPropsandConnectButtonPropsto includehiddenWallets.ConnectModalandConnectButtonto utilize thehiddenWalletsprop.hiddenWalletsin relevant files.Summary by CodeRabbit