[SDK] Feature: Hide the TransactionWidget contract label if unknown #7642
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: e86968a The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 ↗︎
|
WalkthroughA changeset entry was added documenting a UI update in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TransactionWidget
participant UI
User->>TransactionWidget: Initiate transaction
TransactionWidget->>UI: Render "Contract Info" section
alt contractName is valid and known
UI-->>User: Display contract name
else contractName is "UnknownContract", "Unknown Contract", or undefined
UI-->>User: Hide contract name section
end
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ 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 (
|
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. |
gregfromstl
force-pushed
the
greg/hide-unknown-contract-label
branch
from
653f049 to
e86968a
Compare
gregfromstl
added
the
merge-queue
Merge activity
|
joaquim-verges
approved these changes
Jul 17, 2025
![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/Bridge/TransactionPayment.tsx (1)
215-236: Consider using a more maintainable approach for the conditional rendering.The conditional logic correctly implements the feature requirement, but it could be more maintainable and readable.
Consider using an array of invalid contract names for better maintainability:
+const invalidContractNames = ["UnknownContract", "Unknown Contract"]; + {/* Contract Info */} -{contractName !== "UnknownContract" && - contractName !== undefined && - contractName !== "Unknown Contract" && ( +{contractName && !invalidContractNames.includes(contractName) && ( <> <Container flex="row" style={{ alignItems: "center", justifyContent: "space-between", }} > <Text color="secondaryText" size="sm"> Contract </Text> <Text color="primaryText" size="sm"> {contractName} </Text> </Container> <Spacer y="xs" /> </> )}This approach makes it easier to add new invalid contract names in the future and is more readable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/whole-bottles-wait.md(1 hunks)packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.488Z
Learning: Surface breaking changes prominently in PR descriptions
.changeset/whole-bottles-wait.md (3)
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.488Z
Learning: Surface breaking changes prominently in PR descriptions
Learnt from: MananTank
PR: thirdweb-dev/js#7332
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/public-pages/nft/overview/nft-drop-claim.tsx:82-90
Timestamp: 2025-06-13T13:03:41.732Z
Learning: The thirdweb `contract` object is serializable and can safely be used in contexts (e.g., React-Query keys) that require serializable values.
Learnt from: MananTank
PR: thirdweb-dev/js#7227
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/modules/components/OpenEditionMetadata.tsx:26-26
Timestamp: 2025-05-30T17:14:25.332Z
Learning: The ModuleCardUIProps interface already includes a client prop of type ThirdwebClient, so when components use `Omit<ModuleCardUIProps, "children" | "updateButton">`, they inherit the client prop without needing to add it explicitly.
packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx (11)
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/extensions/**/*.{ts,tsx} : Auto-generated contracts from ABI definitions in extensions
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Unified `Wallet` and `Account` interfaces in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Smart wallets with account abstraction in wallet architecture
Learnt from: MananTank
PR: thirdweb-dev/js#7152
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/tokens/shared-page.tsx:41-48
Timestamp: 2025-05-26T16:28:50.772Z
Learning: The `projectMeta` prop is not required for the server-rendered `ContractTokensPage` component in the tokens shared page, unlike some other shared pages where it's needed for consistency.
Learnt from: MananTank
PR: thirdweb-dev/js#7434
File: apps/dashboard/src/app/(app)/team/~/~/contract/[chain]/[contractAddress]/components/project-selector.tsx:62-76
Timestamp: 2025-06-24T21:38:03.155Z
Learning: In the project-selector.tsx component for contract imports, the addToProject.mutate() call is intentionally not awaited (fire-and-forget pattern) to allow immediate navigation to the contract page while the import happens in the background. This is a deliberate design choice to prioritize user experience.
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Support EIP-1193, EIP-5792, EIP-7702 standards in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-06-30T10:26:04.389Z
Learning: Applies to dashboard/**/components/*.client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/wallets/**/*.{ts,tsx} : Support for in-app wallets (social/email login) in wallet architecture
Learnt from: CR
PR: thirdweb-dev/js#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T10:25:29.489Z
Learning: Applies to src/exports/react.native.ts : React Native specific exports are in `src/exports/react.native.ts`
Learnt from: MananTank
PR: thirdweb-dev/js#7356
File: apps/nebula/src/app/not-found.tsx:1-1
Timestamp: 2025-06-17T18:30:52.976Z
Learning: In the thirdweb/js project, the React namespace is available for type annotations (like React.FC) without needing to explicitly import React. This is project-specific configuration that differs from typical TypeScript/React setups.
Learnt from: jnsdls
PR: thirdweb-dev/js#7188
File: apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/accounts/components/accounts-count.tsx:15-15
Timestamp: 2025-05-29T00:46:09.063Z
Learning: In the accounts component at apps/dashboard/src/app/(app)/(dashboard)/(chain)/[chain_id]/[contractAddress]/accounts/components/accounts-count.tsx, the 3-column grid layout (md:grid-cols-3) is intentionally maintained even when rendering only one StatCard, as part of the design structure for this component.
🧬 Code Graph Analysis (1)
packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx (1)
packages/thirdweb/src/react/web/ui/components/text.tsx (1)
Text(17-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx (1)
109-110: Clarify the contract name fallback logic.The fallback value "Unknown Contract" is immediately filtered out by the conditional rendering below, which means the contract info section will never display when contract metadata is missing.
This could be intentional behavior, but please verify:
- Is it correct that the contract section should be completely hidden when no contract metadata is available?
- The
contractName !== undefinedcheck in the conditional (line 216) is redundant since the fallback ensures contractName is never undefined.Consider either:
- Removing the undefined check:
contractName !== "UnknownContract" && contractName !== "Unknown Contract"- Or changing the fallback to
undefinedif you want to preserve the undefined check:transactionDataQuery.data?.contractMetadata?.name.changeset/whole-bottles-wait.md (1)
1-6: Changeset correctly documents the UI improvement.The changeset appropriately categorizes this as a patch-level change and provides a clear, concise description of the modification.
The change improves user experience by hiding placeholder contract labels. Consider if any integration tests or accessibility features might be affected by this UI change, but this appears to be a non-breaking improvement.
size-limit report 📦
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) 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 #7642 +/- ##
==========================================
- Coverage 56.46% 56.43% -0.03%
==========================================
Files 908 908
Lines 58159 58193 +34
Branches 4225 4227 +2
==========================================
+ Hits 32840 32844 +4
- Misses 25211 25239 +28
- Partials 108 110 +2
🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR-Codex overview
This PR updates the
TransactionWidgetcomponent to hide the "UnknownContract" label when the contract name is either "UnknownContract", "Unknown Contract", or undefined.Detailed summary
TransactionPayment.tsx.contractNameis "UnknownContract", "Unknown Contract", or undefined.Summary by CodeRabbit