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.
| 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})`); | |
| } |
| 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: |
a16fd33
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://343b8254.hyperterminal.pages.dev |
| Branch Preview URL: | https://faucet-update.hyperterminal.pages.dev |
1 participant
No description provided.