fix(pos): improve payment error feedback - #612
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Claude finished @ignaciosantise's task in 1m 53s —— View job Code Review
Found 1 issue(s)Issue 1: Toast may fire repeatedly on successive React Query retriesID: activity-toast-retry-dedupe-a3f1 Context:
Recommendation: Gate the toast on a stable derived value so it fires at most once per error event: // Use error.message (primitive) instead of the error object to avoid
// firing on new-reference-but-same-message retries.
useEffect(() => {
if (isError && error?.message && !isEmpty) {
showErrorToast(
"We couldn't refresh payments. Check your internet connection and try again.",
);
}
}, [isError, error?.message, isEmpty]);The rest of the changes are well-structured:
|
There was a problem hiding this comment.
🟡 Changes recommended
The new Activity error-state logic uses an “empty list” heuristic that can misclassify refresh failures (especially when the last successful result was empty) and the CTA passes an async refetch where a sync handler is expected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refines merchant-facing error handling in the POS app by replacing technical error strings with friendlier messaging, and by improving Activity screen behavior when payments fail to load or refresh. It also increments the Android versionCode for the next release.
Changes:
- Replace settings/printer error toasts with merchant-friendly, non-technical messages.
- Update Activity screen to show a retryable empty-state on initial load failure and a lighter refresh-failure toast when data is already present.
- Bump Android
versionCodefrom 3 → 4.
File summaries
| File | Description |
|---|---|
| dapps/pos-app/hooks/use-merchant-flow.ts | Replaces technical save failure messaging with a user-friendly toast while keeping detailed logs. |
| dapps/pos-app/app/settings.tsx | Ensures printer connection failures show a consistent merchant-friendly toast rather than raw errors. |
| dapps/pos-app/app/activity.tsx | Adds initial-load error empty state and refresh-failure toast behavior for payments fetching. |
| dapps/pos-app/app.json | Increments Android versionCode for release progression. |
Review details
Suppressed comments (1)
dapps/pos-app/app/activity.tsx:190
- The empty/error UI currently triggers on any
isError, which will also replace the normal empty states after a refetch failure when the last successful result was an empty list. Consider only showing the retryable error state when the initial load failed (i.e. no cacheddata). Also,EmptyState's CTAonPressis typed as() => void, so passingrefetch(async) directly will fail TypeScript checks—wrap it and ignore the returned promise.
<EmptyState
title="We couldn't load payments"
subtitle="Check your internet connection and try again."
cta={{ label: "Try again", onPress: refetch }}
/>
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
Validation
git diff --checkjest,expo,prettier, and TypeScript are unavailable).