Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR enhances error handling and retry UX in the claim link flow by introducing manual retry capability with deferred NOT_FOUND errors, creating a unified ClaimErrorView component, expanding loading messaging based on retry counts, and adding configurability to the ValidationErrorView component. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
src/components/Claim/Generic/ClaimError.view.tsx (1)
9-14: Consider exportingClaimErrorViewPropsfor reusability.The type is currently private to this module. If other components need to reference or extend these props, exporting the type would improve maintainability.
src/components/Claim/Claim.tsx (1)
295-311: Clarify comment about retry UX.The comment on line 301 says "give user option to retry" but the code shows LOADING state during automatic retries (failureCount < 4). The manual retry button only appears after all automatic retries are exhausted (failureCount >= 4). Consider updating the comment to reflect this behavior more accurately.
- // Don't immediately show NOT_FOUND - give user option to retry - // Link might have just been created + // Don't immediately show NOT_FOUND during automatic retries + // Link might have just been created; after retries exhausted, show manual retry button if (failureCount >= 4) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Claim/Claim.tsx(7 hunks)src/components/Claim/Generic/ClaimError.view.tsx(1 hunks)src/components/Claim/Generic/NotFound.view.tsx(1 hunks)src/components/Claim/Generic/index.ts(1 hunks)src/components/Payment/Views/Error.validation.view.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-18T21:36:11.486Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#535
File: src/components/Claim/Claim.tsx:142-146
Timestamp: 2024-11-18T21:36:11.486Z
Learning: In `src/components/Claim/Claim.tsx`, external calls like token price fetching and cross-chain details retrieval are already encapsulated within existing `try...catch` blocks, so additional error handling may be unnecessary.
Applied to files:
src/components/Claim/Generic/ClaimError.view.tsxsrc/components/Claim/Claim.tsx
🧬 Code graph analysis (2)
src/components/Claim/Generic/ClaimError.view.tsx (2)
src/context/SupportModalContext.tsx (1)
useSupportModalContext(39-45)src/components/0_Bruddle/Button.tsx (1)
Button(76-267)
src/components/Claim/Claim.tsx (2)
src/components/Global/PeanutLoading/index.tsx (1)
PeanutLoading(4-19)src/components/Claim/Generic/ClaimError.view.tsx (1)
ClaimErrorView(16-47)
⏰ 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). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (6)
src/components/Payment/Views/Error.validation.view.tsx (1)
18-18: LGTM! Clean configurability enhancement.The addition of the optional
supportButtonTextprop with a sensible default maintains backward compatibility while allowing customization where needed.Also applies to: 28-28, 79-79
src/components/Claim/Generic/index.ts (1)
4-4: LGTM!The new export properly exposes the
ClaimErrorViewcomponent for use across the claim flow.src/components/Claim/Generic/NotFound.view.tsx (1)
9-9: LGTM! Improved user guidance.The updated message provides clearer context about potential issues and suggests retry action, aligning well with the new retry UX.
src/components/Claim/Claim.tsx (3)
83-96: LGTM! Good retry and refetch strategy.The addition of
refetchandfailureCountenables manual retry, whilerefetchOnWindowFocusandrefetchOnMountimprove UX for users who switch tabs or navigate away and return.
349-365: LGTM! Progressive loading messages improve UX.The conditional messaging provides clear feedback during extended loading:
- Initial load: spinner only
- Early retries (1-2): "Loading your link..."
- Extended retries (3-4): More detailed messaging with attempt counter
The attempt counter math is correct:
failureCount + 1properly shows the current attempt number.
406-427: Good retry UX with fresh attempt sets.Both error states provide manual retry buttons that reset the link state and call
refetch(), giving users a fresh set of automatic retries. This is helpful for transient errors (network issues, newly created links, etc.).Note: For WRONG_PASSWORD errors derived from URL params (lines 208, 232), retrying without changing the URL will reproduce the same error. This is acceptable UX since the retry is quick and users might modify the URL between attempts.
| <Button | ||
| onClick={() => { | ||
| openSupportWithMessage(`I clicked on this link but got an error: ${window.location.href}`) | ||
| }} |
There was a problem hiding this comment.
Guard window access for SSR safety.
Direct access to window.location.href can throw during server-side rendering or initial hydration in Next.js, even in client components.
Apply this diff to safely access the window object:
<Button
onClick={() => {
- openSupportWithMessage(`I clicked on this link but got an error: ${window.location.href}`)
+ openSupportWithMessage(`I clicked on this link but got an error: ${typeof window !== 'undefined' ? window.location.href : ''}`)
}}
size="medium"
shadowSize="4"
variant="stroke"
className="w-full"
>📝 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.
| <Button | |
| onClick={() => { | |
| openSupportWithMessage(`I clicked on this link but got an error: ${window.location.href}`) | |
| }} | |
| <Button | |
| onClick={() => { | |
| openSupportWithMessage(`I clicked on this link but got an error: ${typeof window !== 'undefined' ? window.location.href : ''}`) | |
| }} | |
| size="medium" | |
| shadowSize="4" | |
| variant="stroke" | |
| className="w-full" | |
| > |
🤖 Prompt for AI Agents
In src/components/Claim/Generic/ClaimError.view.tsx around lines 30 to 33, the
onClick handler directly reads window.location.href which can crash during
SSR/hydration; change it to use a safely-guarded value (e.g. const currentHref =
typeof window !== 'undefined' ? window.location.href : '' or compute it inside a
useEffect/handler after checking typeof window) and pass that safe currentHref
into openSupportWithMessage so the component never reads window on the server.
2 participants
No description provided.