Conversation
WalkthroughImplements a transaction status workflow for Pulse Sell: HomeScreen stores transaction context and renders a new TransactionStatus panel; PreviewSell invokes this via a new prop. Adds components for status/details/error/info and a progress step. Polls user operation status, tracks timestamps, txHash, gas fee, and supports close/reset. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant PS as PreviewSell
participant HS as HomeScreen
participant TS as TransactionStatus
participant SVC as getUserOperationStatus
User->>PS: Initiate Sell
PS->>PS: Send batch (userOp)
PS-->>HS: showTransactionStatus(userOpHash, gasFee)
HS->>HS: Store context (sellToken, amount, offer, isBuy)
HS->>TS: Render with userOpHash + context
loop Poll every 2s
TS->>SVC: fetch status(userOpHash)
SVC-->>TS: status + txHash?
TS->>TS: Map to UI state (Starting/Pending/Complete/Failed)
end
alt Completed
TS->>TS: Capture txHash, completedAt
else Failed
TS->>TS: Capture errorDetails (with confirm window)
end
User-->>TS: View Details (optional)
TS-->>HS: Close (outside click / Done)
HS->>HS: Reset status and context
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Deploying x with
|
| Latest commit: |
af7779e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e2b35703.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3681-transaction-st.x-e62.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/apps/pulse/components/Sell/PreviewSell.tsx (2)
2-2: Incorrect import for react-copy-to-clipboard (will break at runtime).Use the default export.
-import { CopyToClipboard } from 'react-copy-to-clipboard'; +import CopyToClipboard from 'react-copy-to-clipboard';
1-12: Normalize CopyToClipboard imports & replace absolute '/src/...' asset usages
Inconsistent CopyToClipboard usage confirmed — some files use named imports ({ CopyToClipboard }) (e.g., src/apps/pulse/components/Sell/PreviewSell.tsx, src/apps/pulse/components/Buy/PreviewBuy.tsx) while many use the default import (e.g., src/apps/pulse/components/Sell/TransactionInfo.tsx, src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx). Standardize to the default form: import CopyToClipboard from 'react-copy-to-clipboard';
Absolute asset references (src="/src/...") found across components and snapshots (e.g., src/apps/pulse/components/Sell/TransactionInfo.tsx, TransactionStatus.tsx, TransactionDetails.tsx and many test snapshots). Replace with module imports (import icon from '../../assets/...'; use
) or consolidate a single, documented convention (public/ vs imported assets) and apply it repo-wide.
🧹 Nitpick comments (23)
src/apps/pulse/components/Sell/TransactionErrorBox.tsx (2)
43-55: Add basic a11y to the expandable “Technical Details” section.Expose state to assistive tech and link the button to the content region.
- <button - type="button" - onClick={() => setIsExpanded(!isExpanded)} + <button + type="button" + aria-expanded={isExpanded} + aria-controls="transaction-error-details" + onClick={() => setIsExpanded((v) => !v)} className="px-2 py-1.5 rounded-[6px] bg-[#FF366C]/30 text-white text-[13px] font-normal flex items-center gap-2" > ... - {isExpanded && ( - <div className="mt-2 p-3 bg-black/20 rounded-[6px] border border-[#FF366C]/30"> + {isExpanded && ( + <div id="transaction-error-details" className="mt-2 p-3 bg-black/20 rounded-[6px] border border-[#FF366C]/30"> <p className="text-white/70 text-[12px] font-mono whitespace-pre-wrap break-all"> {technicalDetails} </p> </div> )}Also applies to: 71-77
56-69: Improve copy button a11y.Provide an explicit label for screen readers; harmless to keep the visible “Copy” text.
- <CopyToClipboard text={technicalDetails} onCopy={handleCopy}> + <CopyToClipboard text={technicalDetails} onCopy={handleCopy}> <button type="button" + aria-label="Copy technical details" className="px-2 py-1.5 rounded-[6px] bg-[#FF366C]/30 text-white text-[13px] font-normal flex items-center gap-2" >src/apps/pulse/components/Sell/ProgressStep.tsx (2)
26-35: Failed state circle uses success color.Use the error color for failed steps.
- if (status === 'failed') { - return `${baseClasses} bg-[#8A77FF]`; - } + if (status === 'failed') { + return `${baseClasses} bg-[#FF366C]`; + }
38-40: Add failed color for connecting line.Show a red connector when a segment failed.
- const getLineClasses = () => { - return `w-0.5 h-4 ml-2 ${lineStatus === 'completed' ? 'bg-[#8A77FF]' : 'bg-[#121116]'}`; - }; + const getLineClasses = () => { + return `w-0.5 h-4 ml-2 ${ + lineStatus === 'completed' + ? 'bg-[#8A77FF]' + : lineStatus === 'failed' + ? 'bg-[#FF366C]' + : 'bg-[#121116]' + }`; + };src/apps/pulse/components/Sell/TransactionInfo.tsx (3)
51-57: Guard against empty/unknown block explorer base URL.Avoid opening a broken relative URL when the chain isn’t supported.
- const handleExternalLink = () => { - window.open( - `${getBlockScan(chainId)}${displayTxHash}`, - '_blank', - 'noopener,noreferrer' - ); - }; + const handleExternalLink = () => { + const base = getBlockScan(chainId); + if (!base || !hasValidTxHash) return; + window.open(`${base}${displayTxHash}`, '_blank', 'noopener,noreferrer'); + };
112-117: Import static assets instead of absolute “/src/…” paths.Absolute src paths are brittle in production builds; import and reference the asset.
+import ConfirmedIcon from '../../assets/confirmed-icon.svg'; ... - <img - src="/src/apps/pulse/assets/confirmed-icon.svg" + <img + src={ConfirmedIcon} alt="confirmed" className="w-[8px] h-[5px]" />
118-121: Avoid string sentinel checks in UI logic.Rather than keying style off lhs === "Status", pass an explicit boolean to detailsEntry (e.g., isStatusRow) for clarity and resilience.
src/apps/pulse/components/App/HomeScreen.tsx (5)
79-86: Pause refresh while transaction status is shown.Prevents churn and unnecessary network calls during the status flow.
- const handleRefresh = useCallback(async () => { + const handleRefresh = useCallback(async () => { // Prevent multiple simultaneous refresh calls - if (isRefreshingHome) { + if (isRefreshingHome || transactionStatus) { return; } ... - }, [ + }, [ refetchWalletPortfolio, isBuy, sellToken, tokenAmount, isInitialized, getBestSellOffer, buyRefreshCallback, previewBuy, isRefreshingHome, + transactionStatus, ]);Also applies to: 116-126
171-180: Gate sell-mode auto-refresh during transaction status.- useEffect(() => { - if (!isBuy && sellToken && tokenAmount && isInitialized) { + useEffect(() => { + if (!isBuy && sellToken && tokenAmount && isInitialized && !transactionStatus) { const interval = setInterval(() => { handleRefresh(); }, 15000); // 15 seconds return () => clearInterval(interval); } return undefined; - }, [isBuy, sellToken, tokenAmount, isInitialized, handleRefresh]); + }, [isBuy, sellToken, tokenAmount, isInitialized, transactionStatus, handleRefresh]);
183-192: Gate buy-mode auto-refresh during transaction status.- useEffect(() => { - if (isBuy && buyRefreshCallback && !previewBuy) { + useEffect(() => { + if (isBuy && buyRefreshCallback && !previewBuy && !transactionStatus) { const interval = setInterval(() => { handleRefresh(); }, 15000); // 15 seconds return () => clearInterval(interval); } return undefined; - }, [isBuy, buyRefreshCallback, previewBuy, handleRefresh]); + }, [isBuy, buyRefreshCallback, previewBuy, transactionStatus, handleRefresh]);
226-241: Only render TransactionStatus when userOpHash is set.Prevents mounting the poller with an empty hash.
- if (transactionStatus) { + if (transactionStatus && userOpHash) { return ( <div className="w-full flex justify-center p-3 mb-[70px]"> <TransactionStatus closeTransactionStatus={closeTransactionStatus} userOpHash={userOpHash} chainId={transactionData?.sellToken?.chainId || 1} gasFee={transactionGasFee} isBuy={transactionData?.isBuy || false} sellToken={transactionData?.sellToken} tokenAmount={transactionData?.tokenAmount} sellOffer={transactionData?.sellOffer} /> </div> ); }
157-160: Reset ephemeral fields on close.Optional hygiene: clear userOpHash and gas fee string on close.
const closeTransactionStatus = () => { setTransactionStatus(false); setTransactionData(null); + setUserOpHash(''); + setTransactionGasFee('≈ $0.00'); };src/apps/pulse/components/Sell/PreviewSell.tsx (4)
28-35: Name the callback param for what it is (userOpHash).Avoid confusion with txHash; you’re passing a UserOperation hash.
interface PreviewSellProps { closePreview: () => void; - showTransactionStatus: (txHash: string, gasFee?: string) => void; + showTransactionStatus: (userOpHash: string, gasFee?: string) => void;
150-165: Don’t close on outside click while executing or awaiting signature.Prevents accidental dismissal mid-flow.
- useEffect(() => { + useEffect(() => { const handleClickOutside = (event: MouseEvent) => { + if (isExecuting || isWaitingForSignature) return; if ( previewModalRef.current && !previewModalRef.current.contains(event.target as Node) ) { closePreview(); } }; ... - }, [closePreview]); + }, [closePreview, isExecuting, isWaitingForSignature]);
193-201: Duplicate unmount cleanup effect.You already clean up the batch at Lines 133-140. Remove this duplicate.
- useEffect(() => { - return () => { - if (sellToken) { - cleanupBatch(sellToken.chainId, 'unmount'); - } - }; - }, [sellToken, cleanupBatch]);
358-363: Guard undefined nativeTokenSymbol in gas fee string.Provide a fallback symbol to avoid “undefined”.
- const gasFeeString = gasCostNative - ? `≈ ${parseFloat(gasCostNative).toFixed(6)} ${nativeTokenSymbol}` + const gasFeeString = gasCostNative + ? `≈ ${parseFloat(gasCostNative).toFixed(6)} ${nativeTokenSymbol || 'NATIVE'}` : '≈ 0.00';src/apps/pulse/components/Sell/TransactionStatus.tsx (4)
63-65: Use environment-agnostic timeout type.Avoid NodeJS.Timeout in the browser.
- const [failureTimeoutId, setFailureTimeoutId] = - useState<NodeJS.Timeout | null>(null); + const [failureTimeoutId, setFailureTimeoutId] = + useState<ReturnType<typeof setTimeout> | null>(null);
297-331: Import assets instead of absolute “/src/…” paths.Use module imports for icons to avoid prod build issues.
+import PendingIcon from '../../assets/pending.svg'; +import ConfirmedIcon from '../../assets/confirmed-icon.svg'; +import FailedIcon from '../../assets/failed-icon.svg'; ... - icon: '/src/apps/pulse/assets/pending.svg', + icon: PendingIcon, ... - icon: '/src/apps/pulse/assets/confirmed-icon.svg', + icon: ConfirmedIcon, ... - icon: '/src/apps/pulse/assets/failed-icon.svg', + icon: FailedIcon,
280-295: Clear any pending failure timeout on unmount.Avoid setState after unmount.
useEffect(() => { const handleClickOutside = (event: MouseEvent) => { if ( transactionStatusRef.current && !transactionStatusRef.current.contains(event.target as Node) ) { closeTransactionStatus(); } }; document.addEventListener('mousedown', handleClickOutside); return () => { document.removeEventListener('mousedown', handleClickOutside); + if (failureTimeoutId) { + clearTimeout(failureTimeoutId); + } }; - }, [closeTransactionStatus]); + }, [closeTransactionStatus, failureTimeoutId]);
447-451: Announce status changes to assistive tech.Add a live region for the status text.
- <div - className={`text-white font-normal text-[20px] ${statusIcon ? 'mt-3' : ''}`} - > + <div + role="status" + aria-live="polite" + className={`text-white font-normal text-[20px] ${statusIcon ? 'mt-3' : ''}`} + > {currentStatus} </div>src/apps/pulse/components/Sell/TransactionDetails.tsx (3)
115-118: Add keyboard support for the tooltip trigger.Enable focus-based access for keyboard users.
Apply this diff:
- <span + <span className="text-white/30 cursor-help" onMouseEnter={() => setShowTooltip(true)} onMouseLeave={() => setShowTooltip(false)} + tabIndex={0} + onFocus={() => setShowTooltip(true)} + onBlur={() => setShowTooltip(false)} >
11-15: Narrow types for status/steps to prevent “string” widening and ensure type‑safe props.This avoids accidental typos and aligns with likely ProgressStep props.
Apply this diff:
interface TransactionDetailsProps { onDone: () => void; userOpHash: string; chainId: number; - status: - | 'Starting Transaction' - | 'Transaction Pending' - | 'Transaction Complete' - | 'Transaction Failed'; + status: TransactionStatus; @@ +type TransactionStatus = + | 'Starting Transaction' + | 'Transaction Pending' + | 'Transaction Complete' + | 'Transaction Failed'; + +type StepName = 'Submitted' | 'Pending' | 'Completed'; +type StepStatus = 'pending' | 'completed' | 'failed'; @@ - const getStepStatus = (step: 'Submitted' | 'Pending' | 'Completed') => { + const getStepStatus = (step: StepName): StepStatus => { @@ - const getStepTimestamp = (step: 'Submitted' | 'Pending' | 'Completed') => { + const getStepTimestamp = (step: StepName): string | undefined => {Also applies to: 55-56, 83-83
194-195: Completed time is wired to pendingCompletedAt.If TransactionInfo.completedAt is meant to reflect final completion, this may be misleading.
Apply this diff as a safe improvement (until a dedicated completedAt is available):
- completedAt={pendingCompletedAt} + completedAt={status === 'Transaction Complete' ? pendingCompletedAt : undefined}Optionally extend props with a proper completedAt and pass that instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/apps/pulse/assets/confirmed-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/failed-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/pending.svgis excluded by!**/*.svgsrc/apps/pulse/assets/transaction-failed-details-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
src/apps/pulse/components/App/HomeScreen.tsx(5 hunks)src/apps/pulse/components/Sell/PreviewSell.tsx(2 hunks)src/apps/pulse/components/Sell/ProgressStep.tsx(1 hunks)src/apps/pulse/components/Sell/TransactionDetails.tsx(1 hunks)src/apps/pulse/components/Sell/TransactionErrorBox.tsx(1 hunks)src/apps/pulse/components/Sell/TransactionInfo.tsx(1 hunks)src/apps/pulse/components/Sell/TransactionStatus.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
PR: pillarwallet/x#374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsx
🧬 Code graph analysis (4)
src/apps/pulse/components/Sell/TransactionStatus.tsx (1)
src/services/userOpStatus.ts (1)
getUserOperationStatus(3-44)
src/apps/pulse/components/Sell/PreviewSell.tsx (2)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/apps/pulse/hooks/useRelaySell.ts (1)
SellOffer(37-43)
src/apps/pulse/components/Sell/TransactionInfo.tsx (1)
src/utils/blockchain.ts (1)
getBlockScan(225-246)
src/apps/pulse/components/App/HomeScreen.tsx (2)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/apps/pulse/hooks/useRelaySell.ts (1)
SellOffer(37-43)
⏰ 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: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (4)
src/apps/pulse/components/Sell/TransactionStatus.tsx (2)
304-326: Minor: icon sizes look off for failed state.The failed icon classes are much smaller than others (w-[2px] h-[10px]). Confirm intended sizing after switching to imported assets.
1-9: Repo-wide: replace absolute asset paths with imported assets.
Automated ripgrep in the sandbox failed ("No files were searched"); unable to confirm occurrences. Run a local search (example: rg -n --hidden -S '/src/apps/pulse/assets/' .) and convert any matches to imported asset imports for consistency.src/apps/pulse/components/Sell/TransactionDetails.tsx (2)
86-88: Confirm UX: hide timestamp on the Completed step intentionally?getStepTimestamp suppresses timestamps for the last step even when completed. If UX expects a completion time, remove the “step === 'Completed'” guard.
1-6: Import asset and use module instead of absolute /src path.Asset exists at src/apps/pulse/assets/usd-coin-usdc-logo.png — apply the diff below; also apply the same change at lines 138-142.
import { format } from 'date-fns'; import { useState } from 'react'; import ProgressStep from './ProgressStep'; import TransactionErrorBox from './TransactionErrorBox'; import TransactionInfo from './TransactionInfo'; +import usdcLogo from '../../assets/usd-coin-usdc-logo.png';- <img + <img className="w-4 h-4 rounded" - src="/src/apps/pulse/assets/usd-coin-usdc-logo.png" + src={usdcLogo} alt="USDC" />
| <div className="relative inline-block"> | ||
| <span | ||
| className="text-white/30 cursor-help" | ||
| onMouseEnter={() => setShowTooltip(true)} | ||
| onMouseLeave={() => setShowTooltip(false)} | ||
| > | ||
| {(() => { | ||
| const symbol = sellToken?.symbol || 'Token'; | ||
| return symbol.length > 3 | ||
| ? `${symbol.slice(0, 3)}...` | ||
| : symbol; | ||
| })()} | ||
| </span> | ||
| {showTooltip && | ||
| sellToken?.symbol && | ||
| sellToken.symbol.length > 3 && ( | ||
| <div className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap"> | ||
| {sellToken.symbol} | ||
| </div> | ||
| )} | ||
| </div> | ||
| )}{' '} |
There was a problem hiding this comment.
Replace invalid div-in-p structure with spans (fixes HTML parsing/layout issues) and tag tooltip for a11y.
A block-level div inside a p is invalid HTML and can break layout (browsers auto-close p). Use spans for the wrapper and tooltip container.
Apply this diff:
- ) : (
- <div className="relative inline-block">
+ ) : (
+ <span className="relative inline-block">
<span
className="text-white/30 cursor-help"
- onMouseEnter={() => setShowTooltip(true)}
- onMouseLeave={() => setShowTooltip(false)}
+ onMouseEnter={() => setShowTooltip(true)}
+ onMouseLeave={() => setShowTooltip(false)}
+ aria-describedby="token-symbol-tooltip"
>
{(() => {
const symbol = sellToken?.symbol || 'Token';
return symbol.length > 3
? `${symbol.slice(0, 3)}...`
: symbol;
})()}
</span>
{showTooltip &&
sellToken?.symbol &&
sellToken.symbol.length > 3 && (
- <div className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap">
- {sellToken.symbol}
- </div>
+ <span
+ id="token-symbol-tooltip"
+ role="tooltip"
+ className="absolute bottom-full left-1/2 transform -translate-x-1/2 mb-2 px-2 py-1 bg-black text-white text-xs rounded shadow-lg z-10 whitespace-nowrap"
+ >
+ {sellToken.symbol}
+ </span>
)}
- </div>
+ </span>
)}{' '}Also applies to: 126-132
🤖 Prompt for AI Agents
In src/apps/pulse/components/Sell/TransactionDetails.tsx around lines 113 to 134
(also apply same change to 126-132), there's a block-level div nested inside a p
which is invalid HTML and breaks layout; replace the outer wrapper and the
tooltip container divs with inline elements (span) so they can legally sit
inside a paragraph, and add accessibility attributes to the tooltip (e.g.,
role="tooltip" and aria-hidden toggled via showTooltip, and ensure the trigger
span uses aria-describedby pointing to the tooltip id) so the tooltip is both
valid and accessible.
| } catch (error) { | ||
| console.error('Failed to get user operation status:', error); | ||
| setCurrentStatus('Transaction Failed'); | ||
| setErrorDetails( | ||
| error instanceof Error | ||
| ? error.message | ||
| : 'Failed to get transaction status' | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Stop polling after a fatal error.
Prevents repeated failing calls/log spam.
} catch (error) {
console.error('Failed to get user operation status:', error);
- setCurrentStatus('Transaction Failed');
+ setCurrentStatus('Transaction Failed');
+ setIsPollingActive(false);
setErrorDetails(
error instanceof Error
? error.message
: 'Failed to get transaction status'
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| console.error('Failed to get user operation status:', error); | |
| setCurrentStatus('Transaction Failed'); | |
| setErrorDetails( | |
| error instanceof Error | |
| ? error.message | |
| : 'Failed to get transaction status' | |
| ); | |
| } | |
| }; | |
| } catch (error) { | |
| console.error('Failed to get user operation status:', error); | |
| setCurrentStatus('Transaction Failed'); | |
| setIsPollingActive(false); | |
| setErrorDetails( | |
| error instanceof Error | |
| ? error.message | |
| : 'Failed to get transaction status' | |
| ); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Sell/TransactionStatus.tsx around lines 257 to 266,
the catch block logs errors and updates state but does not stop the polling
loop, causing repeated failing calls and log spam; modify the handler so that
when a fatal error occurs you stop future polls by clearing the interval or
preventing further recursive setTimeouts (e.g., clearInterval(pollTimer) or set
a "isPolling" flag to false) and ensure any timer reference used to schedule the
next status check is cleaned up or not re-scheduled before returning.
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Refactor