Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the faucet claim process by integrating Privy authentication, removing the Turnstile captcha, and updating the UI. It also updates the default market to 'VOLX-USDH' and adds an effect to the index page to enforce this selection. Feedback includes a suggestion to improve error handling in the faucet claim hook to avoid JSON parsing errors on non-JSON responses, a concern regarding the lack of user guidance when the claim button is disabled due to authentication status, and a warning that the forced market selection in the index page overrides user preferences.
| </div> | ||
|
|
||
| <Button variant="contained" onClick={handleClaim} disabled={!turnstileToken} className="w-full"> | ||
| <Button variant="contained" onClick={handleClaim} disabled={!authenticated} className="w-full"> |
There was a problem hiding this comment.
The 'Claim' button is disabled when the user is not authenticated via Privy (!authenticated), but there is no visual feedback or call-to-action explaining why. A user who has connected their wallet via wagmi but hasn't logged into Privy might find this confusing. Consider updating the button text to 'Login to Claim' or providing a brief message to guide the user when they are not authenticated.
src/lib/faucet/use-faucet-claim.ts
Outdated
| const data = await res.json(); | ||
|
|
||
| if (!res.ok) { | ||
| throw new Error(data.error || `Claim failed (${res.status})`); | ||
| } |
There was a problem hiding this comment.
The response is parsed as JSON before checking res.ok. If the API returns a non-JSON error (e.g., an HTML error page from a load balancer or proxy), res.json() will throw a syntax error, masking the actual HTTP error with an unhelpful message like "Unexpected token < in JSON...". It is safer to handle potential parsing failures or check the status code first to provide a more meaningful error message.
| const data = await res.json(); | |
| if (!res.ok) { | |
| throw new Error(data.error || `Claim failed (${res.status})`); | |
| } | |
| const data = await res.json().catch(() => ({})); | |
| if (!res.ok) { | |
| throw new Error(data.error || `Claim failed (${res.status})`); | |
| } |
src/routes/index.tsx
Outdated
| useEffect(() => { | ||
| setSelectedMarket("all", "VOLX-USDH"); | ||
| }, [setSelectedMarket]); |
There was a problem hiding this comment.
This useEffect forces the selected market to VOLX-USDH every time the index page mounts, which overrides the user's persisted market preference from useMarketStore. This breaks the expected behavior where the application remembers the last market the user was viewing. Since DEFAULT_SELECTED_MARKETS has already been updated to VOLX-USDH in src/domain/market/scope.ts, new users will already see it by default. Forcing it here on every mount is likely redundant and negatively impacts the experience for returning users.
Deploying hyperterminal with
|
| Latest commit: |
9bf56c0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7784a59d.hyperterminal.pages.dev |
| Branch Preview URL: | https://faucet-update.hyperterminal.pages.dev |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the faucet system, replacing Turnstile captcha with Privy authentication and updating the claim logic to interface with a new backend API. The claim amount has been adjusted to 50 USDH, and the UI has been streamlined by removing the multi-step progress indicator and balance requirements. Other changes include updating the default market to VOLX-USDH and replacing navigation icons with brand images. Feedback suggests enhancing the authentication flow by allowing users to log in directly within the faucet modal and adding validation for the API response data to ensure state consistency.
| const open = useFaucetModalOpen(); | ||
| const { close } = useFaucetModalActions(); | ||
| const { address } = useConnection(); | ||
| const { authenticated, getAccessToken } = usePrivy(); |
There was a problem hiding this comment.
Destructure the login function from usePrivy to allow users to authenticate directly from the faucet modal if they are not already logged in.
| const { authenticated, getAccessToken } = usePrivy(); | |
| const { authenticated, getAccessToken, login } = usePrivy(); |
| <Button variant="contained" onClick={handleClaim} disabled={!authenticated} className="w-full"> | ||
| <DropIcon className="size-4" /> | ||
| <Trans>Claim 1,000 USDH</Trans> | ||
| <Trans>Claim 50 USDH</Trans> | ||
| </Button> |
There was a problem hiding this comment.
Instead of disabling the button when the user is not authenticated, it's better to provide a 'Login to Claim' action. This improves the user experience by allowing them to complete the authentication flow without leaving the modal.
<Button
variant="contained"
onClick={authenticated ? handleClaim : login}
className="w-full"
>
<DropIcon className="size-4" />
{authenticated ? <Trans>Claim 50 USDH</Trans> : <Trans>Login to Claim</Trans>}
</Button>
| setResult({ amount: claimData.amount || "1,000", txHash: claimData.txHash }); | ||
| const data = await res.json(); | ||
|
|
||
| setResult({ amount: data.amount, walletAddress: data.walletAddress }); |
There was a problem hiding this comment.
It's safer to validate the API response before updating the state. This ensures that the application doesn't end up with an invalid or incomplete result object if the backend returns unexpected data.
if (!data || typeof data.amount !== "number" || !data.walletAddress) {
throw new Error("Invalid response from faucet API");
}
setResult({ amount: data.amount, walletAddress: data.walletAddress });
1 participant
No description provided.