Frontend #46
Open
Frontend #46
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
- Add fetchEvents function using Stellar RPC getEvents - Create EventTable component for displaying contract events - Add Events tab to transaction history view - Handle empty/error states gracefully - Events limited to ~7 days RPC retention - Network-aware (testnet/mainnet) - No persistence or ingestion as required
- Calculate startLedger as latestLedger - 121000 (~7 days) - Import jsonRpcCall from lib/rpc - This fixes 'startLedger must be positive' error from RPC
- Replace custom theme logic with next-themes ThemeProvider - Add ThemeProvider to layout with proper SSR configuration - Update ThemeToggle to use useTheme hook with mounted state - Add suppressHydrationWarning to html element - Fixes server/client mismatch in theme state
- Make startLedger calculation more conservative (~3 days instead of 7) - Add retry logic when startLedger is out of RPC retention range - Gracefully fallback to no startLedger if range calculation fails - Prevents 'startLedger must be within the ledger range' errors
- Make contract not found errors more user-friendly - Add specific error handling for invalid contract IDs - Improve event fetching error messages - Handle RPC retention limit errors gracefully - Better UX for network and validation errors
- Validate contract ID format before API calls - Prevent invalid requests to RPC endpoints - Show user-friendly error messages for invalid IDs - Improve overall user experience and reduce API errors
Contributor
|
@Emmanuelluxury is attempting to deploy a commit to the Trustless Work Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR implements network-aware event history features with theme provider integration. Changes include adding next-themes support for system theme detection, introducing event-fetching capabilities alongside transactions, refactoring transaction history into a modal interface, enhancing error recovery via network switching, and updating data fetchers to accept network parameters with improved error handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/escrow/EscrowDetails.tsx (1)
242-268: Avoid shipping DEBUG logging enabledLine 242 hard-codes
DEBUG = true, which will log escrow data in production. That can leak sensitive data and add noise. Gate this behind a config flag or disable by default.🔧 Suggested quick fix
-const DEBUG = true; +const DEBUG = false; // enable locally via config if needed
🤖 Fix all issues with AI agents
In `@src/components/escrow/EventTable.tsx`:
- Around line 109-110: The elements in the EventTable component that include the
className string containing "cursor-pointer" (the card-like divs with classes
starting "bg-white/50 ... cursor-pointer ..." and the similar dark-mode variant)
imply clickability but have no onClick handler; either remove "cursor-pointer"
from those className values or add a proper onClick prop (e.g., onRowClick or
onSelect) and corresponding handler function in the EventTable component to
implement the intended interaction; locate the offending className occurrences
in the EventTable component to update the class strings or wire up the click
handler.
In `@src/components/escrow/TransactionDetailModal.tsx`:
- Around line 60-68: The explorer links in TransactionDetailModal are still
using an env var and a hardcoded testnet URL and therefore can point to the
wrong network after switching; update the component to derive a single explorer
base URL from the current network value (the network prop/state used alongside
fetchTransactionDetails and txHash) — e.g., compute derivedExplorerBase =
getExplorerBaseForNetwork(network) or map network => base inside
TransactionDetailModal, then replace the two hardcoded usages for the explorer
buttons with this derivedExplorerBase (reference the transaction hash via
txHash) so both buttons consistently point to the selected network.
In `@src/components/ui/theme-toggle.tsx`:
- Around line 37-38: isDark is computed from resolvedTheme but toggle uses theme
(which can be "system"), causing incorrect flips when theme === "system"; change
toggle to base its decision on resolvedTheme (use isDark or resolvedTheme) so
setTheme sets "light" when resolvedTheme === "dark" and "dark" otherwise; update
the toggle function (and any references) to call setTheme(isDark ? "light" :
"dark") instead of using theme.
In `@src/custom.d.ts`:
- Around line 1-4: Remove the unused ambient module declaration "declare module
'*.css'" (the block that exports a default any) — delete that declaration (and
the file containing it if it only contains this declaration) and ensure no other
code imports CSS using that default-export pattern before committing.
In `@src/hooks/useEscrowData.ts`:
- Around line 18-30: The early return when contractId is falsy currently calls
setError but doesn't clear previous data; update the branch that checks if
(!contractId) to also clear stale state by calling setRaw(null),
setOrganized(null) and setLoading(false) before returning so previous escrow
data isn't shown while the error is displayed; modify the same hook where
contractId is validated (useEscrowData, and the setError call) to include these
state resets.
In `@src/utils/transactionFetcher.ts`:
- Around line 287-325: The code currently swallows connection errors in the
transaction fetch path (the data.error "Unable to connect" branch and the catch
block) and returns an empty EventResponse, which hides connectivity failures;
update the fetch function (the async fetch logic that builds EventMetadata and
returns {events, latestLedger, cursor, hasMore}) to propagate errors instead of
returning empty results: replace the data.error "Unable to connect" early-return
with throwing an Error (or rethrowing the original error) and in the catch block
rethrow the caught error (or throw a new Error with context) so callers of this
transaction fetch routine receive an exception (or alternatively modify the
EventResponse contract to include an error field and populate it) — change the
branches around data.error and the catch to throw rather than returning empty
events to allow the UI to surface real errors.
🧹 Nitpick comments (3)
src/utils/ledgerkeycontract.ts (2)
50-86: Mixed error handling creates ambiguous return semantics.The function now has multiple return paths with different meanings:
null: network error, HTTP error, JSON parse error, RPC error, or contract not found[]: escrow entry exists butval.mapis missing/not an array (line 114)EscrowMap: success with data- Thrown error: missing contract instance, storage, or escrow data
This makes it difficult for consumers to distinguish between "contract not found on this network" (recoverable) vs "network failure" (retry-able) vs "malformed data" (bug).
Consider returning a discriminated result type or at minimum, differentiating "not found" from "error":
// Option: Use a result type type FetchResult = | { status: 'success'; data: EscrowMap } | { status: 'not_found' } | { status: 'error'; message: string };
98-98: Remove debug logging before merge.This debug log will output raw contract storage data to the console in production, which could leak sensitive information and pollute logs.
- console.log("[DEBUG] Raw contract storage:", JSON.stringify(storage, null, 2));src/components/escrow/error-display.tsx (1)
12-12: Fragile error detection via string matching.The
isContractNotFoundcheck relies on exact substrings in the error message. If error messages are refactored elsewhere, this detection will silently break.Consider passing an explicit flag or using an error type/code:
interface ErrorDisplayProps { error: string | null + isRecoverableByNetworkSwitch?: boolean onSwitchNetwork?: () => void switchNetworkLabel?: string } - const isContractNotFound = error?.includes("Contract not found") && error?.includes("Try switching to") + // Use explicit flag passed by parent instead of string parsingThis decouples the display logic from error message formatting.
Comment on lines
+109
to
+110
| className="bg-white/50 backdrop-blur-sm border border-gray-200 rounded-xl p-4 hover:shadow-md hover:bg-white/80 cursor-pointer transition-all duration-200 dark:bg-[#070708] dark:border-[rgba(255,255,255,0.04)] dark:hover:bg-[#0b1115]" | ||
| > |
There was a problem hiding this comment.
Remove pointer cursor or add click behavior
Line 109 and Line 187 set cursor-pointer but there’s no click handler, which implies interactivity that doesn’t exist. Remove the cursor style or wire up an onClick.
🔧 Suggested tweak (remove misleading cursor)
- className="bg-white/50 backdrop-blur-sm border border-gray-200 rounded-xl p-4 hover:shadow-md hover:bg-white/80 cursor-pointer transition-all duration-200 dark:bg-[`#070708`] dark:border-[rgba(255,255,255,0.04)] dark:hover:bg-[`#0b1115`]"
+ className="bg-white/50 backdrop-blur-sm border border-gray-200 rounded-xl p-4 hover:shadow-md hover:bg-white/80 transition-all duration-200 dark:bg-[`#070708`] dark:border-[rgba(255,255,255,0.04)] dark:hover:bg-[`#0b1115`]"- className="hover:bg-purple-50/50 cursor-pointer transition-all duration-200 group dark:hover:bg-[`#081014`]/40"
+ className="hover:bg-purple-50/50 transition-all duration-200 group dark:hover:bg-[`#081014`]/40"Also applies to: 187-188
🤖 Prompt for AI Agents
In `@src/components/escrow/EventTable.tsx` around lines 109 - 110, The elements in
the EventTable component that include the className string containing
"cursor-pointer" (the card-like divs with classes starting "bg-white/50 ...
cursor-pointer ..." and the similar dark-mode variant) imply clickability but
have no onClick handler; either remove "cursor-pointer" from those className
values or add a proper onClick prop (e.g., onRowClick or onSelect) and
corresponding handler function in the EventTable component to implement the
intended interaction; locate the offending className occurrences in the
EventTable component to update the class strings or wire up the click handler.
Comment on lines
+60
to
+68
| const transactionDetails = await fetchTransactionDetails(txHash, network); | ||
| setDetails(transactionDetails); | ||
| } catch (err) { | ||
| setError("Failed to fetch transaction details"); | ||
| console.error("Error fetching transaction details:", err); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }, [txHash]); | ||
| }, [txHash, network]); |
There was a problem hiding this comment.
Align explorer links with the selected network
Line 60 makes the fetch network-aware, but the explorer buttons still use an env var and a hardcoded testnet URL (Line 163 and Line 171). That can send users to the wrong explorer after a network switch. Consider deriving a single explorer network from network (or config) and reuse it for both buttons.
🔧 Suggested tweak (removes the hardcoded testnet URL)
- onClick={() => window.open(`https://stellar.expert/explorer/testnet/tx/${details.txHash}`, "_blank")}
+ onClick={() => window.open(`https://stellar.expert/explorer/${process.env.NEXT_PUBLIC_STELLAR_NETWORK || "testnet"}/tx/${details.txHash}`, "_blank")}🤖 Prompt for AI Agents
In `@src/components/escrow/TransactionDetailModal.tsx` around lines 60 - 68, The
explorer links in TransactionDetailModal are still using an env var and a
hardcoded testnet URL and therefore can point to the wrong network after
switching; update the component to derive a single explorer base URL from the
current network value (the network prop/state used alongside
fetchTransactionDetails and txHash) — e.g., compute derivedExplorerBase =
getExplorerBaseForNetwork(network) or map network => base inside
TransactionDetailModal, then replace the two hardcoded usages for the explorer
buttons with this derivedExplorerBase (reference the transaction hash via
txHash) so both buttons consistently point to the selected network.
Comment on lines
+37
to
+38
| const isDark = resolvedTheme === "dark"; | ||
| const toggle = () => setTheme(theme === "dark" ? "light" : "dark"); |
There was a problem hiding this comment.
Inconsistent theme check between display and toggle logic.
isDark uses resolvedTheme (the actual resolved value), but toggle uses theme which can be "system". When theme is "system" and resolved to dark, clicking toggle will set "light" (correct), but if resolved to light, it will also set "light" (incorrect - should set "dark").
Proposed fix
const isDark = resolvedTheme === "dark";
- const toggle = () => setTheme(theme === "dark" ? "light" : "dark");
+ const toggle = () => setTheme(isDark ? "light" : "dark");📝 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.
Suggested change
| const isDark = resolvedTheme === "dark"; | |
| const toggle = () => setTheme(theme === "dark" ? "light" : "dark"); | |
| const isDark = resolvedTheme === "dark"; | |
| const toggle = () => setTheme(isDark ? "light" : "dark"); |
🤖 Prompt for AI Agents
In `@src/components/ui/theme-toggle.tsx` around lines 37 - 38, isDark is computed
from resolvedTheme but toggle uses theme (which can be "system"), causing
incorrect flips when theme === "system"; change toggle to base its decision on
resolvedTheme (use isDark or resolvedTheme) so setTheme sets "light" when
resolvedTheme === "dark" and "dark" otherwise; update the toggle function (and
any references) to call setTheme(isDark ? "light" : "dark") instead of using
theme.
Comment on lines
+1
to
+4
| declare module '*.css' { | ||
| const content: any; | ||
| export default content; | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how CSS files are actually imported in the codebase
echo "=== CSS import patterns in the codebase ==="
rg -n "import .* from ['\"].*\.css['\"]" --type ts --type tsx -g '!*.d.ts' || echo "No default CSS imports found"
echo ""
echo "=== Side-effect CSS imports ==="
rg -n "^import ['\"].*\.css['\"]" --type ts --type tsx -g '!*.d.ts' || echo "No side-effect CSS imports found"
echo ""
echo "=== Check if Next.js config exists ==="
fd -e js -e ts -e mjs "next.config" --exec cat {}
echo ""
echo "=== Check for CSS module imports ==="
rg -n "import .* from ['\"].*\.module\.css['\"]" --type ts --type tsx -g '!*.d.ts' || echo "No CSS module imports found"
echo ""
echo "=== List files in src directory to understand structure ==="
fd . src --max-depth 2 --type fRepository: Trustless-Work/escrow-viewer
Length of output: 1119
Remove this unused declaration—no CSS is imported using this pattern in the codebase.
The ambient module declaration for CSS default exports is not used anywhere. While src/app/globals.css exists, it's handled by Next.js without requiring this declaration. Removing this file will clean up unused code.
🤖 Prompt for AI Agents
In `@src/custom.d.ts` around lines 1 - 4, Remove the unused ambient module
declaration "declare module '*.css'" (the block that exports a default any) —
delete that declaration (and the file containing it if it only contains this
declaration) and ensure no other code imports CSS using that default-export
pattern before committing.
Comment on lines
+18
to
+30
| if (!contractId) { | ||
| setError("Please enter a contract ID"); | ||
| return; | ||
| } | ||
|
|
||
| // Basic validation for contract ID format | ||
| if (!/^C[A-Z0-9]{55}$/.test(contractId)) { | ||
| setError("Invalid contract ID format. Contract IDs should start with 'C' followed by 55 alphanumeric characters."); | ||
| setRaw(null); | ||
| setOrganized(null); | ||
| setLoading(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Clear stale escrow data when contractId is empty
Line 18 returns early without clearing raw/organized, so stale data can remain visible while the error is shown. Consider clearing state and resetting loading before returning.
🔧 Suggested fix
if (!contractId) {
setError("Please enter a contract ID");
+ setRaw(null);
+ setOrganized(null);
+ setLoading(false);
return;
}📝 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.
Suggested change
| if (!contractId) { | |
| setError("Please enter a contract ID"); | |
| return; | |
| } | |
| // Basic validation for contract ID format | |
| if (!/^C[A-Z0-9]{55}$/.test(contractId)) { | |
| setError("Invalid contract ID format. Contract IDs should start with 'C' followed by 55 alphanumeric characters."); | |
| setRaw(null); | |
| setOrganized(null); | |
| setLoading(false); | |
| return; | |
| } | |
| if (!contractId) { | |
| setError("Please enter a contract ID"); | |
| setRaw(null); | |
| setOrganized(null); | |
| setLoading(false); | |
| return; | |
| } | |
| // Basic validation for contract ID format | |
| if (!/^C[A-Z0-9]{55}$/.test(contractId)) { | |
| setError("Invalid contract ID format. Contract IDs should start with 'C' followed by 55 alphanumeric characters."); | |
| setRaw(null); | |
| setOrganized(null); | |
| setLoading(false); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In `@src/hooks/useEscrowData.ts` around lines 18 - 30, The early return when
contractId is falsy currently calls setError but doesn't clear previous data;
update the branch that checks if (!contractId) to also clear stale state by
calling setRaw(null), setOrganized(null) and setLoading(false) before returning
so previous escrow data isn't shown while the error is displayed; modify the
same hook where contractId is validated (useEscrowData, and the setError call)
to include these state resets.
Comment on lines
+287
to
+325
| if (data.error) { | ||
| if (data.error.message?.includes("Unable to connect")) { | ||
| return { | ||
| events: [], | ||
| latestLedger: 0, | ||
| cursor: undefined, | ||
| hasMore: false | ||
| }; | ||
| } | ||
| throw new Error(data.error.message || "Failed to fetch events"); | ||
| } | ||
|
|
||
| const result = data.result; | ||
| const events: EventMetadata[] = (result.events || []).map((event: any) => ({ | ||
| id: event.id, | ||
| type: event.type, | ||
| ledger: event.ledger, | ||
| contractId: event.contractId, | ||
| topics: event.topics || [], | ||
| value: event.value || "", | ||
| inSuccessfulContractCall: event.inSuccessfulContractCall | ||
| })); | ||
|
|
||
| return { | ||
| events, | ||
| latestLedger: result.latestLedger || 0, | ||
| cursor: result.cursor, | ||
| hasMore: !!result.cursor | ||
| }; | ||
|
|
||
| } catch (error) { | ||
| console.error("Error fetching events:", error); | ||
|
|
||
| // Return graceful error response | ||
| return { | ||
| events: [], | ||
| latestLedger: 0, | ||
| hasMore: false | ||
| }; |
There was a problem hiding this comment.
Surface event connectivity failures instead of silently returning empty results
Line 287–325 returns an empty EventResponse on connection errors, which makes the UI show “no events” instead of an error. Prefer bubbling the error (throw or add an error field) so callers can display a real error state.
🔧 Suggested fix (propagate errors)
- if (data.error) {
- if (data.error.message?.includes("Unable to connect")) {
- return {
- events: [],
- latestLedger: 0,
- cursor: undefined,
- hasMore: false
- };
- }
- throw new Error(data.error.message || "Failed to fetch events");
- }
+ if (data.error) {
+ throw new Error(data.error.message || "Failed to fetch events");
+ }
@@
- } catch (error) {
- console.error("Error fetching events:", error);
-
- // Return graceful error response
- return {
- events: [],
- latestLedger: 0,
- hasMore: false
- };
- }
+ } catch (error) {
+ console.error("Error fetching events:", error);
+ throw error;
+ }🤖 Prompt for AI Agents
In `@src/utils/transactionFetcher.ts` around lines 287 - 325, The code currently
swallows connection errors in the transaction fetch path (the data.error "Unable
to connect" branch and the catch block) and returns an empty EventResponse,
which hides connectivity failures; update the fetch function (the async fetch
logic that builds EventMetadata and returns {events, latestLedger, cursor,
hasMore}) to propagate errors instead of returning empty results: replace the
data.error "Unable to connect" early-return with throwing an Error (or
rethrowing the original error) and in the catch block rethrow the caught error
(or throw a new Error with context) so callers of this transaction fetch routine
receive an exception (or alternatively modify the EventResponse contract to
include an error field and populate it) — change the branches around data.error
and the catch to throw rather than returning empty events to allow the UI to
surface real errors.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
#43 close
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.