feat: add modal display mode to swap widget demo#12421
Merged
Merged
Conversation
Adds an Inline/Modal display toggle to the demo customizer. Modal mode renders a host-style "Swap crypto" CTA that opens the widget in a centered overlay (backdrop/Escape/close button to dismiss), showing how a client integration would present the widget behind a button. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds a modal display mode to the swap widget demo. Users can toggle between inline and modal presentation via a "Display" setting in the customizer, which persists to localStorage. A new ChangesModal Display Mode Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/swap-widget/src/demo/InternalWalletApp.tsx (1)
32-42: ⚡ Quick winConsider wrapping the
widgetconstant inuseMemo.The
widgetconstant is a derived JSX value that should be wrapped inuseMemoper coding guidelines. This mirrors the same recommendation forExternalWalletApp.tsx.♻️ Suggested refactor
+ const widget = useMemo( + () => ( - const widget = ( <SwapWidget partnerCode={partnerCode || undefined} theme={themeConfig} onSwapSuccess={handleSwapSuccess} onSwapError={handleSwapError} showPoweredBy={true} showConnectButton={true} walletConnectProjectId={PROJECT_ID} /> + ), + [partnerCode, themeConfig, handleSwapSuccess, handleSwapError], - ) + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swap-widget/src/demo/InternalWalletApp.tsx` around lines 32 - 42, The JSX constant widget should be memoized with React.useMemo to avoid recreating the SwapWidget on every render; replace the plain widget assignment with useMemo(() => (<SwapWidget .../>), [...]) and include all props and handlers as dependencies (partnerCode or partnerCode-derived value, themeConfig, handleSwapSuccess, handleSwapError, PROJECT_ID and any booleans like showPoweredBy/showConnectButton) so the memo updates correctly when inputs change.Source: Coding guidelines
packages/swap-widget/src/demo/WidgetModal.tsx (1)
14-25: 💤 Low valueConsider using the
closecallback for consistency.The keyboard event handler on line 17 calls
setIsOpen(false)directly, while aclosecallback is already defined on line 12 for this exact purpose. Usingclose()instead would follow the DRY principle and maintain consistency with theopencallback pattern.♻️ Suggested refactor
useEffect(() => { if (!isOpen) return const onKeyDown = (e: KeyboardEvent) => { - if (e.key === 'Escape') setIsOpen(false) + if (e.key === 'Escape') close() } window.addEventListener('keydown', onKeyDown) document.body.style.overflow = 'hidden' return () => { window.removeEventListener('keydown', onKeyDown) document.body.style.overflow = '' } - }, [isOpen]) + }, [isOpen, close])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swap-widget/src/demo/WidgetModal.tsx` around lines 14 - 25, The Escape key handler in the useEffect should call the existing close callback instead of directly calling setIsOpen(false); update the onKeyDown function in WidgetModal (the handler currently referencing setIsOpen) to call close() so the modal is closed via the canonical callback, keeping behavior consistent with the open/close pattern and avoiding direct state mutation; ensure close is in scope and leave the addEventListener/removeEventListener and body overflow cleanup as-is.packages/swap-widget/src/demo/ExternalWalletApp.tsx (1)
81-90: ⚡ Quick winConsider wrapping the
widgetconstant inuseMemo.The
widgetconstant is a derived JSX value based onpartnerCode,themeConfig, and the callback handlers. Per coding guidelines, derived values should be wrapped inuseMemoto prevent unnecessary recreation on every render.♻️ Suggested refactor
+ const widget = useMemo( + () => ( - const widget = ( <SwapWidget partnerCode={partnerCode || undefined} theme={themeConfig} onSwapSuccess={handleSwapSuccess} onSwapError={handleSwapError} showPoweredBy={true} showConnectButton={false} /> + ), + [partnerCode, themeConfig, handleSwapSuccess, handleSwapError], - ) + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/swap-widget/src/demo/ExternalWalletApp.tsx` around lines 81 - 90, The JSX constant widget (the <SwapWidget ... /> created from partnerCode, themeConfig, handleSwapSuccess, handleSwapError) should be memoized with useMemo to avoid recreating the element each render; update ExternalWalletApp to import/use React.useMemo and wrap the widget creation in useMemo with a dependency array containing partnerCode, themeConfig, handleSwapSuccess, and handleSwapError so the SwapWidget is only recreated when those values change.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/swap-widget/src/demo/ExternalWalletApp.tsx`:
- Around line 81-90: The JSX constant widget (the <SwapWidget ... /> created
from partnerCode, themeConfig, handleSwapSuccess, handleSwapError) should be
memoized with useMemo to avoid recreating the element each render; update
ExternalWalletApp to import/use React.useMemo and wrap the widget creation in
useMemo with a dependency array containing partnerCode, themeConfig,
handleSwapSuccess, and handleSwapError so the SwapWidget is only recreated when
those values change.
In `@packages/swap-widget/src/demo/InternalWalletApp.tsx`:
- Around line 32-42: The JSX constant widget should be memoized with
React.useMemo to avoid recreating the SwapWidget on every render; replace the
plain widget assignment with useMemo(() => (<SwapWidget .../>), [...]) and
include all props and handlers as dependencies (partnerCode or
partnerCode-derived value, themeConfig, handleSwapSuccess, handleSwapError,
PROJECT_ID and any booleans like showPoweredBy/showConnectButton) so the memo
updates correctly when inputs change.
In `@packages/swap-widget/src/demo/WidgetModal.tsx`:
- Around line 14-25: The Escape key handler in the useEffect should call the
existing close callback instead of directly calling setIsOpen(false); update the
onKeyDown function in WidgetModal (the handler currently referencing setIsOpen)
to call close() so the modal is closed via the canonical callback, keeping
behavior consistent with the open/close pattern and avoiding direct state
mutation; ensure close is in scope and leave the
addEventListener/removeEventListener and body overflow cleanup as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32fd5cc6-50e7-4048-bc1b-dd36cf98dbc6
📒 Files selected for processing (5)
packages/swap-widget/src/demo/App.csspackages/swap-widget/src/demo/DemoCustomizer.tsxpackages/swap-widget/src/demo/ExternalWalletApp.tsxpackages/swap-widget/src/demo/InternalWalletApp.tsxpackages/swap-widget/src/demo/WidgetModal.tsx
Memoize the SwapWidget element in both demo apps and close the modal via the canonical close callback in the Escape handler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Adds an Inline / Modal display toggle to the swap widget demo app's customizer to better demonstrate how a client would integrate the widget behind a button.
WidgetModaldemo component: a host-style "Swap crypto" CTA (themed via the demo's accent color) that opens the widget in a centered overlay with blurred backdrop. Dismissable via backdrop click, Escape, or close button; body scroll is locked while open.Demo-only change: nothing under
src/demo/is part of the published SDK (distis built fromsrc/index.tsvia tsup; verified the builtindex.css/index.jscontain no demo code or styles).Issue (if applicable)
closes #
Risk
Near zero — isolated to the swap widget demo app. No widget, SDK, or web app code paths affected. No on-chain transaction changes.
None.
Testing
Engineering
cd packages/swap-widget && pnpm devpnpm run buildand confirmdist/index.csscontains nodemo-classesOperations
Screenshots (if applicable)
🤖 Generated with Claude Code
Summary by CodeRabbit