Revamp/4 request page#443
Conversation
Restyle request detail (ActionBar, Appeal, Challenge, Evidence, Timeline, RequestIdentityCard, skeletons), claim wizard (Info, Photo, Video, Review, Finalized, Form stepper) and cross-chain sections to the new design system. Referral wiring in the claim flow is added later in the referral PR.
Write evidence attachments with fileURI for ERC-1497 interop, and use a 2-column challenge-reason grid on small screens.
- Introduced new accordion item animation in Tailwind configuration. - Updated global styles to ensure buttons do not wrap text and added uniform sizing for action buttons. - Refactored Challenge component to improve state management and user experience with new evidence form fields. - Implemented a new RequestModal for consistent modal behavior across the application. - Enhanced InfoTooltip component for better user interaction and information display. - Improved accessibility and styling in various components, including ActionButton, CurrencyField, and Progress. - Added skeleton loading states for evidence sections to enhance user feedback during data fetching.
…raction - Added VouchingActions component to streamline vouching and funding processes. - Integrated new modal components for better user experience during request actions. - Updated ActionBar to manage state for withdraw and appeal functionalities. - Improved loading states and error handling across various components. - Refactored existing components to utilize new hooks for better state management and user feedback.
✅ Deploy Preview for proof-of-humanity-v2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR refactors request actions into reusable modal-driven flows, adds appeal and evidence workflows, improves transaction error context, updates identity and timeline presentation, and extends shared UI components for loading, tooltips, accordions, funding, and status display. ChangesRequest workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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: 9
🧹 Nitpick comments (1)
src/app/[pohid]/[chain]/[request]/RequestIdentityCard.tsx (1)
270-272: 📐 Maintainability & Code Quality | 🔵 TrivialSolidity-style doc comment on a React component.
@notice/@devtags are Solidity NatSpec conventions; for a TS/React function, standard TSDoc (/** ... */with@param/@returns) would be more idiomatic and consistent with typical frontend tooling.
[optional_and_nitpick]🤖 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 `@src/app/`[pohid]/[chain]/[request]/RequestIdentityCard.tsx around lines 270 - 272, Update the documentation comment for the React component rendering the registration video to use standard TSDoc wording instead of Solidity-specific `@notice` and `@dev` tags. Preserve the existing description about waiting for identity data and reusing the media block across responsive layouts.
🤖 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.
Inline comments:
In `@src/app/`[pohid]/[chain]/[request]/ActionBar.tsx:
- Around line 152-164: Update the disabled condition on FundButton in ActionBar
so funding is disabled for all users whenever lockClaimed is true, while
preserving the existing enabled behavior when the humanity is not claimed and
the claimed tooltip handling.
In `@src/app/`[pohid]/[chain]/[request]/Appeal.tsx:
- Around line 299-347: The appeal deadline state in the query flow is stale and
does not gate funding after expiration. Update the snapshot and surrounding
Appeal component logic to retain the midpoint and final deadline timestamps,
schedule a state update or refetch when each boundary is reached while the page
remains open, and recompute deadline status; use the resulting state to disable
both funding forms after appealPeriodEnd while preserving midpoint-specific
losing-side restrictions.
In `@src/app/`[pohid]/[chain]/[request]/RequestEvidenceSection.tsx:
- Around line 80-85: The evidence rendering flow in RequestEvidenceSection.tsx
must render immediately with fallback identities instead of awaiting unbounded
profile enrichment; update EvidenceWithProfiles usage to enrich profiles
progressively. In src/data/evidence.ts lines 45-71, bound concurrent profile
lookups and cache results using normalized submitter addresses so repeated
identities reuse a single lookup.
In `@src/app/`[pohid]/[chain]/[request]/Vouch.tsx:
- Around line 254-257: Unify the pending/submission guard across the vouch flow
in the RequestModal usage and the related gasless, on-chain, close, and
Return-button handlers. Reuse one lock that blocks overlapping submissions and
prevents closing or returning until callbacks complete, and reset the lock and
submitted state when opening a new request so setSubmitted(true) cannot persist
stale success state.
- Around line 304-318: Replace the clickable span in the on-chain vouch action
with a native button, preserving its styling and “or vouch on chain” label. Bind
addVouch to the button’s onClick, use disabled={isOnchainLoading ||
isReconciling}, and retain the existing loading-dependent visual states without
manually relying on aria-disabled or aria-busy.
In `@src/components/Accordion.tsx`:
- Around line 127-132: Update the persisted content container in Accordion so it
becomes non-focusable immediately when open is false, rather than waiting for
the close animation or relying only on aria-hidden. Apply inert or the
component’s established equivalent focus guard while preserving the existing
mounting and visibility behavior controlled by hasOpened and isRendered.
In `@src/components/AddEvidenceModal.tsx`:
- Around line 161-179: Pass the expected chain ID into AddEvidenceModal and use
it in both the submit flow and the relevant tooltip. Before uploadEvidence runs
in submit, revalidate the connected wallet’s current chain against that expected
ID and return without uploading when it differs; keep the existing upload and
prepare behavior for a matching chain, and update the tooltip to communicate the
same chain requirement.
In `@src/components/InfoTooltip.tsx`:
- Around line 27-44: Update the InfoTooltip component’s visibility handling so
hover, focus, and click all derive from one shared visibility state, keeping
aria-expanded synchronized whenever the tooltip is shown. Assign the tooltip a
stable unique ID and reference it from the trigger via aria-describedby, while
ensuring hidden tooltip content is excluded from assistive technology when not
visible.
In `@src/contracts/hooks/useWagmiWrite.ts`:
- Around line 173-175: Update the "error" case in the write-status handling of
useWagmiWrite so it does not clear writeInFlightRef.current after a transaction
hash exists. Preserve the guard while receipt polling fails, and use the
existing hash to retry or reconcile the transaction before allowing another
non-idempotent write.
---
Nitpick comments:
In `@src/app/`[pohid]/[chain]/[request]/RequestIdentityCard.tsx:
- Around line 270-272: Update the documentation comment for the React component
rendering the registration video to use standard TSDoc wording instead of
Solidity-specific `@notice` and `@dev` tags. Preserve the existing description about
waiting for identity data and reusing the media block across responsive layouts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a7de5de-513b-4156-93da-d35766e695f2
⛔ Files ignored due to path filters (3)
src/icons/ArrowRight.svgis excluded by!**/*.svgsrc/icons/XdaiToken.svgis excluded by!**/*.svgsrc/icons/upload.svgis excluded by!**/*.svg
📒 Files selected for processing (41)
src/app/[pohid]/[chain]/[request]/ActionBar.tsxsrc/app/[pohid]/[chain]/[request]/Appeal.tsxsrc/app/[pohid]/[chain]/[request]/Challenge.tsxsrc/app/[pohid]/[chain]/[request]/Evidence.tsxsrc/app/[pohid]/[chain]/[request]/Funding.tsxsrc/app/[pohid]/[chain]/[request]/Info.tsxsrc/app/[pohid]/[chain]/[request]/RemoveVouch.tsxsrc/app/[pohid]/[chain]/[request]/RequestErrorState.tsxsrc/app/[pohid]/[chain]/[request]/RequestEvidenceSection.tsxsrc/app/[pohid]/[chain]/[request]/RequestIdentityCard.tsxsrc/app/[pohid]/[chain]/[request]/RequestLoadingSkeleton.tsxsrc/app/[pohid]/[chain]/[request]/RequestVouchSection.tsxsrc/app/[pohid]/[chain]/[request]/Timeline.tsxsrc/app/[pohid]/[chain]/[request]/TimelineSection.tsxsrc/app/[pohid]/[chain]/[request]/Vouch.tsxsrc/app/[pohid]/[chain]/[request]/error.tsxsrc/app/[pohid]/[chain]/[request]/page.tsxsrc/app/[pohid]/cross-chain/crossChainUi.tsxsrc/app/globals.csssrc/components/Accordion.tsxsrc/components/ActionButton.tsxsrc/components/AddEvidenceModal.tsxsrc/components/CopyButton.tsxsrc/components/CurrencyField.tsxsrc/components/EvidenceFormFields.tsxsrc/components/Field.tsxsrc/components/FileUploadZone.tsxsrc/components/IdentityReferenceRow.tsxsrc/components/InfoTooltip.tsxsrc/components/Progress.tsxsrc/components/Request/Grid.tsxsrc/components/Request/StatusBadge.tsxsrc/components/RequestModal.tsxsrc/components/RequestPunishedVouchNotice.tsxsrc/components/StatusIcon.tsxsrc/components/VideoThumbnail.tsxsrc/config/chains.tssrc/contracts/hooks/useWagmiWrite.tssrc/data/evidence.tssrc/hooks/useFundingAmount.tstailwind.config.cjs
…ponents - Added new mediaResolve animation to Tailwind configuration for improved visual effects. - Updated global styles to ensure consistent button sizing and layout on larger screens. - Refactored ActionBar and Vouch components to improve user interaction and loading states. - Enhanced Evidence and AddEvidenceModal components for better user feedback during evidence submission. - Improved accessibility and responsiveness in various components, including InfoTooltip and LoadableImage.
…proved user experience - Replaced inline modal content in Vouch component with a dedicated VouchModalContent component for better organization and readability. - Added new icons and improved layout for vouching methods, enhancing visual clarity and user interaction. - Implemented acknowledgment checkbox and method selection for vouching, streamlining the submission process. - Introduced new SVG icons for enhanced visual representation of vouching options.
… and RequestIdentityCard components - Simplified the layout in RemoveVouch by removing unnecessary div wrappers around the ActionButton. - Enhanced the fallback UI in RequestIdentityCard's Suspense components for better user experience during loading states. - Updated IPFS fetch function to include a timeout for improved error handling during data retrieval.
|
looks nice. just this I would add maybe 8px or 16px gap or whichever number looks better. |
|
@kemuru took the spacing from design here https://www.figma.com/design/3RAWp2mZC9zB2pKtoJ8jWu/POH-NEW?node-id=62-6415&t=sLtbzjrYh6jYhTHg-0 , I believe it was intentional |
…rience - Added detailed error context handling in various components, including Vouch, Appeal, Challenge, and Funding, to provide clearer feedback on transaction failures. - Updated GraphQL queries to include additional fields such as creationTime and submitter in the HistoricalWinnerClaim query for better data representation. - Enhanced CopyButton and ActionButton components with improved accessibility features and tooltip support for better user interaction. - Refactored the Vouch component to streamline modal handling and improve the overall user experience during vouching actions.
|
@coderabbitai review all |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
src/components/ActionButton.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding the
"use client"directive.Since this shared component relies on React hooks (
useState,useId), it fundamentally requires client-side execution. Adding the"use client"directive explicitly marks it as a client boundary, preventing Next.js from throwing build or runtime errors if a Server Component accidentally imports it directly.♻️ Proposed refactor
+"use client"; + import React, { useId, useState } from "react";🤖 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 `@src/components/ActionButton.tsx` at line 1, Add the "use client" directive at the top of the ActionButton module, before imports, so the component’s useState and useId hooks execute within a Next.js client boundary.
🤖 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.
Inline comments:
In `@src/app/`[pohid]/[chain]/[request]/Appeal.tsx:
- Around line 301-311: Update handleFundSuccess and closeAppealModal so a
successful funding action keeps the appeal funding controls locked until
currentChallenge/claimerFunds or challengerFunds reflects the new contribution.
Preserve the success state or add an optimistic funded-side
update/reconciliation flag across modal close and reopen, and clear it only
after indexed totals catch up; prevent another funding submission while
reconciliation is pending.
In `@src/app/`[pohid]/[chain]/[request]/Evidence.tsx:
- Around line 229-249: Update the evidence anchor IDs and matching hrefs in the
Evidence component, including the current and historical accordion sections, to
incorporate both pohId and requestIndex. Ensure each request instance generates
unique request-evidence-top and request-evidence-bottom anchors while preserving
the existing scroll-link behavior.
In `@src/app/`[pohid]/[chain]/[request]/RemoveVouch.tsx:
- Around line 116-126: Update the render condition in RemoveVouch so
ActionButton is rendered whenever web3Loaded is true, removing the
userChainId/chain.id and disabled gating. Preserve trigger.disabled and
trigger.tooltip so the button remains visible on the wrong chain and provides
the chain-switch guidance.
In `@src/app/`[pohid]/[chain]/[request]/RequestIdentityCard.tsx:
- Around line 146-150: Update the LoadableImage alt text in RequestIdentityCard
to use the displayed claimer name for the profile photo, with a generic fallback
when that name is unavailable; leave the existing fallbackLabel and image
behavior unchanged.
In `@src/app/`[pohid]/[chain]/[request]/RequestLoadingSkeleton.tsx:
- Around line 17-69: Reorder the skeleton children inside the main flex
container to match RequestIdentityCard’s mobile order: render the header,
request details, and profile aside in that sequence, while preserving the
desktop layout through the existing responsive classes. Update the surrounding
containers as needed so the profile content remains the aside on larger screens
without changing the skeleton’s visual placeholders.
In `@src/app/`[pohid]/[chain]/[request]/RequestVouchSection.tsx:
- Around line 112-117: Update the vouching-state labels to use “Potential
vouchers” while the request status is “vouching,” retaining “Vouched by” for
confirmed states. Apply this consistently in RequestVouchSection.tsx lines
112-117 and both fallback label sites in page.tsx lines 140 and 283, including
the existing optimistic indicator behavior where applicable.
In `@src/components/ActionButton.tsx`:
- Around line 110-117: Update the tooltip span in ActionButton to include
aria-hidden bound to the existing tooltipVisible state, so hidden tooltips are
excluded from the accessibility tree while visible tooltips remain accessible;
preserve the current styling and visibility behavior.
- Around line 81-84: Update the onClick handler in ActionButton to accept the
click event, preserve the isDisabled early return, and invoke the optional
onClick callback safely while forwarding that event object.
In `@src/components/InfoTooltip.tsx`:
- Around line 24-28: Update the outer wrapper in InfoTooltip to handle keydown
events and dismiss the tooltip when the Escape key is pressed, reusing the
existing hide handler. Preserve the current mouse enter and leave behavior and
ensure other keys remain unaffected.
In `@src/components/VideoThumbnail.tsx`:
- Around line 43-47: Update the video element in VideoThumbnail to use
metadata-only preloading instead of preload="auto", while preserving the
existing first-frame loading and error handlers.
In `@src/contracts/hooks/useWagmiWrite.ts`:
- Around line 188-197: Update the unknown receipt-failure handling in
useWagmiWrite.ts around the transactionError and onError path so it uses a
non-terminal callback, or otherwise supports a later onSuccess for the same
transaction hash; do not leave the hash unsettled after invoking a terminal
onError. Update the callback contract in src/contracts/hooks/types.ts lines
44-51 to keep terminal callbacks mutually exclusive only when unknown polling
failures no longer invoke onError.
- Around line 170-205: In the success case of the write status handler, inspect
receipt.status before constructing the success context or invoking
effectsRef.current?.onSuccess?. Treat a receipt with status "reverted" as an
error and route it through the existing reverted-error handling, while
preserving normal success behavior for successful receipts.
---
Nitpick comments:
In `@src/components/ActionButton.tsx`:
- Line 1: Add the "use client" directive at the top of the ActionButton module,
before imports, so the component’s useState and useId hooks execute within a
Next.js client boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e14b188d-2cd1-4a00-980f-745ddf4bcf7a
⛔ Files ignored due to path filters (9)
src/generated/graphql.tsis excluded by!**/generated/**src/icons/AlertTriangle.svgis excluded by!**/*.svgsrc/icons/ArrowRight.svgis excluded by!**/*.svgsrc/icons/ChainLink.svgis excluded by!**/*.svgsrc/icons/FileText.svgis excluded by!**/*.svgsrc/icons/Signature.svgis excluded by!**/*.svgsrc/icons/Users.svgis excluded by!**/*.svgsrc/icons/XdaiToken.svgis excluded by!**/*.svgsrc/icons/upload.svgis excluded by!**/*.svg
📒 Files selected for processing (53)
schemas/historicalWinnerClaim.gqlsrc/app/[pohid]/(profile)/page.tsxsrc/app/[pohid]/RevokeModal.tsxsrc/app/[pohid]/[chain]/[request]/ActionBar.tsxsrc/app/[pohid]/[chain]/[request]/Appeal.tsxsrc/app/[pohid]/[chain]/[request]/Challenge.tsxsrc/app/[pohid]/[chain]/[request]/Evidence.tsxsrc/app/[pohid]/[chain]/[request]/Funding.tsxsrc/app/[pohid]/[chain]/[request]/Info.tsxsrc/app/[pohid]/[chain]/[request]/RemoveVouch.tsxsrc/app/[pohid]/[chain]/[request]/RequestErrorState.tsxsrc/app/[pohid]/[chain]/[request]/RequestEvidenceSection.tsxsrc/app/[pohid]/[chain]/[request]/RequestIdentityCard.tsxsrc/app/[pohid]/[chain]/[request]/RequestLoadingSkeleton.tsxsrc/app/[pohid]/[chain]/[request]/RequestVouchSection.tsxsrc/app/[pohid]/[chain]/[request]/Timeline.tsxsrc/app/[pohid]/[chain]/[request]/TimelineSection.tsxsrc/app/[pohid]/[chain]/[request]/Vouch.tsxsrc/app/[pohid]/[chain]/[request]/VouchModalContent.tsxsrc/app/[pohid]/[chain]/[request]/error.tsxsrc/app/[pohid]/[chain]/[request]/page.tsxsrc/app/[pohid]/claim/Form.tsxsrc/app/[pohid]/cross-chain/PendingRelaySection.tsxsrc/app/[pohid]/cross-chain/TransferSection.tsxsrc/app/[pohid]/cross-chain/UpdateStateSection.tsxsrc/app/[pohid]/cross-chain/crossChainUi.tsxsrc/app/globals.csssrc/components/Accordion.tsxsrc/components/ActionButton.tsxsrc/components/AddEvidenceModal.tsxsrc/components/CopyButton.tsxsrc/components/CurrencyField.tsxsrc/components/EvidenceFormFields.tsxsrc/components/Field.tsxsrc/components/FileUploadZone.tsxsrc/components/IdentityReferenceRow.tsxsrc/components/InfoTooltip.tsxsrc/components/LoadableImage.tsxsrc/components/Progress.tsxsrc/components/Request/Grid.tsxsrc/components/Request/StatusBadge.tsxsrc/components/RequestModal.tsxsrc/components/RequestPunishedVouchNotice.tsxsrc/components/StatusIcon.tsxsrc/components/VideoThumbnail.tsxsrc/config/chains.tssrc/contracts/hooks/types.tssrc/contracts/hooks/useWagmiWrite.tssrc/data/evidence.tssrc/hooks/useActionFeedback.tssrc/hooks/useFundingAmount.tssrc/utils/ipfs.tstailwind.config.cjs
| const handleFundSuccess = (amount: bigint, txHash?: Hash) => { | ||
| setFundSuccess({ amount, txHash }); | ||
| }; | ||
|
|
||
| const closeAppealModal = () => { | ||
| setAppealModalOpen(false); | ||
| router.refresh(); | ||
| setFundSuccess(null); | ||
| if (fundSuccess) { | ||
| void queryClient.invalidateQueries({ queryKey: appealSnapshotQueryKey }); | ||
| router.refresh(); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep appeal funding locked until the indexed totals catch up.
After success, closing clears fundSuccess, while claimerFunds and challengerFunds still come from the potentially stale subgraph currentChallenge. Reopening can therefore submit another funding transaction against outdated remaining amounts. Optimistically update the funded side or retain a reconciliation lock until currentChallenge reflects the contribution.
Also applies to: 428-547
🤖 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 `@src/app/`[pohid]/[chain]/[request]/Appeal.tsx around lines 301 - 311, Update
handleFundSuccess and closeAppealModal so a successful funding action keeps the
appeal funding controls locked until currentChallenge/claimerFunds or
challengerFunds reflects the new contribution. Preserve the success state or add
an optimistic funded-side update/reconciliation flag across modal close and
reopen, and clear it only after indexed totals catch up; prevent another funding
submission while reconciliation is pending.
| <div | ||
| className="mb-4 mt-4 flex animate-accordionItemIn flex-col items-center gap-3" | ||
| id="request-evidence-top" | ||
| > | ||
| <ActionButton | ||
| disabled={evidenceTrigger.disabled} | ||
| onClick={() => setModalOpen(true)} | ||
| label="Add Evidence" | ||
| tooltip={evidenceTrigger.tooltip} | ||
| className="w-[min(100%,10.625rem)]" | ||
| /> | ||
| {evidenceItems.length > 1 && | ||
| !lastEvidenceVisible && | ||
| !isAnimating && ( | ||
| <a | ||
| className="text-sm text-peach" | ||
| href="#request-evidence-bottom" | ||
| > | ||
| Scroll to oldest evidence ↓ | ||
| </a> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the evidence anchor IDs instance-specific.
Current and historical evidence accordions can coexist, causing duplicate request-evidence-top/request-evidence-bottom IDs and shortcuts that scroll to the wrong section. Include pohId and requestIndex in both IDs and matching hrefs.
Also applies to: 270-270, 289-297
🤖 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 `@src/app/`[pohid]/[chain]/[request]/Evidence.tsx around lines 229 - 249,
Update the evidence anchor IDs and matching hrefs in the Evidence component,
including the current and historical accordion sections, to incorporate both
pohId and requestIndex. Ensure each request instance generates unique
request-evidence-top and request-evidence-bottom anchors while preserving the
existing scroll-link behavior.
| return ( | ||
| web3Loaded && | ||
| (userChainId === chain.id || disabled) && ( | ||
| <div className="flex gap-4"> | ||
| <ActionButton | ||
| onClick={removeOnchainVouch} | ||
| label="Remove Vouch" | ||
| className="mb-2 w-auto" | ||
| isLoading={status.write === "pending"} | ||
| disabled={trigger.disabled} | ||
| tooltip={trigger.tooltip} | ||
| /> | ||
| </div> | ||
| <ActionButton | ||
| onClick={removeOnchainVouch} | ||
| label="Remove Vouch" | ||
| className="mb-2 w-auto" | ||
| isLoading={isRemoveVouchLoading} | ||
| disabled={trigger.disabled} | ||
| tooltip={trigger.tooltip} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep “Remove Vouch” visible on the wrong chain.
The render condition hides the button when userChainId !== chain.id, making the wrong-chain trigger and tooltip unreachable. Render it whenever Web3 is loaded and let trigger.disabled provide the chain-switch guidance.
Proposed fix
return (
- web3Loaded &&
- (userChainId === chain.id || disabled) && (
+ web3Loaded && (
<ActionButton📝 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.
| return ( | |
| web3Loaded && | |
| (userChainId === chain.id || disabled) && ( | |
| <div className="flex gap-4"> | |
| <ActionButton | |
| onClick={removeOnchainVouch} | |
| label="Remove Vouch" | |
| className="mb-2 w-auto" | |
| isLoading={status.write === "pending"} | |
| disabled={trigger.disabled} | |
| tooltip={trigger.tooltip} | |
| /> | |
| </div> | |
| <ActionButton | |
| onClick={removeOnchainVouch} | |
| label="Remove Vouch" | |
| className="mb-2 w-auto" | |
| isLoading={isRemoveVouchLoading} | |
| disabled={trigger.disabled} | |
| tooltip={trigger.tooltip} | |
| /> | |
| return ( | |
| web3Loaded && ( | |
| <ActionButton | |
| onClick={removeOnchainVouch} | |
| label="Remove Vouch" | |
| className="mb-2 w-auto" | |
| isLoading={isRemoveVouchLoading} | |
| disabled={trigger.disabled} | |
| tooltip={trigger.tooltip} | |
| /> |
🤖 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 `@src/app/`[pohid]/[chain]/[request]/RemoveVouch.tsx around lines 116 - 126,
Update the render condition in RemoveVouch so ActionButton is rendered whenever
web3Loaded is true, removing the userChainId/chain.id and disabled gating.
Preserve trigger.disabled and trigger.tooltip so the button remains visible on
the wrong chain and provides the chain-switch guidance.
| <LoadableImage | ||
| className="h-32 w-32 cursor-pointer rounded-full object-cover md:bg-cover md:bg-center md:bg-no-repeat" | ||
| alt="image" | ||
| fallbackLabel="Profile photo unavailable" | ||
| src={photoUrl} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Give the profile photo meaningful alt text.
alt="image" conveys no identity or purpose. Use the displayed claimer name, with a generic fallback.
Proposed fix
- alt="image"
+ alt={
+ displayedClaimerName
+ ? `${displayedClaimerName} profile photo`
+ : "Profile photo"
+ }📝 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.
| <LoadableImage | |
| className="h-32 w-32 cursor-pointer rounded-full object-cover md:bg-cover md:bg-center md:bg-no-repeat" | |
| alt="image" | |
| fallbackLabel="Profile photo unavailable" | |
| src={photoUrl} | |
| <LoadableImage | |
| className="h-32 w-32 cursor-pointer rounded-full object-cover md:bg-cover md:bg-center md:bg-no-repeat" | |
| alt={ | |
| displayedClaimerName | |
| ? `${displayedClaimerName} profile photo` | |
| : "Profile photo" | |
| } | |
| fallbackLabel="Profile photo unavailable" | |
| src={photoUrl} |
🤖 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 `@src/app/`[pohid]/[chain]/[request]/RequestIdentityCard.tsx around lines 146 -
150, Update the LoadableImage alt text in RequestIdentityCard to use the
displayed claimer name for the profile photo, with a generic fallback when that
name is unavailable; leave the existing fallbackLabel and image behavior
unchanged.
| {/* Identity card */} | ||
| <div className="border-stroke bg-whiteBackground mb-1 overflow-hidden rounded-card border shadow-soft-inset"> | ||
| <div className="flex flex-col md:flex-row"> | ||
| {/* Profile aside */} | ||
| <div className="border-stroke flex animate-pulse flex-col items-center gap-4 border-b px-6 py-6 md:w-[26%] md:min-w-[16rem] md:shrink-0 md:border-b-0 md:border-r"> | ||
| <div className="bg-grey h-32 w-32 rounded-full" /> | ||
| <div className="bg-grey h-7 w-40 rounded" /> | ||
| <div className="bg-grey h-4 w-52 max-w-full rounded" /> | ||
| <div className="bg-grey h-4 w-44 max-w-full rounded" /> | ||
| <div className="border-stroke mt-4 flex w-full flex-col items-center gap-3 border-y py-4"> | ||
| <div className="bg-grey h-3 w-20 rounded" /> | ||
| <div className="flex gap-2"> | ||
| <div className="bg-grey h-8 w-8 rounded-full" /> | ||
| <div className="bg-grey h-8 w-8 rounded-full" /> | ||
| <div className="bg-grey h-8 w-8 rounded-full" /> | ||
| </div> | ||
| </div> | ||
| <div className="bg-grey mt-2 h-3 w-28 rounded" /> | ||
| </div> | ||
|
|
||
| <div className="bg-grey aspect-video w-full rounded" /> | ||
| <div className="bg-grey mt-3 h-4 w-52 rounded" /> | ||
|
|
||
| <div className="mt-8 flex flex-wrap gap-2"> | ||
| <div className="bg-grey h-9 w-28 rounded" /> | ||
| <div className="bg-grey h-9 w-28 rounded" /> | ||
| <div className="bg-grey h-9 w-28 rounded" /> | ||
| {/* Request details */} | ||
| <div className="flex w-full min-w-0 animate-pulse flex-col gap-4 p-6 lg:p-8"> | ||
| <div className="bg-grey h-[8.25rem] w-full rounded-input md:h-12" /> | ||
| <div className="bg-grey h-12 w-full rounded-input" /> | ||
| <div className="flex justify-center"> | ||
| <div className="bg-grey h-4 w-32 rounded" /> | ||
| </div> | ||
| <div className="bg-grey aspect-[1.8] w-full rounded-2xl" /> | ||
| <div className="flex justify-center md:justify-end"> | ||
| <div className="bg-grey h-4 w-28 rounded" /> | ||
| </div> | ||
| <div className="border-stroke border-t pt-6"> | ||
| <div className="bg-grey h-5 w-24 rounded" /> | ||
| <div className="mt-6 flex flex-col gap-6"> | ||
| {[0, 1, 2].map((index) => ( | ||
| <div key={index} className="flex gap-4"> | ||
| <div className="flex w-6 shrink-0 flex-col items-center"> | ||
| <div className="bg-grey h-5 w-5 rounded-full" /> | ||
| {index < 2 && ( | ||
| <div className="bg-grey mt-2 h-10 w-px" /> | ||
| )} | ||
| </div> | ||
| <div className="min-w-0 flex-1"> | ||
| <div className="bg-grey h-4 w-40 max-w-full rounded" /> | ||
| <div className="bg-grey mt-2 h-3 w-24 rounded" /> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Mirror the resolved card’s mobile ordering to avoid layout shift.
The skeleton renders the complete profile aside first, while RequestIdentityCard uses contents and ordered children to render header → request info → profile on mobile. Match those orders in the skeleton to avoid a large shift when Suspense resolves.
🤖 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 `@src/app/`[pohid]/[chain]/[request]/RequestLoadingSkeleton.tsx around lines 17
- 69, Reorder the skeleton children inside the main flex container to match
RequestIdentityCard’s mobile order: render the header, request details, and
profile aside in that sequence, while preserving the desktop layout through the
existing responsive classes. Update the surrounding containers as needed so the
profile content remains the aside on larger screens without changing the
skeleton’s visual placeholders.
| <span | ||
| id={tooltipId} | ||
| role="tooltip" | ||
| className={twMerge( | ||
| "tooltip-surface pointer-events-none absolute bottom-full left-1/2 z-50 mb-2 w-max max-w-[240px] -translate-x-1/2 text-center text-sm transition-opacity", | ||
| tooltipVisible ? "opacity-100" : "opacity-0", | ||
| )} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Hide invisible tooltip content from assistive technology.
Using opacity-0 visually hides the tooltip, but it remains present in the DOM's accessibility tree. Adding aria-hidden={!tooltipVisible} prevents screen readers from discovering and reading the tooltip content when navigating the document normally, aligning with the accessible pattern implemented in InfoTooltip.
♿ Proposed accessibility fix
<span
id={tooltipId}
role="tooltip"
+ aria-hidden={!tooltipVisible}
className={twMerge(
"tooltip-surface pointer-events-none absolute bottom-full left-1/2 z-50 mb-2 w-max max-w-[240px] -translate-x-1/2 text-center text-sm transition-opacity",
tooltipVisible ? "opacity-100" : "opacity-0",
)}
>📝 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.
| <span | |
| id={tooltipId} | |
| role="tooltip" | |
| className={twMerge( | |
| "tooltip-surface pointer-events-none absolute bottom-full left-1/2 z-50 mb-2 w-max max-w-[240px] -translate-x-1/2 text-center text-sm transition-opacity", | |
| tooltipVisible ? "opacity-100" : "opacity-0", | |
| )} | |
| > | |
| <span | |
| id={tooltipId} | |
| role="tooltip" | |
| aria-hidden={!tooltipVisible} | |
| className={twMerge( | |
| "tooltip-surface pointer-events-none absolute bottom-full left-1/2 z-50 mb-2 w-max max-w-[240px] -translate-x-1/2 text-center text-sm transition-opacity", | |
| tooltipVisible ? "opacity-100" : "opacity-0", | |
| )} | |
| > |
🤖 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 `@src/components/ActionButton.tsx` around lines 110 - 117, Update the tooltip
span in ActionButton to include aria-hidden bound to the existing tooltipVisible
state, so hidden tooltips are excluded from the accessibility tree while visible
tooltips remain accessible; preserve the current styling and visibility
behavior.
| <span | ||
| className={cn("relative inline-flex", className)} | ||
| onMouseEnter={show} | ||
| onMouseLeave={hide} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add an Escape key handler to dismiss the tooltip.
WCAG 1.4.13 requires that content triggered by hover or focus be dismissible without moving the pointer or shifting focus. Adding an onKeyDown handler to the outer wrapper ensures keyboard users can close the tooltip using the Escape key, maintaining consistency with the pattern implemented in ActionButton.
♿ Proposed accessibility fix
<span
className={cn("relative inline-flex", className)}
onMouseEnter={show}
onMouseLeave={hide}
+ onKeyDown={(e) => {
+ if (e.key === "Escape") hide();
+ }}
>📝 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.
| <span | |
| className={cn("relative inline-flex", className)} | |
| onMouseEnter={show} | |
| onMouseLeave={hide} | |
| > | |
| <span | |
| className={cn("relative inline-flex", className)} | |
| onMouseEnter={show} | |
| onMouseLeave={hide} | |
| onKeyDown={(e) => { | |
| if (e.key === "Escape") hide(); | |
| }} | |
| > |
🤖 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 `@src/components/InfoTooltip.tsx` around lines 24 - 28, Update the outer
wrapper in InfoTooltip to handle keydown events and dismiss the tooltip when the
Escape key is pressed, reusing the existing hide handler. Preserve the current
mouse enter and leave behavior and ensure other keys remain unaffected.
| src={videoSrc} | ||
| preload="metadata" | ||
| preload="auto" | ||
| playsInline | ||
| onLoadedData={() => setIsLoading(false)} | ||
| onError={() => { | ||
| setIsLoading(false); | ||
| setHasError(true); | ||
| }} | ||
| onError={() => setHasError(true)} |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
According to WHATWG HTML and MDN, how may browsers load video data for preload="auto" versus preload="metadata"?
💡 Result:
According to the WHATWG HTML specification and MDN documentation, the preload attribute is an enumerated attribute that serves as a hint to the browser regarding how much video data should be buffered upon page load [1][2][3]. Because it is a hint, browsers are not strictly required to follow it, and implementation behavior can vary based on the browser, its configuration, and network conditions [3][4][5]. The two values function as follows: preload="metadata": This indicates that the browser should fetch only the video's metadata—such as dimensions, track list, duration, and potentially the first few frames—rather than the entire file [2][3][4]. The specification notes that while the goal is to stop at metadata, browsers may sometimes fetch additional frames as well [2]. preload="auto": This signals that the browser may optimistically download the entire video file, even if playback is not immediately expected [3][4]. The browser aims to cache enough data so that playback can proceed without requiring pauses for further buffering [2][4]. Important considerations regarding these hints: 1. Precedence: The autoplay attribute takes precedence over the preload attribute [2][3][5]. If autoplay is specified, the browser will naturally begin buffering the video to enable playback, regardless of the preload hint [2][3]. 2. Browser-Specific Implementation: Browser behavior is not uniform [6]. For example, a browser might force preload="none" under specific conditions, such as when Data Saver mode is enabled or on certain cellular connections, regardless of the author's specified hint [4]. 3. Default Behavior: The specification advises that the default value for the preload attribute be set to metadata [3][7]. However, as noted in the MDN documentation, the actual default can vary between browsers [3]. If a video element has the loading="lazy" attribute applied, that attribute takes precedence over the preload attribute, deferring any data fetching until the video is near or within the viewport [1][3][8].
Citations:
- 1: https://html.spec.whatwg.org/multipage/media.html
- 2: https://html.spec.whatwg.org/dev/media.html
- 3: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/video
- 4: https://web.dev/articles/fast-playback-with-preload
- 5: https://www.w3schools.com/tags/att_video_preload.asp
- 6: https://www.stevesouders.com/blog/2013/04/12/html5-video-preload/
- 7: https://stackoverflow.com/questions/39360088/does-the-metadata-preload-attribute-in-html5-load-the-entire-video
- 8: http://www.w3.org/html/wg/drafts/html/master/media.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'src/components/VideoThumbnail.tsx' 'src/components/*' | sed -n '1,120p'
echo '---'
ast-grep outline src/components/VideoThumbnail.tsx --view expanded
echo '---'
cat -n src/components/VideoThumbnail.tsx | sed -n '1,220p'Repository: Proof-Of-Humanity/proof-of-humanity-v2-web
Length of output: 4893
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- VideoThumbnail usages ---'
rg -n "VideoThumbnail" src
echo '--- MediaFallback ---'
ast-grep outline src/components/MediaFallback.tsx --view expanded
echo '--- file ---'
cat -n src/components/MediaFallback.tsx | sed -n '1,220p'Repository: Proof-Of-Humanity/proof-of-humanity-v2-web
Length of output: 1595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- RequestIdentityCard excerpt ---'
cat -n src/app/[pohid]/[chain]/[request]/RequestIdentityCard.tsx | sed -n '250,330p'
echo '--- video-related patterns ---'
rg -n 'preload="|<video|`#t`=0\.001|onLoadedData|onError' src/components src/appRepository: Proof-Of-Humanity/proof-of-humanity-v2-web
Length of output: 8904
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/components/Previewed.tsx | sed -n '1,220p'Repository: Proof-Of-Humanity/proof-of-humanity-v2-web
Length of output: 4085
Use metadata preload for the thumbnail src/components/VideoThumbnail.tsx:37-47 — this only needs the first frame, and the preview modal already loads the full video on demand. preload="auto" can pull the entire IPFS asset during page render.
Proposed fix
- preload="auto"
+ preload="metadata"📝 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.
| src={videoSrc} | |
| preload="metadata" | |
| preload="auto" | |
| playsInline | |
| onLoadedData={() => setIsLoading(false)} | |
| onError={() => { | |
| setIsLoading(false); | |
| setHasError(true); | |
| }} | |
| onError={() => setHasError(true)} | |
| src={videoSrc} | |
| preload="metadata" | |
| playsInline | |
| onLoadedData={() => setIsLoading(false)} | |
| onError={() => setHasError(true)} |
🤖 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 `@src/components/VideoThumbnail.tsx` around lines 43 - 47, Update the video
element in VideoThumbnail to use metadata-only preloading instead of
preload="auto", while preserving the existing first-frame loading and error
handlers.
| case "success": { | ||
| if (lastSettledHashRef.current === txHash) break; | ||
| lastSettledHashRef.current = txHash; | ||
| writeInFlightRef.current = false; | ||
| const ctx: WriteSuccessContext = { | ||
| contract, | ||
| functionName: String(functionName), | ||
| args: lastWriteRef.current?.args, | ||
| value: lastWriteRef.current?.value, | ||
| chainId: lastWriteRef.current?.chainId ?? currentChainId, | ||
| // A sped-up (repriced) tx confirms under a different hash, and the | ||
| // wait resolves with that replacement receipt. | ||
| txHash: receipt?.transactionHash ?? txHash, | ||
| receipt, | ||
| }; | ||
| effectsRef.current?.onSuccess?.(ctx); | ||
| break; | ||
| } | ||
| case "error": { | ||
| if (transactionError instanceof BaseError) { | ||
| if (lastUnknownHashRef.current !== txHash) { | ||
| lastUnknownHashRef.current = txHash; | ||
| effectsRef.current?.onError?.(transactionError, { | ||
| kind: "unknown", | ||
| txHash, | ||
| }); | ||
| } | ||
| break; | ||
| } | ||
| if (lastSettledHashRef.current === txHash) break; | ||
| lastSettledHashRef.current = txHash; | ||
| writeInFlightRef.current = false; | ||
| effectsRef.current?.onError?.(transactionError, { | ||
| kind: "reverted", | ||
| txHash, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '"viem"|viem@' package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || true
curl -fsSL \
https://raw.githubusercontent.com/wevm/viem/v2.34.0/src/actions/public/waitForTransactionReceipt.ts |
rg -n -C3 'status|BaseError|revert'Repository: Proof-Of-Humanity/proof-of-humanity-v2-web
Length of output: 425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant hook implementation.
sed -n '130,230p' src/contracts/hooks/useWagmiWrite.ts
# Inspect viem receipt/error typing for the installed version.
node - <<'JS'
const fs = require('fs');
const path = require('path');
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log('viem version in package.json:', pkg.dependencies?.viem || pkg.devDependencies?.viem || 'not found');
const lock = fs.readFileSync('yarn.lock', 'utf8');
for (const line of lock.split('\n')) {
if (line.startsWith('viem@2.34.0') || line.startsWith('viem@>=2.33.3') || line.startsWith('viem@^2.31.7')) {
console.log(line);
}
}
JS
# Search for all status checks and BaseError handling in the hook.
rg -n 'receipt\.status|BaseError|onSuccess|onError|waitForTransactionReceipt' src/contracts/hooks/useWagmiWrite.tsRepository: Proof-Of-Humanity/proof-of-humanity-v2-web
Length of output: 4032
🌐 Web query:
viem v2.34.0 waitForTransactionReceipt receipt.status reverted success branch BaseError
💡 Result:
In Viem, the waitForTransactionReceipt action is designed to return a transaction receipt upon the successful inclusion of a transaction in a block [1][2]. Regarding reverted transactions, while some earlier documentation or user expectations suggested the action should throw an error when a transaction reverts [3][4][5], the current implementation returns the transaction receipt object, which includes a status field of 'reverted' [3][4]. There is a dedicated error class named TransactionReceiptRevertedError that inherits from BaseError, which is defined in the Viem source code [6]. However, this error is not automatically thrown by the standard waitForTransactionReceipt action simply because a transaction reverts; rather, the action returns the receipt, allowing developers to inspect the status manually [3][4][7]. If you need to handle reverted transactions, you should check the status property of the returned receipt object [7]: const receipt = await publicClient.waitForTransactionReceipt({ hash }); if (receipt.status === 'reverted') { // Handle the revert, for example by throwing a custom error throw new TransactionReceiptRevertedError({ receipt }); } This approach is consistent with the library's design, which separates the act of waiting for receipt confirmation from the interpretation of the transaction's outcome [7]. For developers who require earlier detection of revert reasons, using simulateContract before executing a transaction is the recommended practice to identify potential failures without incurring gas costs for a transaction that is likely to revert [6][7].
Citations:
- 1: https://viem.sh/docs/actions/public/waitForTransactionReceipt
- 2: https://github.com/wevm/viem/blob/a59b5630/src/actions/public/waitForTransactionReceipt.ts
- 3: waitForTransactionReceipt doesn't throw on Revert wevm/viem#3290
- 4: bug: waitForTransactionReceipt does not throw on reverted transaction wevm/viem#1767
- 5: https://v1.viem.sh/docs/actions/public/waitForTransactionReceipt.html
- 6: https://github.com/wevm/viem/blob/a59b5630/src/errors/transaction.ts
- 7: Should contract revert errors be thrown on waitForTransactionReceipt rather than writeContract wevm/viem#3890
Check receipt.status before calling onSuccess. BaseError only covers thrown failures; a mined receipt with status: "reverted" still reaches the success branch and gets reported as success.
🤖 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 `@src/contracts/hooks/useWagmiWrite.ts` around lines 170 - 205, In the success
case of the write status handler, inspect receipt.status before constructing the
success context or invoking effectsRef.current?.onSuccess?. Treat a receipt with
status "reverted" as an error and route it through the existing reverted-error
handling, while preserving normal success behavior for successful receipts.
| case "error": { | ||
| if (transactionError instanceof BaseError) { | ||
| if (lastUnknownHashRef.current !== txHash) { | ||
| lastUnknownHashRef.current = txHash; | ||
| effectsRef.current?.onError?.(transactionError, { | ||
| kind: "unknown", | ||
| txHash, | ||
| }); | ||
| } | ||
| break; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Unknown receipt failures violate the terminal callback contract.
The hook reports onError while deliberately leaving the hash unsettled, allowing a later onSuccess for the same transaction.
src/contracts/hooks/useWagmiWrite.ts#L188-L197: route unknown receipt failures through a non-terminal callback, or explicitly support a later success.src/contracts/hooks/types.ts#L44-L51: only promise mutually exclusive terminal callbacks if unknown polling failures no longer useonError.
📍 Affects 2 files
src/contracts/hooks/useWagmiWrite.ts#L188-L197(this comment)src/contracts/hooks/types.ts#L44-L51
🤖 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 `@src/contracts/hooks/useWagmiWrite.ts` around lines 188 - 197, Update the
unknown receipt-failure handling in useWagmiWrite.ts around the transactionError
and onError path so it uses a non-terminal callback, or otherwise supports a
later onSuccess for the same transaction hash; do not leave the hash unsettled
after invoking a terminal onError. Update the callback contract in
src/contracts/hooks/types.ts lines 44-51 to keep terminal callbacks mutually
exclusive only when unknown polling failures no longer invoke onError.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Style