feat: wire bottom snack bar to wallet buy button with translations#110
Open
ice-selene wants to merge 5 commits into
Open
feat: wire bottom snack bar to wallet buy button with translations#110ice-selene wants to merge 5 commits into
ice-selene wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughExports and UI wiring were added: Changes
Sequence DiagramsequenceDiagram
participant User as User
participant AB as ActionButton
participant ABRow as ActionButtonsRow
participant WS as WalletScreen
participant Hook as useWalletActions
participant SN as BottomSnackBar
User->>AB: tap Buy
AB->>ABRow: onPress()
ABRow->>WS: call onBuyPress()
WS->>Hook: showSnackBar()
Hook-->>WS: isSnackBarVisible = true
WS->>SN: render(message, onDismiss)
SN->>User: display "Buy crypto easily. Coming soon."
User->>SN: dismiss
SN->>WS: onDismiss -> hideSnackBar()
WS->>Hook: hideSnackBar()
Hook-->>WS: isSnackBarVisible = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (4)
frontend/packages/wallet-ui/src/screens/WalletScreen.tsx (2)
40-42: Consider memoizing the inline style object.The style object
{ paddingHorizontal: scaleSize(16), paddingBottom: scaleSize(16) }is recreated on every render. While not a major performance concern, memoizing it withuseMemowould be more consistent with the patterns used elsewhere in the codebase (e.g.,useWalletScreenStyles, theuseMemopatterns inActionButton).✨ Suggested improvement
+ const snackBarPadding = useMemo( + () => ({ paddingHorizontal: scaleSize(16), paddingBottom: scaleSize(16) }), + [scaleSize], + ); + return ( <View style={s.screen}> ... - <View style={[styles.snackBarWrapper, { paddingHorizontal: scaleSize(16), paddingBottom: scaleSize(16) }]}> + <View style={[styles.snackBarWrapper, snackBarPadding]}> <BottomSnackBar message={translate("walletUi:buyCryptoComingSoon")} isVisible={actions.isSnackBarVisible} onDismiss={actions.hideSnackBar} /> </View> </View> );You'll also need to add
useMemoto the imports on line 1.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet-ui/src/screens/WalletScreen.tsx` around lines 40 - 42, The inline style object passed to the View in WalletScreen (styles.snackBarWrapper) is recreated each render; wrap the object `{ paddingHorizontal: scaleSize(16), paddingBottom: scaleSize(16) }` in a useMemo and replace the inline literal with that memoized variable, adding useMemo to the imports; keep the dependency array minimal (e.g., [] or include scaleSize if it can change) and reuse the memoized style in the View to avoid allocating a new object each render.
33-33: Simplify the conditional prop passing.Same pattern as in
ActionButtonsRow- the conditional spread is unnecessarily verbose sinceonBuyPressis optional.✨ Suggested simplification
- <ActionButtonsRow {...(actions.showSnackBar ? { onBuyPress: actions.showSnackBar } : {})} /> + <ActionButtonsRow onBuyPress={actions.showSnackBar} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet-ui/src/screens/WalletScreen.tsx` at line 33, The conditional spread around ActionButtonsRow is unnecessary—replace the spread logic with a direct optional prop by passing onBuyPress={actions.showSnackBar} to ActionButtonsRow (since onBuyPress is already optional) instead of using the {...(actions.showSnackBar ? { onBuyPress: actions.showSnackBar } : {})} pattern; update the component invocation where ActionButtonsRow is used to remove the spread and supply onBuyPress directly.frontend/packages/wallet-ui/src/screens/useWalletActions.ts (1)
10-10: Use full word instead of abbreviation.The parameter
pin the functional update should use a full word likepreviousper coding guidelines that discourage abbreviations.✨ Suggested fix
- const toggleBalance = useCallback(() => setIsBalanceVisible((p) => !p), []); + const toggleBalance = useCallback(() => setIsBalanceVisible((previous) => !previous), []);As per coding guidelines: "Use full words in naming; no abbreviations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet-ui/src/screens/useWalletActions.ts` at line 10, Rename the abbreviated functional-update parameter in toggleBalance to a full word: change the arrow callback passed to setIsBalanceVisible inside the toggleBalance function so the parameter `p` uses a descriptive name like `previous` (e.g., in the toggleBalance const and its use of setIsBalanceVisible) to comply with naming guidelines.frontend/packages/wallet-ui/src/components/ActionButtonsRow.tsx (1)
21-21: Simplify the conditional prop passing.The conditional spread
{...(onBuyPress ? { onPress: onBuyPress } : {})}is unnecessarily verbose. SinceonPressis optional inActionButton, you can passundefineddirectly.✨ Suggested simplification
- <ActionButton iconName="wallet-buycrypto" label={translate("walletUi:buyAction")} filled {...(onBuyPress ? { onPress: onBuyPress } : {})} /> + <ActionButton iconName="wallet-buycrypto" label={translate("walletUi:buyAction")} filled onPress={onBuyPress} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/wallet-ui/src/components/ActionButtonsRow.tsx` at line 21, Replace the verbose conditional prop spread in ActionButtonsRow.tsx: instead of using {...(onBuyPress ? { onPress: onBuyPress } : {})} when rendering ActionButton, pass the optional handler directly as onPress={onBuyPress}; this leverages the fact that onPress is optional on ActionButton and will be undefined when onBuyPress is not provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/packages/wallet-ui/src/components/ActionButtonsRow.tsx`:
- Line 21: Replace the verbose conditional prop spread in ActionButtonsRow.tsx:
instead of using {...(onBuyPress ? { onPress: onBuyPress } : {})} when rendering
ActionButton, pass the optional handler directly as onPress={onBuyPress}; this
leverages the fact that onPress is optional on ActionButton and will be
undefined when onBuyPress is not provided.
In `@frontend/packages/wallet-ui/src/screens/useWalletActions.ts`:
- Line 10: Rename the abbreviated functional-update parameter in toggleBalance
to a full word: change the arrow callback passed to setIsBalanceVisible inside
the toggleBalance function so the parameter `p` uses a descriptive name like
`previous` (e.g., in the toggleBalance const and its use of setIsBalanceVisible)
to comply with naming guidelines.
In `@frontend/packages/wallet-ui/src/screens/WalletScreen.tsx`:
- Around line 40-42: The inline style object passed to the View in WalletScreen
(styles.snackBarWrapper) is recreated each render; wrap the object `{
paddingHorizontal: scaleSize(16), paddingBottom: scaleSize(16) }` in a useMemo
and replace the inline literal with that memoized variable, adding useMemo to
the imports; keep the dependency array minimal (e.g., [] or include scaleSize if
it can change) and reuse the memoized style in the View to avoid allocating a
new object each render.
- Line 33: The conditional spread around ActionButtonsRow is unnecessary—replace
the spread logic with a direct optional prop by passing
onBuyPress={actions.showSnackBar} to ActionButtonsRow (since onBuyPress is
already optional) instead of using the {...(actions.showSnackBar ? { onBuyPress:
actions.showSnackBar } : {})} pattern; update the component invocation where
ActionButtonsRow is used to remove the spread and supply onBuyPress directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d62b6948-87e5-447c-86c9-6168a55b7e77
📒 Files selected for processing (8)
frontend/packages/ui/src/index.tsfrontend/packages/wallet-ui/src/components/ActionButton.tsxfrontend/packages/wallet-ui/src/components/ActionButtonsRow.tsxfrontend/packages/wallet-ui/src/screens/WalletScreen.tsxfrontend/packages/wallet-ui/src/screens/useWalletActions.tsfrontend/packages/wallet-ui/src/translations/de.tsfrontend/packages/wallet-ui/src/translations/en.tsfrontend/packages/wallet-ui/src/translations/fr.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wires the BottomSnackBar component to the Wallet Buy button. Tapping "Buy" shows "Buy crypto easily. Coming soon." for 3 seconds.
Changes
Summary by CodeRabbit