-
Notifications
You must be signed in to change notification settings - Fork 545
[Dashboard] Refactor: Bridge UI Cleanup and Fixes #7165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…perience - Added `constants.ts` to configure Thirdweb client for the bridge application. - Created `BridgeLayout` component to structure the bridge page layout. - Developed `BridgePage` to handle the main functionality of the Universal Bridge, including metadata and dynamic content. - Introduced `Providers.client.tsx` to wrap the application with necessary context providers. - Implemented `UniversalBridgeEmbed` component for embedding payment functionalities with theme support. This commit establishes the foundational components for the Universal Bridge feature, improving the overall user interface and experience.
- Added specific error messages for insufficient native funds in both `SwapConfirmationScreen` and `TransferConfirmationScreen`. - Improved user feedback by providing clear titles and messages when transactions cannot be approved or confirmed due to lack of funds for gas.
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 context provider component and a layout were introduced for the dashboard bridge app. The bridge embed component was refactored to use a shared client instance from new constants. The bridge page logic was updated to await search parameters and simplified to use the new client setup. Error handling for insufficient gas funds was enhanced in wallet swap and transfer confirmation screens. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BridgePage
participant BridgeLayout
participant Providers
participant UniversalBridgeEmbed
participant bridgeAppThirdwebClient
User->>BridgePage: Access bridge page
BridgePage->>BridgeLayout: Render layout with children
BridgeLayout->>Providers: Wrap children with providers
Providers->>UniversalBridgeEmbed: Render embed component
UniversalBridgeEmbed->>bridgeAppThirdwebClient: Use shared client for PayEmbed
Suggested labels
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/dashboard/src/app/bridge/constants.ts (1)
20-22
: Consider removing or conditional logging for the console.log.The console.log statement will execute in all non-production environments. Consider using a more specific condition or removing it to avoid cluttering logs in staging/development.
- console.log("Setting domains for bridge app", THIRDWEB_BRIDGE_URL); + // Optional: Use conditional logging or remove entirely + // console.log("Setting domains for bridge app", THIRDWEB_BRIDGE_URL);apps/dashboard/src/app/bridge/page.tsx (1)
18-21
: Good refactoring for Next.js async searchParams pattern.The function rename to
BridgePage
improves clarity, and the async searchParams handling aligns with Next.js 15 patterns. However, consider adding error handling for the await operation.Consider adding error handling for robustness:
export default async function BridgePage({ searchParams, }: { searchParams: Promise<Record<string, string | string[]>> }) { - const { chainId } = await searchParams; + const { chainId } = await searchParams.catch(() => ({}));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/dashboard/src/app/bridge/components/client/Providers.client.tsx
(1 hunks)apps/dashboard/src/app/bridge/components/client/UniversalBridgeEmbed.tsx
(1 hunks)apps/dashboard/src/app/bridge/constants.ts
(1 hunks)apps/dashboard/src/app/bridge/layout.tsx
(1 hunks)apps/dashboard/src/app/bridge/page.tsx
(2 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/ConfirmationScreen.tsx
(3 hunks)packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/TransferConfirmationScreen.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/dashboard/src/app/bridge/constants.ts (3)
apps/dashboard/src/constants/urls.ts (2)
THIRDWEB_BRIDGE_URL
(27-28)THIRDWEB_INSIGHT_API_DOMAIN
(24-25)packages/thirdweb/src/exports/thirdweb.ts (1)
createThirdwebClient
(24-24)apps/dashboard/src/@/constants/public-envs.ts (2)
NET_PUBLIC_DASHBOARD_THIRDWEB_CLIENT_ID
(1-2)NEXT_PUBLIC_IPFS_GATEWAY_URL
(13-15)
apps/dashboard/src/app/bridge/components/client/UniversalBridgeEmbed.tsx (1)
apps/dashboard/src/app/bridge/constants.ts (1)
bridgeAppThirdwebClient
(46-46)
apps/dashboard/src/app/bridge/layout.tsx (1)
apps/dashboard/src/app/bridge/components/client/Providers.client.tsx (1)
Providers
(6-20)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/TransferConfirmationScreen.tsx (3)
129-135
: Good enhancement for gas fund error detection.The addition of specific error handling for insufficient gas funds during the approval step provides clearer user feedback compared to generic error messages.
148-152
: Improved user rejection error detection.Expanding the user rejection detection to include "user closed modal" and "user denied" covers more wallet implementations and user interaction patterns, which improves the robustness of error handling.
158-164
: Consistent gas fund error handling for transfer step.The error handling for insufficient gas funds during transfer/execute operations is consistent with the approval step, providing a cohesive user experience.
packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/ConfirmationScreen.tsx (3)
81-87
: Consistent gas fund error detection for approval.The error handling enhancement matches the pattern established in TransferConfirmationScreen.tsx, ensuring consistent user experience across different wallet flows.
106-112
: Consistent gas fund error detection for swap confirmation.This maintains the same error handling pattern for the swap step, providing clear feedback when users lack sufficient native funds for gas.
350-350
: Critical fix: Ensure error message is captured in state.Adding
setError((e as Error).message)
is essential for the UI error handling logic to access and display specific error messages. This was a missing piece that could have prevented proper error feedback to users.apps/dashboard/src/app/bridge/components/client/Providers.client.tsx (1)
6-20
: LGTM! Clean provider setup with proper configuration.The provider hierarchy is well-structured with appropriate configurations:
- ThemeProvider correctly configured for bridge app's dark theme requirements
- Toaster properly positioned for toast notifications
- Clean component composition pattern
Note: The ThirdwebProvider has no explicit client prop, which appears intentional since the client is now centrally managed via the constants file.
apps/dashboard/src/app/bridge/constants.ts (1)
35-43
: Excellent client configuration pattern.The centralized client setup with explicit configuration is well-implemented:
- Proper use of public client ID for frontend usage
- Explicit
secretKey: undefined
for clarity- IPFS gateway configuration with environment variable fallback
- Clean separation of concerns with the helper function
apps/dashboard/src/app/bridge/components/client/UniversalBridgeEmbed.tsx (1)
7-15
: Great refactor! Simplified component interface with centralized client usage.The removal of the
client
prop and adoption of the sharedbridgeAppThirdwebClient
improves:
- Component reusability (no need to pass client everywhere)
- Maintainability (centralized client configuration)
- API surface reduction (cleaner component interface)
The relative import paths are also an improvement for maintainability.
apps/dashboard/src/app/bridge/layout.tsx (1)
12-29
: Well-structured layout component following Next.js best practices.The layout implementation is excellent:
- Proper Inter font configuration with variable and display swap
- Appropriate HTML structure with lang attribute
suppressHydrationWarning
correctly used for theme-dependent rendering- Clean utility class composition with
cn
- Proper provider wrapping to establish context for all bridge components
This creates a solid foundation for the bridge app's UI structure.
apps/dashboard/src/app/bridge/page.tsx (1)
35-35
: Minor UI adjustment approved.The bottom spacing reduction from
bottom-24
tobottom-8
appears to be a visual refinement for better layout spacing.
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 #7165 +/- ##
==========================================
- Coverage 55.70% 55.66% -0.05%
==========================================
Files 904 904
Lines 58358 58391 +33
Branches 4114 4111 -3
==========================================
- Hits 32510 32505 -5
- Misses 25743 25781 +38
Partials 105 105
🚀 New features to boost your workflow:
|
size-limit report 📦
|
PR-Codex overview
This PR introduces a
Providers
component for managing themes and notifications in thedashboard
app, enhances error handling in theConfirmationScreen
andTransferConfirmationScreen
, and updates theUniversalBridgeEmbed
to use a new client setup.Detailed summary
Providers
component inProviders.client.tsx
for theme and notification management.BridgeLayout
to include theProviders
component.SwapConfirmationScreen
andTransferConfirmationScreen
.UniversalBridgeEmbed
to usebridgeAppThirdwebClient
.bridgeAppThirdwebClient
with environment-specific configurations inconstants.ts
.RoutesPage
toBridgePage
inpage.tsx
and adjusted client handling.Summary by CodeRabbit
New Features
Improvements
Updates