Quick-actions row - #517
Conversation
|
Someone is attempting to deploy a commit to the miracle656's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@attyolu Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
@Miracle656 please merger. Thank you. |
Miracle656
left a comment
There was a problem hiding this comment.
components/QuickActions.tsx itself is good work. It's properly typed with an exported QuickActionItem, takes an actions prop instead of hardcoding the list, and — the part most people skip — every action carries an accessibilityLabel. Thank you for that.
The PR around it needs restructuring though.
🚨 1. This would replace the working swap screen with a placeholder
main already has a real frontend/mobile/app/swap.tsx — 150 lines, live Soroswap quoting with debounce. This PR adds a 20-line version:
export default function SwapScreen() {
return (
<View style={styles.container}>
<Text style={styles.title}>Swap</Text>
<Text style={styles.subtitle}>Swap tokens instantly on Soroban DEX</Text>
</View>
)
}That's an add/add conflict, and resolving it the wrong way silently deletes a working feature. Same category of problem for the other three stubs.
🚨 2. Undeclared stack on #515
This branch contains #515's commit 7673d1d, so five of its files are the NFT gallery, not quick actions:
frontend/wallet/app/nfts/page.tsx 589+
frontend/wallet/lib/nfts.ts 212+
frontend/wallet/lib/__tests__/nfts.test.ts 114+
frontend/wallet/app/dashboard/page.tsx 25+/20-
frontend/wallet/tsconfig.json 1+/4-
Stacking is fine — #510 does it and says so in its description, which is all I need. Please add a line to the description here noting it builds on #515.
It does mean this inherits #515's blocker: the NFT gallery currently shows fabricated NFTs to users who own none, with owner rewritten to their real wallet address. Details are on #515. This PR can't merge until that's fixed there.
What I'd like instead
Quick actions only need the routes to exist — they don't need you to author them. #507 (navigation shell) creates stubs for every route in the app, including send, receive, swap, and buy, and it lands before this. So:
- Drop all four stub screens (
app/send.tsx,app/receive.tsx,app/swap.tsx,app/buy.tsx). #507 provides them,mainalready has the realswap, #512 is building the real swap execute screen, and #522 is building the real buy screen. - Keep
components/QuickActions.tsx, its test, and theapp/index.tsxwiring. - Rebase onto #507, then point the routes at its tree. Note #507 puts
sendandreceiveinside the tab group, so/sendand/receiveinQUICK_ACTIONSwill need to match whatever that resolves to.
That should take this from 16 files to about 3, and removes the conflict entirely.
On the test
import { QUICK_ACTIONS, QuickActionItem } from "../QuickActions";It only imports the constant, so it asserts the array's shape and never renders the component. That means a broken onPress, a crash in render, or a missing accessibility label all pass. Since you've already got the a11y labels right, it'd be a shame not to assert them — @testing-library/react-native with render() plus getByLabelText would cover the behaviour that actually matters.
Not a blocker, but worth doing while the component is fresh.
Attribution
Same note as on #515: commits are authored flourishbar <you@example.com>, the git default placeholder, so GitHub records the author as invalid-email-address and none of this counts as your contribution for Wave tracking. Worth fixing:
git config --global user.email "your-real-github-email@example.com"
git config --global user.name "Your Name"Reduce this branch to the quick-actions row Miracle656#460 asks for: - Drop the CAP-46 NFT gallery (frontend/wallet/app/nfts, lib/nfts.ts and its test, the dashboard/page.tsx and tsconfig edits). That work is Miracle656#349, reviewed separately in Miracle656#515 and closed there — it injected fixture NFTs whenever a wallet held none or the fetch failed, with owner rewritten to the connected address, which Miracle656#349 explicitly prohibits. - Drop the duplicate root app/send.tsx and app/receive.tsx; /send and /receive are owned by the (tabs) group. Keep main's buy.tsx, swap.tsx, index.tsx, tsconfig.json and @types/jest. - Wire QuickActions into (tabs)/dashboard.tsx, which is what makes the four actions reachable — the component was added but never rendered anywhere. tsc clean; jest 9 suites / 160 tests.
|
Merging the quick-actions row. Three things I changed before merging: Removed the NFT gallery. This branch also carried Removed duplicate route stubs. The branch added Wired it into the dashboard. This is the one that mattered for the acceptance criterion: Verified: One follow-up worth noting: the component hardcodes |
- Restore QuickActions on the dashboard. This branch predates Miracle656#517 and its dashboard rewrite dropped the component and its import, which would have removed the quick-actions row. - fetchTransfers swallowed every failure and returned [], so an unreachable indexer, an HTTP error and a genuinely empty wallet all rendered as "No transactions yet." It now throws; useInitActivityFeed already catches into its error state, which was being returned but never used. - ActivityFeed takes an error prop and renders a distinct message, and the dashboard passes it through. tsc clean; jest 11 suites / 198 tests; expo lint clean.
…ent (#528) * feat(mobile): port activity feed store and create ActivityFeed component - Create frontend/mobile/lib/activityFeed.ts with hydrate/append lifecycle ported from frontend/wallet - Wire to Wraith GET /transfers/:address endpoint with 15s polling - Create ActivityFeed component with filter pills, skeleton loading, swap display - Update mobile dashboard to integrate the feed Closes #461 * fix(activity-feed): robust amount/timestamp parsing, swap detection, env docs * fix(activity-feed): address PR review — move feed to dashboard tab, use wallet address from store, add fetch timeout * Merge origin/main into activity-feed-mobile - Restore QuickActions on the dashboard. This branch predates #517 and its dashboard rewrite dropped the component and its import, which would have removed the quick-actions row. - fetchTransfers swallowed every failure and returned [], so an unreachable indexer, an HTTP error and a genuinely empty wallet all rendered as "No transactions yet." It now throws; useInitActivityFeed already catches into its error state, which was being returned but never used. - ActivityFeed takes an error prop and renders a distinct message, and the dashboard passes it through. tsc clean; jest 11 suites / 198 tests; expo lint clean. * chore(mobile): resync package-lock after the merge The lockfile auto-merged into a state npm 10 rejected: npm error `npm ci` can only install packages when your package.json and package-lock.json ... are in sync npm error Missing: @jest/transform@29.7.0 from lock file Regenerated with npm 10 so it matches what CI installs. --------- Co-authored-by: ezedike-evan <120946193+ezedike-evan@users.noreply.github.com>
##closes #460