Conversation
🦋 Changeset detectedLatest commit: 7fd9d53 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 |
WalkthroughThis PR refactors the payment and home screens architecture by replacing the pay-mode module with a new pay structure, adding installments calculator and related modals, updating design tokens (fonts from BDOGrotesk to SplineSans, numeric spacing to theme tokens), modernizing animations, and introducing new sheets for credit limits, installments, and payment modes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 delivers a comprehensive redesign of the application's core user interface, specifically targeting the home and payments sections. The changes aim to modernize the application's appearance, improve navigation, and introduce new functionalities such as an installments calculator and enhanced card management options. The update focuses on providing a more intuitive and visually appealing experience for users interacting with their financial data and card services. Highlights
Changelog
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 significant UI/UX overhaul and new features, primarily focusing on the 'Pay' experience and card functionalities. Key changes include replacing old fonts with 'SplineSans', renaming the 'Pay Mode' tab to 'Payments' with updated icons and haptic feedback, and implementing a new 'Installments Calculator' and related sheets for managing card payment modes. The card status component has been restyled, and the benefits carousel refined with animation and accessibility improvements. Maestro tests have been updated and expanded to cover these new features and UI changes. Numerous spacing and animation token adjustments were made across various components (e.g., activity, add funds, card details, portfolio summary) to unify the design language. Additionally, the logic for aggregating overdue payments was fixed to correctly calculate discounts, and the collateralUSD calculation was refactored for conciseness. A review comment highlighted a potential unreliability in the spotlight component's measurement logic due to fixed setTimeout durations.
| overduePayments.set(maturity, { | ||
| amount: (overduePayments.get(maturity)?.amount ?? 0n) + previewValue, | ||
| discount: Number(WAD - (previewValue * WAD) / positionAmount) / 1e18, | ||
| }); |
There was a problem hiding this comment.
There's a bug in how overdue payments are aggregated. The discount is calculated for each individual borrow position and then overwritten if multiple positions share the same maturity. The positionAmount is not being accumulated, leading to an incorrect discount calculation for aggregated positions. The fix is to accumulate both amount and positionAmount and calculate the discount on the totals.
const existing = overduePayments.get(maturity);
const newAmount = (existing?.amount ?? 0n) + previewValue;
const newPositionAmount = (existing?.positionAmount ?? 0n) + positionAmount;
overduePayments.set(maturity, {
amount: newAmount,
positionAmount: newPositionAmount,
discount: newPositionAmount > 0n ? Number(WAD - (newAmount * WAD) / newPositionAmount) / 1e18 : 0,
});
| const collateralUSD = useMemo(() => { | ||
| if (!markets) return 0n; | ||
| let total = 0n; | ||
| for (const market of markets) { | ||
| if (market.floatingDepositAssets > 0n) { | ||
| total += (market.floatingDepositAssets * market.usdPrice) / 10n ** BigInt(market.decimals); | ||
| } | ||
| } | ||
| return total; | ||
| }, [markets]); |
There was a problem hiding this comment.
The collateralUSD calculation can be made more concise and functional by using the reduce method instead of a for loop. This improves readability and aligns with modern JavaScript practices.
| const collateralUSD = useMemo(() => { | |
| if (!markets) return 0n; | |
| let total = 0n; | |
| for (const market of markets) { | |
| if (market.floatingDepositAssets > 0n) { | |
| total += (market.floatingDepositAssets * market.usdPrice) / 10n ** BigInt(market.decimals); | |
| } | |
| } | |
| return total; | |
| }, [markets]); | |
| const collateralUSD = useMemo( | |
| () => | |
| markets?.reduce( | |
| (total, market) => | |
| total + | |
| (market.floatingDepositAssets > 0n | |
| ? (market.floatingDepositAssets * market.usdPrice) / 10n ** BigInt(market.decimals) | |
| : 0n), | |
| 0n, | |
| ) ?? 0n, | |
| [markets], | |
| ); |
| useEffect(() => { | ||
| let remeasureTimer: ReturnType<typeof setTimeout>; | ||
| const timer = setTimeout(() => { | ||
| targetRef.current?.measureInWindow((x, y, width, height) => { | ||
| if (width > 0 && height > 0 && y >= 0 && y + height <= screenHeight) { | ||
| setTarget({ x, y, width, height }); | ||
| return; | ||
| } | ||
| if (width > 0 && height > 0) { | ||
| const contentY = scrollOffset.current + y; | ||
| scrollRef.current?.scrollTo({ y: Math.max(0, contentY - screenHeight / 3), animated: true }); | ||
| } else { | ||
| scrollRef.current?.scrollTo({ y: 0, animated: true }); | ||
| } | ||
| remeasureTimer = setTimeout(() => { | ||
| targetRef.current?.measureInWindow((x2, y2, w2, h2) => { | ||
| if (w2 > 0 && h2 > 0) setTarget({ x: x2, y: y2, width: w2, height: h2 }); | ||
| }); | ||
| }, 400); | ||
| }); | ||
| }, 600); | ||
| return () => { | ||
| clearTimeout(timer); | ||
| clearTimeout(remeasureTimer); | ||
| }; | ||
| }, [screenHeight, scrollOffset, scrollRef, targetRef]); |
There was a problem hiding this comment.
The logic to measure the target component and scroll it into view uses nested setTimeouts with fixed durations (600ms and 400ms). This can be unreliable, as scroll animations or layout calculations might take longer on different devices or under heavy load, leading to the spotlight being misplaced. A more robust approach could involve using onLayout to get initial measurements and a more declarative way to handle scrolling and re-measuring, if available, or using a library designed for this purpose.
There was a problem hiding this comment.
Actionable comments posted: 34
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/swaps/Swaps.tsx (2)
231-249:⚠️ Potential issue | 🟠 MajorDuplicate
useEffect— the route-sync effect is defined twice with identical deps and logic.Lines 231–239 and 241–249 are exact duplicates. Every
routechange triggersupdateSwaptwice, which is unnecessary and could cause subtle bugs (e.g., extra re-renders or stale-closure issues if one is later modified independently). Remove one of them.Proposed fix: remove the duplicate effect
useEffect(() => { if (route) { if (activeInput === "from") { updateSwap((old) => ({ ...old, toAmount: route.toAmount ?? 0n, tool: route.tool ?? "" })); } else { updateSwap((old) => ({ ...old, fromAmount: route.fromAmount ?? 0n, tool: route.tool ?? "" })); } } }, [activeInput, route]); - - useEffect(() => { - if (route) { - updateSwap((old) => { - return activeInput === "from" - ? { ...old, toAmount: route.toAmount ?? 0n, tool: route.tool ?? "" } - : { ...old, fromAmount: route.fromAmount ?? 0n, tool: route.tool ?? "" }; - }); - } - }, [activeInput, route]);
530-539: 🧹 Nitpick | 🔵 TrivialUnreachable ternary branch —
caution && !acknowledgedis checked twice.Line 532 and line 536 both check
caution && !acknowledged, making the second branch (line 536–537) unreachable dead code. The first match at line 532 already handles this case.Proposed simplification
color={ disabled || (caution && !acknowledged) ? "$interactiveOnDisabled" : caution && acknowledged ? "$interactiveOnBaseErrorSoft" - : caution && !acknowledged - ? "$interactiveOnBaseErrorSoft" - : "$interactiveOnBaseBrandDefault" + : "$interactiveOnBaseBrandDefault" }src/utils/usePortfolio.ts (1)
64-64: 🧹 Nitpick | 🔵 TrivialDuplicated WETH→ETH normalization logic.
The same
symbol.slice(3) === "WETH" ? "ETH" : symbol.slice(3)expression appears in both theportfoliomemo (Line 64) andprotocolAssetsmemo (Line 97). Consider extracting a small helper to keep this DRY.♻️ Proposed helper
Add at the bottom of the file:
const normalizeSymbol = (symbol: string) => symbol.slice(3) === "WETH" ? "ETH" : symbol.slice(3);Then replace both occurrences:
- depositMarkets.push({ market, symbol: symbol.slice(3) === "WETH" ? "ETH" : symbol.slice(3), usdValue }); + depositMarkets.push({ market, symbol: normalizeSymbol(symbol), usdValue });- symbol: market.symbol.slice(3) === "WETH" ? "ETH" : market.symbol.slice(3), + symbol: normalizeSymbol(market.symbol),Also applies to: 97-97
| rightOf: Exa Card pay mode | ||
| - assertVisible: Change the pay mode before each purchase and pay how you want. | ||
| - tapOn: Close | ||
|
|
There was a problem hiding this comment.
Remove trailing blank line.
YAMLlint flags a trailing empty line at the end of the file.
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 72-72: too many blank lines (1 > 0)
(empty-lines)
| - extendedWaitUntil: | ||
| visible: 30 days of free travel insurance | ||
| timeout: 15000 | ||
| - tapOn: 30 days of free travel insurance | ||
| - assertVisible: Copy your ID and get 30 days of travel insurance for free on Pax Assistance. | ||
| - tapOn: COPY ID | ||
| - runFlow: { file: tapAria.yaml, env: { aria: Close } } | ||
| - extendedWaitUntil: | ||
| visible: 20% OFF on eSims | ||
| timeout: 15000 | ||
| - tapOn: 20% OFF on eSims | ||
| - assertVisible: Stay connected around the world. | ||
| - assertVisible: Terms & conditions | ||
| - runFlow: { file: tapAria.yaml, env: { aria: Close } } | ||
| - extendedWaitUntil: | ||
| visible: Visa Signature benefits | ||
| timeout: 15000 | ||
| - tapOn: Visa Signature benefits | ||
| - assertVisible: Visa Signature Exa Card benefits | ||
| - assertVisible: A world of benefits. | ||
| - runFlow: { file: tapAria.yaml, env: { aria: Close } } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Benefits carousel content is hardcoded — consider parameterizing if content is dynamic.
The test asserts specific benefit strings ("30 days of free travel insurance", "20% OFF on eSims", "Visa Signature benefits"). If marketing content changes, these tests will break silently. This is fine for stable content, but worth noting for maintenance.
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 39-39: too many spaces inside braces
(braces)
[error] 39-39: too many spaces inside braces
(braces)
[error] 39-39: too many spaces inside braces
(braces)
[error] 39-39: too many spaces inside braces
(braces)
[error] 46-46: too many spaces inside braces
(braces)
[error] 46-46: too many spaces inside braces
(braces)
[error] 46-46: too many spaces inside braces
(braces)
[error] 46-46: too many spaces inside braces
(braces)
[error] 53-53: too many spaces inside braces
(braces)
[error] 53-53: too many spaces inside braces
(braces)
[error] 53-53: too many spaces inside braces
(braces)
[error] 53-53: too many spaces inside braces
(braces)
| - copyTextFrom: | ||
| text: \d+[,.]\d{2} | ||
| above: "${output.maturity}" | ||
| text: \$[\d,]+\.\d{2} | ||
| below: Upcoming payments | ||
| leftOf: Repay | ||
| - assertTrue: ${Math.abs(output.debt - Number(maestro.copiedText) - Number(amount)) < 0.02} | ||
| rightOf: "${output.maturity}" | ||
| - assertTrue: ${Math.abs(output.debt - Number(maestro.copiedText.replace(/[^\d.]/g, '')) - Number(amount)) < 0.02} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the Maestro config or CI sets a specific locale
rg -n -i 'locale\|LANG\|LC_' .maestro/ --include='*.yaml' --include='*.yml' || echo "No locale config found in .maestro/"
echo "---"
rg -n -i 'locale\|LANG\|LC_' .github/ -g '*.yml' -g '*.yaml' || echo "No locale config found in .github/"Repository: exactly/exa
Length of output: 588
🏁 Script executed:
# Search for Maestro configuration files and test setup
find . -name ".maestro" -o -name "maestro.yaml" -o -name "maestro.yml" | head -20Repository: exactly/exa
Length of output: 65
🏁 Script executed:
# Check for any environment variable setup or locale handling in test files
rg -n 'Locale\|locale\|LANG' . --include='*.yaml' --include='*.yml' --include='*.java' --include='*.kt' --include='*.js' --include='*.ts' --max-count=20 || echo "No locale settings found"Repository: exactly/exa
Length of output: 538
🏁 Script executed:
# Read the repay.yaml file to understand the context
cat -n .maestro/subflows/repay.yamlRepository: exactly/exa
Length of output: 2296
🏁 Script executed:
# Search for Maestro references or documentation
find . -type f \( -name "README*" -o -name "*.md" \) | head -10 | xargs grep -l "Maestro" 2>/dev/null || echo "No README found"Repository: exactly/exa
Length of output: 70
🏁 Script executed:
# Check for Maestro configuration in common locations
ls -la .maestro/Repository: exactly/exa
Length of output: 787
🏁 Script executed:
# Search for any test setup, runner config, or environment files
find . -maxdepth 2 -type f \( -name "*.config.*" -o -name "setup.*" -o -name "test.env" -o -name ".env*" \) 2>/dev/null | head -20Repository: exactly/exa
Length of output: 481
🏁 Script executed:
# Check Maestro config
cat .maestro/config.yamlRepository: exactly/exa
Length of output: 106
🏁 Script executed:
# Check package.json for test setup and locale info
cat package.json | grep -A 20 '"scripts"' || cat package.jsonRepository: exactly/exa
Length of output: 1611
Fix number parsing to handle non-US locales used in e2e tests.
Line 55 asserts output.debt - Number(maestro.copiedText.replace(/[^\d.]/g, '')) - Number(amount) < 0.02. The regex /[^\d.]/g assumes US number format, but the e2e:iphone test script explicitly uses --device-locale=es-419 (Spanish, Latin America), which formats numbers with a comma as the decimal separator and dot as the thousands separator (e.g., $1.234,56). In this locale, the regex would strip the comma (the actual decimal separator) but preserve the dot (a thousands separator), producing an incorrect number like 1.23456 instead of 1234.56, causing the assertion to fail.
Additionally, the copyTextFrom pattern on line 52 (\$[\d,]+\.\d{2}) assumes US format and would not match the es-419 format, which could cause upstream failures. The number parsing needs to be locale-aware or the locale configuration should be standardized across all test environments.
| <XStack | ||
| height={96} | ||
| borderRadius="$r4" | ||
| overflow="hidden" | ||
| alignItems="center" | ||
| justifyContent="flex-end" | ||
| backgroundColor="#1A181A" | ||
| > | ||
| <View position="absolute" top={0} left={0} right={0} bottom={0} alignItems="center" justifyContent="center"> | ||
| <CardBg width="100%" height="100%" preserveAspectRatio="xMidYMid meet" /> | ||
| </View> | ||
| <Exa | ||
| width={50} | ||
| height={20} | ||
| style={styles.exa} | ||
| {...(Platform.OS === "web" ? undefined : { shouldRasterizeIOS: true })} | ||
| /> |
There was a problem hiding this comment.
Hardcoded color #1A181A bypasses theming.
This won't adapt to light/dark mode changes. Consider using a theme token (e.g., $backgroundInverse or a dedicated card-background token) instead.
| <View | ||
| position="absolute" | ||
| right={0} | ||
| left={0} | ||
| top={0} | ||
| bottom={0} | ||
| justifyContent="flex-end" | ||
| cursor="pointer" | ||
| top={2} | ||
| bottom={2} | ||
| left={2} | ||
| right={2} | ||
| width={width / 2 - 6} | ||
| borderRadius="$r_0" | ||
| backgroundColor={isDebit ? "$cardDebitInteractive" : "$cardCreditInteractive"} | ||
| x={isDebit ? 0 : width / 2} | ||
| animation="default" | ||
| animateOnly={["transform"]} | ||
| /> |
There was a problem hiding this comment.
Indicator width is negative before first layout.
width / 2 - 6 evaluates to -6 while width is still 0 (before onLayout fires). This briefly produces a negative-width View. Guard with Math.max(0, ...) or skip rendering the indicator until width > 0.
Proposed fix
- width={width / 2 - 6}
+ width={Math.max(0, width / 2 - 6)}| async function setCardMode(mode: number) { | ||
| await auth(); | ||
| const response = await api.card.$patch({ json: { mode } }); | ||
| if (!response.ok) throw new APIError(response.status, stringOrLegacy(await response.json())); | ||
| return response.json(); | ||
| } | ||
| export const cardModeMutationOptions = { | ||
| mutationKey: ["card", "mode"] as const, | ||
| mutationFn: setCardMode, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for cardMode usage and setCardMode calls
rg -n "cardMode|setCardMode" --type=ts --type=tsx -C3Repository: exactly/exa
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Search for card mode related constants or enums
rg -n "(mode.*=|const.*mode)" --type=ts --type=tsx -B2 -A2 | head -100Repository: exactly/exa
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Search for cardModeMutationOptions usage
rg -n "cardModeMutationOptions" -C5Repository: exactly/exa
Length of output: 3092
🏁 Script executed:
#!/bin/bash
# Find mutateMode calls to see what values are passed
rg -n "mutateMode\(" -C3Repository: exactly/exa
Length of output: 1051
🏁 Script executed:
#!/bin/bash
# Search for installment or mode related constants
rg -n "installment|MODE|mode.*=" --type=ts -i -C1 | head -150Repository: exactly/exa
Length of output: 9286
🏁 Script executed:
#!/bin/bash
# Search for CardDetails type definition
rg -n "CardDetails" --type=ts -A5 | head -60Repository: exactly/exa
Length of output: 4103
🏁 Script executed:
#!/bin/bash
# Search for onModeChange handler definitions
rg -n "onModeChange" --type=ts -B3 -A3Repository: exactly/exa
Length of output: 5510
🏁 Script executed:
#!/bin/bash
# Check if there are any mode-related type definitions or constants
rg -n "type.*[Mm]ode|enum.*[Mm]ode" --type=ts -A3Repository: exactly/exa
Length of output: 297
🏁 Script executed:
#!/bin/bash
# Find the getCard function to see what CardDetails includes
rg -n "async function getCard|function getCard" -A20 --type=tsRepository: exactly/exa
Length of output: 2482
🏁 Script executed:
#!/bin/bash
# Check what the API card response looks like
rg -n "api.card" --type=ts -B2 -A2 | head -80Repository: exactly/exa
Length of output: 2187
🏁 Script executed:
#!/bin/bash
# Check the API card route definition for mode parameter validation
rg -n "server/api/card" --type=ts -lRepository: exactly/exa
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Look at the card API route to understand mode validation
cat server/api/card.ts | head -150Repository: exactly/exa
Length of output: 4985
🏁 Script executed:
#!/bin/bash
# Check if MAX_INSTALLMENTS is defined in the client-side code
rg -n "MAX_INSTALLMENTS" --type=ts -B2 -A2Repository: exactly/exa
Length of output: 6541
🏁 Script executed:
#!/bin/bash
# Search for any type aliases or enums related to card mode in client code
rg -n "type.*Mode|enum.*Mode" --type=ts -A5Repository: exactly/exa
Length of output: 37
mode: number should be narrowed to a union type for better type safety.
Card mode values are a known set: 0 for immediate payment or 1 through MAX_INSTALLMENTS for installment counts. While the API validates this range server-side, the client-side parameter remains unbounded. Define a type alias (e.g., type CardMode = 0 | 1 | 2 | 3 | 4 | 5 | 6; or generate from MAX_INSTALLMENTS) and use it for setCardMode and related callbacks to prevent invalid values during development.
| queryClient.setMutationDefaults(cardModeMutationOptions.mutationKey, { | ||
| ...cardModeMutationOptions, | ||
| onMutate: async (newMode: number) => { | ||
| await queryClient.cancelQueries({ queryKey: ["card", "details"] }); | ||
| const previous = queryClient.getQueryData(["card", "details"]); | ||
| queryClient.setQueryData(["card", "details"], (old: CardDetails) => ({ ...old, mode: newMode })); | ||
| return { previous }; | ||
| }, | ||
| onError: (error, _, context) => { | ||
| if (context?.previous) queryClient.setQueryData(["card", "details"], context.previous); | ||
| reportError(error); | ||
| }, | ||
| onSettled: async (data) => { | ||
| await queryClient.invalidateQueries({ queryKey: ["card", "details"] }); | ||
| if (data && "mode" in data && data.mode > 0) queryClient.setQueryData(["settings", "installments"], data.mode); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Optimistic update can corrupt card details when cache is empty.
On Line 144, old can be undefined (query not yet fetched) or null (returned by getCard for missing card). Spreading undefined/null produces { mode: newMode }, silently wiping all other card fields from the cache and causing downstream UI breakage.
🐛 Proposed fix
onMutate: async (newMode: number) => {
await queryClient.cancelQueries({ queryKey: ["card", "details"] });
const previous = queryClient.getQueryData(["card", "details"]);
- queryClient.setQueryData(["card", "details"], (old: CardDetails) => ({ ...old, mode: newMode }));
+ queryClient.setQueryData(["card", "details"], (old: CardDetails) => old ? { ...old, mode: newMode } : old);
return { previous };
},| const { fixedPools, floatingUtilization, totalFloatingDepositAssets, totalFloatingBorrowAssets } = market; | ||
| const { floatingBackupBorrowed, interestRateModel } = market; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Combine destructuring into a single statement.
Two consecutive destructurings from the same market object can be merged for compactness.
♻️ Proposed fix
- const { fixedPools, floatingUtilization, totalFloatingDepositAssets, totalFloatingBorrowAssets } = market;
- const { floatingBackupBorrowed, interestRateModel } = market;
+ const { fixedPools, floatingUtilization, totalFloatingDepositAssets, totalFloatingBorrowAssets, floatingBackupBorrowed, interestRateModel } = market;As per coding guidelines, "enforce maximally compact code style; do not introduce line breaks inside objects, arrays, or function arguments voluntarily".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { fixedPools, floatingUtilization, totalFloatingDepositAssets, totalFloatingBorrowAssets } = market; | |
| const { floatingBackupBorrowed, interestRateModel } = market; | |
| const { fixedPools, floatingUtilization, totalFloatingDepositAssets, totalFloatingBorrowAssets, floatingBackupBorrowed, interestRateModel } = market; |
| const aSymbol = a.symbol; | ||
| const bSymbol = b.symbol; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inline trivial property accesses instead of extracting them.
After the refactor, aSymbol and bSymbol are just a.symbol and b.symbol — trivial property accesses that no longer warrant local variables. The previous version needed extraction because it had conditional type-check + slice logic.
♻️ Proposed inline
return combined.sort((a, b) => {
if (options?.sortBy === "usdcFirst") {
- const aSymbol = a.symbol;
- const bSymbol = b.symbol;
- if (aSymbol === "USDC" && bSymbol !== "USDC") return -1;
- if (bSymbol === "USDC" && aSymbol !== "USDC") return 1;
+ if (a.symbol === "USDC" && b.symbol !== "USDC") return -1;
+ if (b.symbol === "USDC" && a.symbol !== "USDC") return 1;
}As per coding guidelines, "keep single-use values or expressions at the point of use (inline by default); extract only for reuse or genuine complexity".
| 400: { normal: "SplineSans-Regular" }, | ||
| 600: { normal: "SplineSans-SemiBold" }, | ||
| 700: { normal: "SplineSans-SemiBold" }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Search for SplineSans font files in the project
fd -i "splinesans" --type fRepository: exactly/exa
Length of output: 135
🏁 Script executed:
# Also search more broadly for font files and assets
fd -i "font" --type d | head -20Repository: exactly/exa
Length of output: 72
🏁 Script executed:
# Look at the tamagui.config.ts file to understand the full font configuration context
find . -name "tamagui.config.ts" -type fRepository: exactly/exa
Length of output: 74
🏁 Script executed:
# Read tamagui.config.ts around the font configuration (lines 215-235)
sed -n '210,240p' tamagui.config.tsRepository: exactly/exa
Length of output: 1125
No Bold font variant available—weight 700 falls back to SemiBold.
The project assets include only SplineSans-Regular.otf and SplineSans-SemiBold.otf. The mapping of weight 700 (labeled bold) to SplineSans-SemiBold is the only viable fallback. Consider adding a SplineSans-Bold.otf variant to the assets if true bold text is needed for design requirements.
Summary by CodeRabbit
New Features
Style
Refactor