[MNY-353] SDK: TransactionWidget UI improvements #8607
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 01004e8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
WalkthroughRedesigned TransactionWidget loading UI with Skeleton and Spinner components, spacing/typography tweaks, and reorganized contract/address/network layout. Storybook stories added/renamed for ETH transfer variants. Fixture updated to use Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
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. This stack of pull requests is managed by Graphite. Learn more about stacking. |
size-limit report 📦
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8607 +/- ##
==========================================
+ Coverage 53.19% 53.21% +0.01%
==========================================
Files 922 922
Lines 61480 61457 -23
Branches 4032 4041 +9
==========================================
- Hits 32706 32703 -3
+ Misses 28676 28656 -20
Partials 98 98
🚀 New features to boost your workflow:
|
Merge activity
|
<!--
## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes"
If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000):
## Notes for the reviewer
Anything important to call out? Be sure to also clarify these in your comments.
## How to test
Unit tests, playground, etc.
-->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR focuses on UI improvements for the `TransactionWidget` in the `thirdweb` package, enhancing user experience with better layout, spacing, and loading indicators.
### Detailed summary
- Changed the `chain` in `simpleBuyRequest` from `baseSepolia` to `base`.
- Updated the story names and added new stories for `TransactionWidget`.
- Improved layout and spacing in `TransactionPayment`.
- Replaced static loading elements with `Skeleton` components for better loading states.
- Enhanced button styles and added a loading spinner.
- Refined contract info and address display for better clarity.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **UI Improvements**
* Improved TransactionWidget loading states with clearer visual spinner components
* Enhanced visual presentation and layout of transaction details, addresses, and network information
* Refined typography, spacing, and styling throughout the widget for better readability and consistency
<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
bot
force-pushed
the
mny-353
branch
from
60285cd to
01004e8
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx (1)
252-269: Fix the contract name condition - string mismatch bug.Line 252 checks for
contractName !== "UnknownContract"(no space), but Line 134 sets the default to"Unknown Contract"(with space). This mismatch means the condition will never work as intended.The condition can be simplified since the default is already set on Line 134:
🔎 Proposed fix
- {contractName !== "UnknownContract" && - contractName !== undefined && - contractName !== "Unknown Contract" && ( + {contractName !== "Unknown Contract" && ( <Container flex="row"
🤖 Fix all issues with AI Agents
In @packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx:
- Around line 282-294: The anchor uses a forced cast transaction.to as string
even though PreparedTransaction.to is optional; update the rendering in
TransactionPayment to handle a possibly undefined transaction.to by either (a)
conditionally rendering the entire <a> element only when transaction.to is
defined (check transaction.to and call shortenAddress(transaction.to) safely),
or (b) if business logic guarantees presence, replace the cast with a non-null
assertion transaction.to! and document that guarantee; reference the transaction
object and shortenAddress call to locate the change and ensure the href build
and display use a null-safe value.
🧹 Nitpick comments (1)
packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx (1)
466-501: Consider removing the unused theme parameter.The
SkeletonHeadercomponent receives athemeparameter (renamed to_props) but doesn't use it. While the underscore prefix indicates it's intentionally unused, you could simplify by removing the parameter entirely since the component now uses theSkeletoncomponent directly.The
SkeletonRowcomponent is well-structured and cleanly implemented.🔎 Proposed refactor
-const SkeletonHeader = (_props: { theme: Theme }) => ( +const SkeletonHeader = () => ( <ContainerAnd update the call site on Line 163:
- <SkeletonHeader theme={theme} /> + <SkeletonHeader />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.changeset/easy-teams-agree.mdpackages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsxpackages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsxpackages/thirdweb/src/stories/Bridge/fixtures.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/thirdweb/src/stories/Bridge/fixtures.ts
- .changeset/easy-teams-agree.md
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsxpackages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsxpackages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx
**/*.stories.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Add Storybook stories (
*.stories.tsx) alongside new UI components for documentation
Files:
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsxpackages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsxpackages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx
**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For new UI components, add Storybook stories (*.stories.tsx) alongside the code
Files:
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsx
🧬 Code graph analysis (1)
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsx (1)
packages/thirdweb/src/stories/Bridge/fixtures.ts (1)
TRANSACTION_UI_OPTIONS(799-839)
⏰ 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). (7)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: Size
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
packages/thirdweb/src/stories/Bridge/Transaction/TransactionWidget.stories.tsx (1)
25-44: LGTM! Well-structured story variants.The progressive story variants (BaseEthTransfer → BaseEthTransferWithDescription → BaseEthTransferWithImage) effectively demonstrate the component's flexibility with metadata. The naming convention aligns well with the base chain usage in the fixtures.
packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx (6)
19-19: LGTM! Necessary imports for UI improvements.The added imports (
radius,Skeleton,Spinner) are appropriately used throughout the component for consistent styling and improved loading states.Also applies to: 33-33, 35-35
167-194: Excellent loading state improvements!The refactored loading UI provides better visual feedback:
- Dashed lines for clearer section separation
- Increased spacing improves readability
- Structured SkeletonRow components with consistent sizing
- Disabled button with Spinner clearly indicates an active loading state
298-321: Great addition of the Network section!The new network display provides valuable context about which chain the transaction will execute on, with consistent styling (ChainIcon + ChainName) that matches the rest of the UI.
336-344: Good overflow handling for cost display.The ellipsis styling with
maxWidth: "60%"prevents long cost values from breaking the two-column layout while maintaining readability.
426-442: LGTM! Consistent button styling.The
borderRadius: radius.fullapplied to both the execute and connect buttons provides visual consistency across different authentication states.
246-378: Excellent layout restructuring!The changes improve the overall visual hierarchy:
- Consistent
"lg"spacing between major sections- Dashed lines for clear visual separation
- Cleaner Container structure with
flex="column"andgap="sm"for info rows- Logical grouping of Contract, Address, Network, Cost, and Network Fees
These changes significantly enhance readability and maintainability.
| <a | ||
| href={`https://thirdweb.com/${transaction.chain.id}/${transaction.to}`} | ||
| rel="noopener noreferrer" | ||
| style={{ | ||
| color: theme.colors.accentText, | ||
| fontFamily: "monospace", | ||
| fontSize: fontSize.sm, | ||
| textDecoration: "none", | ||
| }} | ||
| /> | ||
| target="_blank" | ||
| > | ||
| {shortenAddress(transaction.to as string)} | ||
| </a> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for PreparedTransaction type definition
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs rg "type PreparedTransaction|interface PreparedTransaction" -lRepository: thirdweb-dev/js
Length of output: 1301
🏁 Script executed:
# Search for the type definition more broadly
rg "PreparedTransaction\s*=" -A 20 --type ts --type tsxRepository: thirdweb-dev/js
Length of output: 86
🏁 Script executed:
# Also check the file itself to see how transaction is typed
cat -n packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx | head -50Repository: thirdweb-dev/js
Length of output: 2767
🏁 Script executed:
cat -n packages/thirdweb/src/transaction/prepare-transaction.ts | head -150Repository: thirdweb-dev/js
Length of output: 4942
🏁 Script executed:
# Search for the PreparedTransaction type definition specifically
rg "type PreparedTransaction" packages/thirdweb/src/transaction/prepare-transaction.ts -A 30Repository: thirdweb-dev/js
Length of output: 1043
Remove type assertion or add null-safety check before using transaction.to.
The to property in PreparedTransaction is optional (to?: Address | undefined), so the assertion transaction.to as string on line 293 bypasses necessary null-safety checks. Either guard with a conditional before using, or replace the assertion with a non-null assertion (transaction.to!) if business logic guarantees it's always defined.
🤖 Prompt for AI Agents
In @packages/thirdweb/src/react/web/ui/Bridge/TransactionPayment.tsx around
lines 282 - 294, The anchor uses a forced cast transaction.to as string even
though PreparedTransaction.to is optional; update the rendering in
TransactionPayment to handle a possibly undefined transaction.to by either (a)
conditionally rendering the entire <a> element only when transaction.to is
defined (check transaction.to and call shortenAddress(transaction.to) safely),
or (b) if business logic guarantees presence, replace the cast with a non-null
assertion transaction.to! and document that guarantee; reference the transaction
object and shortenAddress call to locate the change and ensure the href build
and display use a null-safe value.
PR-Codex overview
This PR focuses on improving the UI of the
TransactionWidgetin thethirdwebpackage, enhancing the user experience during transaction processing.Detailed summary
chainfrombaseSepoliatobaseinsimpleBuyRequest.BaseEthTransfer,BaseEthTransferWithDescription, andBaseEthTransferWithImage.TransactionPaymentcomponent with improved spacing, loading indicators, and button styles.Summary by CodeRabbit
UI Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.