Conversation
🦋 Changeset detectedLatest commit: 4b22d84 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe PR encompasses multiple improvements: enhancing AssetLogo and ChainLogo component APIs to support URI and border-radius customization, refactoring Input component styling from inline styles to props, updating navigation to direct successful operations to an activity screen, and adjusting test automation flows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a series of miscellaneous fixes and user interface enhancements across the mobile application. The changes aim to improve the visual fidelity and user experience by refining carousel animations, ensuring correct display of asset and chain logos, and standardizing the styling of various input fields. These updates contribute to a more polished and consistent application appearance. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a collection of miscellaneous fixes and improvements. The most significant change is the refactoring of the authentication screen's carousel to use react-native-reanimated-carousel, which enhances transitions and maintainability. Other changes include fixes for missing asset logos and improvements to Tamagui input styles. My review focuses on improving code quality by addressing redundancy and code duplication. I've identified several instances where style props on Input components are now redundant due to the addition of direct style props, and also pointed out a duplicated helper function that should be extracted to a shared utility. All original comments have been retained as they are valid code quality improvements and do not conflict with the provided rules. Overall, these are good improvements, and my suggestions aim to polish the codebase further.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #696 +/- ##
==========================================
+ Coverage 64.01% 64.36% +0.35%
==========================================
Files 169 173 +4
Lines 5324 5391 +67
Branches 1501 1515 +14
==========================================
+ Hits 3408 3470 +62
- Misses 1730 1737 +7
+ Partials 186 184 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/send-funds/Receiver.tsx (1)
85-97: Keep Inputstyletypography-only; remove border values from inline style.Lines 88, 94, and 95 contain redundant border styling. Border values (
borderColor,borderWidth) should use Tamagui props only. ThefontSizemust be moved from the Tamagui prop (line 93) to thestyleobject as an exception for Input typography, butborderColorandborderWidthin thestyleobject should be removed.Proposed fix
<Input flex={1} placeholder={t("Enter {{chain}} address", { chain: chain.name })} borderColor="$uiNeutralTertiary" borderTopRightRadius={0} borderBottomRightRadius={0} value={value} onChangeText={handleChange} - fontSize={15} borderWidth={1} - style={{ borderColor: "$uiNeutralTertiary", borderWidth: 1, fontSize: 15 }} + style={{ fontSize: 15 }} focusStyle={{ borderColor: "$borderBrandStrong", borderWidth: 1 }} backgroundColor="$backgroundSoft" />src/components/pay-mode/PaySelector.tsx (1)
156-169: Remove redundant fontSize prop and clean up style object to typography only.The Input component doesn't apply typography styles (fontSize) passed as direct props. Line 162's
fontSize={20}won't be applied and is redundant with the style object on line 164. Additionally, move flex and padding out of the style object—they should be handled via Tamagui props. Keep onlyfontSizein the style object (this is a valid exception for Input's typography handling).Proposed fix
<Input borderWidth={0} inputMode="decimal" maxLength={6} numberOfLines={1} onChangeText={setInput} - fontSize={20} - padding={0} - style={{ fontSize: 20, padding: 0, flex: 1 }} + style={{ fontSize: 20 }} textAlign="right" value={input} width="100%" flex={1} />src/components/swaps/TokenInput.tsx (1)
174-183: Remove redundant typography props—they are no-ops.The
fontFamily,fontSize,fontWeight, andletterSpacingprops are ignored by the Input component. Only thestyleobject applies these values, which is the correct approach here.♻️ Suggested cleanup
- fontFamily="BDOGrotesk-Regular" - fontSize={28} - fontWeight="bold" - letterSpacing={-0.2} style={{ fontFamily: "BDOGrotesk-Regular", fontSize: 28, fontWeight: "bold", letterSpacing: -0.2, }}
🤖 Fix all issues with AI agents
In `@src/components/auth/Auth.tsx`:
- Around line 116-128: The Carousel JSX in Auth.tsx is explicitly passing the
loop prop even though react-native-reanimated-carousel defaults loop to true;
remove the redundant loop prop from the Carousel element (the one using props
data={pages}, renderItem={renderItem}, onSnapToItem={handleSnapToItem}, etc.) so
the component remains concise and behavior unchanged—only remove the loop
attribute, do not alter other props or handlers like
autoPlay/autoPlayInterval/onProgressChange.
In `@src/components/auth/Pagination.tsx`:
- Around line 46-50: Replace the hardcoded backgroundColor in the AnimatedView
return (the prop backgroundColor="rgba(0,0,0,0.1)") with a design token from
tamagui.config.ts; update the AnimatedView that uses styles.dot /
rContainerStyle to reference an appropriate neutral token (e.g.,
$uiNeutralSecondary or another opacity/neutral token available in your design
system) or create a new token there if needed, ensuring no raw rgba values
remain in Pagination.tsx and all styling uses the tamagui tokens instead.
In `@src/components/benefits/BenefitsSection.tsx`:
- Around line 67-71: The prop type for scrollOffset is currently declared as `{
value: number }`; update it to use react-native-reanimated's SharedValue<number>
for stronger typing and compatibility (import SharedValue from
'react-native-reanimated' or the appropriate named import) and change the prop
signature where scrollOffset is referenced (e.g., in the BenefitsSection
component/type annotation) to `scrollOffset: SharedValue<number>` and update any
usages accordingly to satisfy the new type.
- Around line 158-166: The calculateDistance worklet is duplicated; extract it
into a single exported utility (named calculateDistance) in a shared utils
module, preserve the exact signature and the "worklet" directive, then replace
the local function definitions in the BenefitsSection and Pagination components
to import and use that exported calculateDistance; ensure TypeScript
export/imports are added and no logic changes are made.
- Around line 62-91: PaginationDot currently passes an inline style object ({
height: 8, borderRadius: 10 }) into AnimatedView which violates the
no-inline-style rule; update PaginationDot to move those properties into Tamagui
styled props or a StyleSheet entry and apply them via a named style or component
props instead of an object literal — e.g., replace the inline object in the
style array passed to AnimatedView with a reference to a StyleSheet style (e.g.,
styles.dot) or use Tamagui props on AnimatedView (height, borderRadius/ br) so
the final style array becomes [styles.dot, rStyle, rColorStyle] or the
AnimatedView uses Tamagui props together with rStyle and rColorStyle; ensure you
modify the return in PaginationDot and add the new StyleSheet or Tamagui-styled
component alongside it.
In `@src/components/loans/AmountSelector.tsx`:
- Around line 106-110: In AmountSelector.tsx remove the redundant typography
props (fontSize={34}, fontWeight="400", letterSpacing={-0.2}) passed alongside
the style object on the Input/Tamagui component and keep only the style={{
fontSize: 34, fontWeight: 400, letterSpacing: -0.2 }} (or alternatively update
the Input component to accept those props), so locate the Input usage in the
AmountSelector component and delete the duplicated prop entries while retaining
the single style prop for typography.
In `@src/components/shared/AmountSelector.tsx`:
- Around line 129-131: Remove the redundant fontSize prop on the Tamagui Input
instances in AmountSelector (e.g., the Input used inside the AmountSelector
component that currently has both fontSize={24} and style={{ fontSize: 24 }});
keep the single source of truth by leaving the fontSize inside the style object
(style={{ fontSize: 24 }}) and delete the direct fontSize prop, and apply the
same cleanup to the other duplicate occurrence later in the file (the second
Input instance that repeats fontSize props).
In `@src/components/shared/AssetLogo.tsx`:
- Around line 28-41: AssetLogo is always fetching and filtering tokens even when
a uri prop (defaultUri) is provided; update the useQuery(lifiTokensOptions) call
to be conditional by passing enabled: !defaultUri so the query is skipped when a
uri exists, and guard the chainTokens/filter logic to run only when defaultUri
is falsy before calling getTokenLogoURI; adjust references in the function to
use the conditional result (useQuery, lifiTokensOptions, chainTokens,
getTokenLogoURI, defaultUri/uri) so no unnecessary work occurs when an explicit
uri is supplied.
1bed8a1 to
3434f0d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/auth/ListItem.tsx (1)
18-47: Prefer a function declaration for the memoized component.This multi-line component is declared as a function expression; declare it and memoize on export to match the codebase convention.
♻️ Suggested refactor
-export default memo(function ListItem({ item, animationValue }: ListItemProperties) { - const aspectRatio = useAspectRatio(); - const { width, height } = useWindowDimensions(); - const itemWidth = Platform.OS === "web" ? height * aspectRatio : width; - - // animationValue: -1 (left), 0 (center/active), 1 (right) - /* istanbul ignore next */ - const rBackgroundStyle = useAnimatedStyle(() => { - const animatedScale = interpolate(animationValue.value, [-1, 0, 1], [0.5, 1, 0.5], Extrapolation.CLAMP); - const interpolatedOpacity = interpolate(animationValue.value, [-1, 0, 1], [0.3, 1, 0.3], Extrapolation.CLAMP); - return { transform: [{ scale: animatedScale }], opacity: interpolatedOpacity }; - }, [animationValue]); - - /* istanbul ignore next */ - const rImageStyle = useAnimatedStyle(() => { - const animatedScale = interpolate(animationValue.value, [-1, 0, 1], [0.7, 1, 0.7], Extrapolation.CLAMP); - return { transform: [{ scale: animatedScale }] }; - }, [animationValue]); - - return ( - <View width={itemWidth} aspectRatio={aspectRatio} justifyContent="center" alignItems="center"> - <AnimatedView style={rBackgroundStyle} width="100%" height="100%"> - <item.backgroundImage width="100%" height="100%" /> - </AnimatedView> - <AnimatedView style={[StyleSheet.absoluteFillObject, rImageStyle]} width="100%" height="100%"> - <item.image width="100%" height="100%" /> - </AnimatedView> - </View> - ); -}); +function ListItem({ item, animationValue }: ListItemProperties) { + const aspectRatio = useAspectRatio(); + const { width, height } = useWindowDimensions(); + const itemWidth = Platform.OS === "web" ? height * aspectRatio : width; + + // animationValue: -1 (left), 0 (center/active), 1 (right) + /* istanbul ignore next */ + const rBackgroundStyle = useAnimatedStyle(() => { + const animatedScale = interpolate(animationValue.value, [-1, 0, 1], [0.5, 1, 0.5], Extrapolation.CLAMP); + const interpolatedOpacity = interpolate(animationValue.value, [-1, 0, 1], [0.3, 1, 0.3], Extrapolation.CLAMP); + return { transform: [{ scale: animatedScale }], opacity: interpolatedOpacity }; + }, [animationValue]); + + /* istanbul ignore next */ + const rImageStyle = useAnimatedStyle(() => { + const animatedScale = interpolate(animationValue.value, [-1, 0, 1], [0.7, 1, 0.7], Extrapolation.CLAMP); + return { transform: [{ scale: animatedScale }] }; + }, [animationValue]); + + return ( + <View width={itemWidth} aspectRatio={aspectRatio} justifyContent="center" alignItems="center"> + <AnimatedView style={rBackgroundStyle} width="100%" height="100%"> + <item.backgroundImage width="100%" height="100%" /> + </AnimatedView> + <AnimatedView style={[StyleSheet.absoluteFillObject, rImageStyle]} width="100%" height="100%"> + <item.image width="100%" height="100%" /> + </AnimatedView> + </View> + ); +} + +export default memo(ListItem);As per coding guidelines: "Prefer function declarations for all multi-line functions; use function expressions or arrow functions only for single-line implementations."
🤖 Fix all issues with AI agents
In `@src/components/auth/Pagination.tsx`:
- Around line 63-69: The StyleSheet entry styles.dot in Pagination.tsx uses
hardcoded borderRadius: 4 and height: 4; update it to use Tamagui design tokens
instead (either pass token values through Pagination component props and use
them in the Tamagui component, or resolve tokens before creating the StyleSheet
and reference those token variables). Locate the styles constant and replace the
numeric literals for borderRadius and height with token references from your
tamagui.config.ts (e.g., spacing or radius tokens) or with props supplied to the
Pagination/dot renderer so the values come from the design system rather than
hardcoded numbers.
In `@src/components/swaps/TokenInput.tsx`:
- Around line 174-177: The typography props passed directly to the Tamagui Input
in TokenInput.tsx (fontFamily, fontSize, fontWeight, letterSpacing) won’t be
applied; update the Input usage (the Input component instance in TokenInput) to
remove these top-level props and instead pass them inside the style prop object
(e.g. style={{ fontFamily: ..., fontSize: ..., fontWeight: ..., letterSpacing:
... }}), ensuring all four typography values are moved into style so the Input
renders the intended typography.
91dfc86 to
ac01b11
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/pay-mode/PaySelector.tsx`:
- Around line 162-167: In PaySelector.tsx, the Input used in the PaySelector
component is passing typography props (fontSize) directly which Tamagui ignores;
move fontSize into the component's style prop (e.g., style={{ fontSize: 20,
...existingStyle }}) instead of as a top-level prop on the Input, and do the
same for any other typography props (fontFamily, fontWeight, lineHeight,
letterSpacing) if present so the Input receives them via style while keeping
value={input}, padding/width/flex/textAlign as props.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/shared/Input.tsx (1)
3-12:⚠️ Potential issue | 🟡 MinorReplace hardcoded
fontSize: 15with a design token.The concern about typography styles not applying in
styled()is not accurate—Tamagui's Input component supports typography props (fontSize, fontFamily, lineHeight) both as direct props and in styled definitions. However, the code violates styling guidelines by using a hardcoded numeric value instead of a design token. ReplacefontSize: 15with an appropriate token fromtamagui.config.ts(e.g.,fontSize: "$4"or"$5").
🤖 Fix all issues with AI agents
In `@src/app/`(main)/(home)/_layout.tsx:
- Line 63: The inline style on TabSlot should be replaced by Tamagui layout
props: wrap <TabSlot> in a Tamagui YStack and remove the style prop, e.g. put
TabSlot as a child of YStack and apply Tamagui shorthand like flex={1} (and any
other needed layout props) on the YStack instead of style={{ flex: 1 }}; update
the JSX around the TabSlot element accordingly.
In `@src/components/auth/ListItem.tsx`:
- Around line 13-35: The component is currently exported inline via
memo(function ListItem...) which hinders stack traces; refactor by creating a
separate named function declaration function ListItem({ item, animationValue }:
ListItemProperties) { ... } containing the existing hooks and animated styles
(rBackgroundStyle, rImageStyle, useAspectRatio, useWindowDimensions logic), then
export it with export default memo(ListItem) so memoization is preserved but the
function is a proper named declaration.
In `@src/components/auth/Pagination.tsx`:
- Around line 71-81: The component is currently an anonymous function passed
directly into memo; rename it to a named function declaration (function
Pagination({...}) { ... }) and then export it via export default
memo(Pagination) so the component follows the function-declaration guideline;
update the existing parameter and type signature in the named function and keep
the same props (length, scrollOffset, progress, isScrolling) and usage inside
the body.
In `@src/components/benefits/BenefitsSection.tsx`:
- Around line 63-104: styles.dot currently uses hardcoded numeric values for
height and borderRadius; remove the StyleSheet and instead apply Tamagui
token-based sizing directly on the AnimatedView inside PaginationDot (e.g., use
Tamagui size/space/radii tokens as props such as size, height, br or width
tokens) and compute dynamic width/opactiy with rStyle/rColorStyle as animated
styles only; update PaginationDot to accept token names or map the numeric sizes
to the corresponding tamagui tokens (from tamagui.config.ts) and ensure
AnimatedView receives those token props rather than numeric style values so all
sizing uses design tokens.
In `@src/components/send-funds/Receiver.tsx`:
- Around line 93-94: In Receiver.tsx remove the redundant prop fontSize={15}
from the Input usage (unless you intentionally plan to override it) and keep
borderWidth if needed; also verify that the shared Input component
(../shared/Input.tsx) actually accepts and forwards typography props (e.g.,
fontSize) through its styled wrapper and prop spread so future overrides work—if
it doesn't, update the Input component to pass style/typography props to the
underlying input element.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/shared/Input.tsx (1)
3-11:⚠️ Potential issue | 🟠 MajorMove
fontSizetostyleprop for Input typography to take effect.Tamagui's Input component does not apply typography styles (fontSize, fontWeight, etc.) when passed directly as props—they must be provided via the
styleprop object. This is a required exception to the no-inline-styles rule for Input components.🔧 Suggested fix
export default styled(Input, { - fontSize: 15, + style: { fontSize: 15 }, padding: "$s3", borderColor: "$borderBrandStrong", color: "$uiNeutralPrimary", borderRadius: "$r3", placeholderTextColor: "$uiNeutralSecondary", focusStyle: { borderColor: "$borderBrandStrong" }, focusVisibleStyle: { outlineWidth: 0, borderColor: "$borderBrandStrong", outlineColor: "$borderBrandStrong" }, });
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/loans/Review.tsx (1)
397-404:⚠️ Potential issue | 🟡 MinorExtract "/activity" route to a typed constant for reuse and safety.
The hardcoded string is used twice in this component (lines 403 and 471). Extract it to a constant to follow the guideline "Leverage Expo Router's typed routes to ensure link safety. Do not construct route strings manually."
♻️ Suggested fix
-import { useRouter } from "expo-router"; +import { useRouter } from "expo-router"; +import type { Href } from "expo-router"; @@ export default function Review() { const router = useRouter(); + const activityRoute: Href = "/activity"; @@ - router.dismissTo("/activity"); + router.dismissTo(activityRoute); @@ - router.dismissTo("/activity"); + router.dismissTo(activityRoute);Also applies to: 467-472
🤖 Fix all issues with AI agents
In `@src/components/add-funds/Bridge.tsx`:
- Around line 485-486: Replace the hard-coded "/activity" string passed to
router.dismissTo with the app's typed route helper/constant (do not construct
route strings manually); update the two sites where
router.dismissTo("/activity") is used (the occurrence near router.dismissTo and
the second occurrence around lines 550-551) to call the typed route function or
constant (e.g., use the project's typed Routes/route helper such as
routes.activity() or ActivityRoute constant) so navigation uses the typed route
API instead of a literal string.
In `@src/components/loans/AmountSelector.tsx`:
- Around line 106-109: The typography props on the Input in AmountSelector
(fontSize, fontWeight, letterSpacing) will be ignored; move them into a style
object passed via the style prop (e.g., style={{ fontSize: 34, fontWeight:
'400', letterSpacing: -0.2 }}) on the Input component and keep or move cursor
styling into that style object as appropriate so Tamagui applies the typography
correctly.
In `@src/components/shared/AmountSelector.tsx`:
- Around line 129-131: The fontSize prop on the Input in AmountSelector should
be applied via the style object instead of as a direct prop; revert
fontSize={24} to using style={{ fontSize: 24 }} on the Input (leave borderWidth
and flex props as-is), so the typography is actually applied by Tamagui's Input
component.
In `@src/components/swaps/Success.tsx`:
- Around line 128-131: The onPress handler is using a hard-coded route string
("/activity"); replace this with the app's Expo Router typed route
helper/constant (e.g., ActivityRoute or routes.activity) when calling
router.dismissTo to ensure type-safe navigation. Edit the onPress block that
calls queryClient.invalidateQueries and router.dismissTo so it continues to
invalidate and report errors but passes the typed route constant instead of the
literal "/activity" (update any imports to bring in the typed route helper if
needed).
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.