Gas tank Page #491
Gas tank Page #491
Conversation
📝 WalkthroughWalkthroughAdds a new Gas Tank app with UI components (AppWrapper, HomeScreen, balance/history cards), data types and hooks (useGasTankHistory), PillarX integration (tile, manifest, routing), and UI wiring for top-up, onboarding, polling, and portfolio loading. Changes
Sequence Diagram(s)sequenceDiagram
participant AppWrapper
participant Wallet as Wallet/Account
participant Paymaster as Paymaster API
participant HistoryAPI as Paymaster History API
participant UI as HomeScreen/Cards
AppWrapper->>Wallet: request accountAddress
Wallet-->>AppWrapper: accountAddress
alt has accountAddress
AppWrapper->>Paymaster: fetch gas tank balance
Paymaster-->>AppWrapper: balance
AppWrapper->>HistoryAPI: fetch gas tank history
HistoryAPI-->>AppWrapper: raw transactions
AppWrapper->>AppWrapper: process & normalize transactions
AppWrapper->>UI: render balance, history, loading states
else no accountAddress
AppWrapper->>UI: render unauthenticated state
end
UI->>AppWrapper: user triggers TopUp
AppWrapper->>UI: show TopUpScreen
UI->>Paymaster: refetch balance/history on completion
Paymaster-->>UI: updated data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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.
Actionable comments posted: 13
Fix all issues with AI Agents 🤖
In @src/apps/gas-tank/components/App/HomeScreen.tsx:
- Around line 76-79: The onBack handler only calls setOnboardingScreen(null) and
refetchGasTankBalance(); update it to also refresh the transaction history after
returning from TopUpScreen by calling the transaction-refresh function (e.g.,
refetchTransactionHistory or refetchGasTankTransactions) in the same block so
the new top-up appears in the history; modify the onBack callback where
setOnboardingScreen and refetchGasTankBalance are called to invoke that refetch
function as well.
- Line 98: The top-of-file comment that reads "// ... existing code ..." in the
HomeScreen component is misleading because this is a new file; remove or replace
that comment with a clear, accurate description (or delete it entirely). Locate
the placeholder comment in src/apps/gas-tank/components/App/HomeScreen.tsx
around the HomeScreen component declaration and either delete the "// ...
existing code ..." line or replace it with a concise comment describing the
component's purpose.
In @src/apps/gas-tank/components/History/GasTankHistoryCard.tsx:
- Around line 40-43: The component incorrectly uses useMemo to perform a side
effect—resetting state—so replace the useMemo block with a useEffect that
watches transactions and calls setDisplayedLimit(10); specifically change the
existing useMemo(() => { setDisplayedLimit(10); }, [transactions]) to
useEffect(() => { setDisplayedLimit(10); }, [transactions]) so the state reset
runs as an effect when the transactions array changes.
- Around line 62-75: The switch in GasTankHistoryCard (the sort comparator
handling cases 'amount' and 'token') declares consts inside cases which can leak
scope; fix by wrapping each affected case body in its own block (e.g., case
'amount': { /* compute aAmount, bAmount, aValue, bValue */ break; }) or
alternatively declare aAmount/bAmount/aValue/bValue before the switch and assign
inside cases so no block-scoped declarations cross case boundaries; update the
cases 'amount' and 'token' accordingly.
In @src/apps/gas-tank/components/History/GasTankSkeleton.tsx:
- Around line 1-8: The file uses the React.FC type for the
SkeletonTransactionRow component but never imports React; add an import for
React (e.g., import React from "react") at the top of the file so the React.FC
type is defined and the component compiles and runs correctly.
In @src/apps/gas-tank/components/History/TransactionRow.tsx:
- Around line 37-43: The TransactionRow clickable container lacks keyboard
activation: add an onKeyDown handler on the same div (where role, tabIndex and
onClick are set) that, when isClickable is true, listens for Enter (Enter or
NumpadEnter) and Space keys, prevents default for Space, and invokes
handleClick; ensure the handler is only attached/active when isClickable is true
and reuse the existing handleClick function so mouse and keyboard activation
behave identically.
- Around line 1-7: The file uses the React.FC type for the component but does
not import React; add an import for React (e.g., add "import React from
'react';" at the top) or change the component signature to remove React.FC and
use a plain function with TransactionRowProps; update the TransactionRow
declaration (the component using React.FC) so the type is resolved by either
importing React or switching to "function TransactionRow(props:
TransactionRowProps) { ... }".
In @src/apps/gas-tank/hooks/useGasTankHistory.ts:
- Around line 134-137: The useEffect that calls fetchHistory is missing
fetchHistory in its dependency array; since fetchHistory is recreated each
render and captures walletAddress, wrap fetchHistory in useCallback (e.g., const
fetchHistory = useCallback(..., [walletAddress, /* other deps */])) so it is
stable, then update the useEffect to depend on fetchHistory (or include
fetchHistory in the dependency array) to satisfy ESLint and ensure correct
behavior.
In @src/apps/pillarx-app/index.tsx:
- Around line 130-138: The gasTankTile Projection currently forces type-safety
off by casting data to any; instead remove the data property or set it to
undefined, or supply a correctly-typed value matching Projection's allowed types
(e.g., TokenData[] | Advertisement | MediaGridData | Points | TokensMarketData |
AlgoInsightsData) for the GAS_TANK tile; update the gasTankTile object (id:
'gas-tank-tile', layout: ApiLayout.GAS_TANK) to omit data or assign a properly
typed structure rather than using "as any".
In @src/apps/pulse/components/Onboarding/TopUpScreen.tsx:
- Line 64: The prop hasPortfolioData is declared in the component props and
destructured in the TopUpScreen component but never used; remove it from the
Props/interface/type definition and from the function parameter destructuring in
TopUpScreen (and from any defaultProps or prop spread in that component) so the
component signature no longer exposes hasPortfolioData; also update any local
references or tests that expect this prop.
In @src/components/AppIcon.tsx:
- Around line 26-29: Remove the hardcoded special-case branch that checks appId
=== 'gas-tank' inside the AppIcon component (the block that calls
setIconSrc(GasTankIcon) and returns); instead let the existing standard
icon-loading flow (the dynamic import logic that loads ../apps/${appId}/icon.png
and sets icon via setIconSrc) handle gas-tank the same as other internal apps,
and ensure an icon.png is added at src/apps/gas-tank/icon.png so the dynamic
import resolves successfully.
In @src/providers/AllowedAppsProvider.tsx:
- Around line 89-125: Extract the duplicated defaultApps array into a single
constant (e.g., DEFAULT_ALLOWED_APPS) outside the try/catch in
AllowedAppsProvider.tsx and reuse it in both the success and error paths; update
the allApps computation to use DEFAULT_ALLOWED_APPS (const allApps =
data?.length ? [...DEFAULT_ALLOWED_APPS, ...data] : DEFAULT_ALLOWED_APPS) and
call setAllowed(DEFAULT_ALLOWED_APPS) in the catch branch (when !expired),
ensuring the type ApiAllowedApp is preserved.
- Around line 90-102: The gas-tank entry in AllowedAppsProvider's defaultApps
includes an unused logo property which is ignored by AppIcon's special-case for
appId === 'gas-tank'; remove the dead `logo` field from the gas-tank object in
defaultApps (or alternatively update AppIcon's special-case to import the same
path declared there or refactor gas-tank to the standard dynamic icon pattern)
so the provider and AppIcon are consistent; locate the gas-tank object in
defaultApps inside AllowedAppsProvider and remove the `logo:
'../apps/gas-tank/assets/gas-tank-icon.svg'` key (or change AppIcon’s
special-case to use that path or migrate gas-tank to dynamic icon loading).
🧹 Nitpick comments (8)
src/apps/pulse/components/Search/Search.tsx (1)
469-472: Apply the same strict type checking to the Asset branch for consistency.The Token branch now correctly validates both property existence and numeric type. However, the Asset branch (lines 449-450) still uses the permissive check
'priceChange24h' in item ? item.priceChange24h || 0.0 : 0.0, which doesn't verify the type. IfpriceChange24hexists but is not a number (e.g., null, undefined, or a string), it could propagate an incorrect type todailyPriceChange.🔎 Proposed fix to align Asset branch with Token branch
dailyPriceChange: - 'priceChange24h' in item ? item.priceChange24h || 0.0 : 0.0, + 'priceChange24h' in item && + typeof item.priceChange24h === 'number' + ? item.priceChange24h + : 0.0,src/apps/pulse/hooks/useGasTankBalance.ts (1)
93-105: Optional: Consider visibility-based polling and configurable intervals.The 30-second polling interval runs continuously regardless of component visibility or user activity. Consider these optional enhancements:
- Pause polling when the page/tab is not visible using the Page Visibility API
- Make the polling interval configurable via a parameter or environment variable
- Add exponential backoff on consecutive errors
These improvements can reduce unnecessary API calls and provide better resilience.
Example: Add visibility-based polling
// Polling: Refetch balance every 30 seconds useEffect(() => { if (!walletAddress || !paymasterUrl) return; const POLL_INTERVAL = 30000; // 30 seconds + let intervalId: NodeJS.Timeout | null = null; + + const startPolling = () => { + if (intervalId) return; + intervalId = setInterval(() => { + fetchGasTankBalance(); + }, POLL_INTERVAL); + }; + + const stopPolling = () => { + if (intervalId) { + clearInterval(intervalId); + intervalId = null; + } + }; + + const handleVisibilityChange = () => { + if (document.hidden) { + stopPolling(); + } else { + startPolling(); + } + }; + + document.addEventListener('visibilitychange', handleVisibilityChange); + startPolling(); - const intervalId = setInterval(() => { - fetchGasTankBalance(); - }, POLL_INTERVAL); // Cleanup interval on unmount - return () => clearInterval(intervalId); + return () => { + stopPolling(); + document.removeEventListener('visibilitychange', handleVisibilityChange); + }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [walletAddress, paymasterUrl]);src/apps/pillarx-app/components/GasTankTile/GasTankTile.tsx (3)
1-1: Remove empty line at the start of the file.Unnecessary blank line at the top of the file.
🔎 Proposed fix
- import React from 'react';
26-27: Consider renamingformatTransactionsto better reflect its purpose.The variable slices data but doesn't format it. A name like
displayedTransactionsorpreviewTransactionswould be clearer.🔎 Proposed fix
// Take first 5 transactions - const formatTransactions = transactions.slice(0, 5); + const previewTransactions = transactions.slice(0, 5);Then update usages on lines 98 and 102:
- ) : formatTransactions.length === 0 ? ( + ) : previewTransactions.length === 0 ? (- {formatTransactions.map((tx) => ( + {previewTransactions.map((tx) => (
149-164: Consider reusing skeleton components from GasTankSkeleton.tsx.The
SkeletonBalancestyled component duplicates pulse animation logic that already exists inGasTankSkeleton.tsx. However, since this is a simplified version for the tile context, the duplication is acceptable.src/apps/gas-tank/components/Balance/GasTankBalanceCard.tsx (1)
22-25: Consider guarding against NaN in totalSpend calculation.If
tx.usdcAmountcontains an invalid numeric string,parseFloatreturnsNaN, which would corrupt the sum. Consider adding a fallback.🔎 Proposed fix
const totalSpend = transactions .filter((tx) => tx.type === 'Spend') - .reduce((sum, tx) => sum + parseFloat(tx.usdcAmount), 0); + .reduce((sum, tx) => sum + (parseFloat(tx.usdcAmount) || 0), 0);src/apps/gas-tank/components/App/HomeScreen.tsx (1)
11-32: Consider reducing prop count through composition or context.The interface defines 20+ props, which indicates tight coupling and potential prop drilling. Consider grouping related state (e.g., search-related, token-related, portfolio-related) into objects or using React Context for widely-shared state.
💡 Example refactoring approach
Group related props into cohesive objects:
interface SearchState { searching: boolean; setSearching: Dispatch<SetStateAction<boolean>>; isSearchingFromTopup: boolean; setIsSearchingFromTopup: Dispatch<SetStateAction<boolean>>; } interface TokenState { buyToken: SelectedToken | null; setBuyToken: Dispatch<SetStateAction<SelectedToken | null>>; sellToken: SelectedToken | null; setSellToken: Dispatch<SetStateAction<SelectedToken | null>>; topupToken: SelectedToken | null; setTopupToken: Dispatch<SetStateAction<SelectedToken | null>>; isBuy: boolean; setIsBuy: Dispatch<SetStateAction<boolean>>; } interface HomeScreenProps { accountAddress: string | null; searchState: SearchState; tokenState: TokenState; // ... other grouped props }Alternatively, consider using React Context for shared state like portfolio and balance data that multiple components need.
src/providers/AllowedAppsProvider.tsx (1)
106-106: Remove unnecessary identity map operation.The
.map((app: ApiAllowedApp) => app)is a no-op that simply returns the same array without transformation.🔎 Proposed fix
- const allApps = data?.length ? [...defaultApps, ...data] : defaultApps; - setAllowed(allApps.map((app: ApiAllowedApp) => app)); + const allApps = data?.length ? [...defaultApps, ...data] : defaultApps; + setAllowed(allApps);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/gas-tank/assets/gas-tank-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/history.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
src/apps/gas-tank/components/App/AppWrapper.tsxsrc/apps/gas-tank/components/App/HomeScreen.tsxsrc/apps/gas-tank/components/Balance/GasTankBalanceCard.tsxsrc/apps/gas-tank/components/History/GasTankHistoryCard.tsxsrc/apps/gas-tank/components/History/GasTankSkeleton.tsxsrc/apps/gas-tank/components/History/TransactionRow.tsxsrc/apps/gas-tank/hooks/useGasTankHistory.tssrc/apps/gas-tank/index.tsxsrc/apps/gas-tank/manifest.jsonsrc/apps/gas-tank/types/gasTank.tssrc/apps/pillarx-app/components/GasTankTile/GasTankTile.tsxsrc/apps/pillarx-app/index.tsxsrc/apps/pillarx-app/utils/configComponent.tssrc/apps/pulse/components/Onboarding/TopUpScreen.tsxsrc/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/hooks/useGasTankBalance.tssrc/components/AppIcon.tsxsrc/providers/AllowedAppsProvider.tsxsrc/types/api.ts
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
📚 Learning: 2025-08-20T14:01:31.715Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 387
File: src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx:118-125
Timestamp: 2025-08-20T14:01:31.715Z
Learning: In src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx, the "My rank" calculation uses 'trading' context while the list uses 'migration' context when calling calculateFinalPxPoints. This is intentional design to control where the 200-point bonus is applied - the 'trading' context with 'all' timeTab bypasses the bonus while 'migration' context allows it, preventing points from being added in the wrong place.
Applied to files:
src/types/api.ts
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pillarx-app/utils/configComponent.tssrc/apps/pillarx-app/components/GasTankTile/GasTankTile.tsxsrc/apps/pulse/components/Onboarding/TopUpScreen.tsxsrc/apps/pillarx-app/index.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required title field. The TileTitleProps structure aligns with how it's used in the TokensMarketData type in api.ts.
Applied to files:
src/apps/pillarx-app/utils/configComponent.tssrc/apps/pillarx-app/components/GasTankTile/GasTankTile.tsxsrc/apps/pillarx-app/index.tsx
📚 Learning: 2025-12-03T10:01:15.801Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 471
File: src/apps/pillarx-app/components/AlgoInsightsTile/AlgoInsightsTile.tsx:36-48
Timestamp: 2025-12-03T10:01:15.801Z
Learning: In the AlgoInsightsTile component (src/apps/pillarx-app/components/AlgoInsightsTile/AlgoInsightsTile.tsx), the data is always hardcoded and guaranteed to be present with complete history arrays for all timeframes, so edge case handling for empty or sparse history data is not required.
Applied to files:
src/apps/pillarx-app/utils/configComponent.tssrc/apps/pillarx-app/components/GasTankTile/GasTankTile.tsxsrc/apps/pillarx-app/index.tsx
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/pillarx-app/components/GasTankTile/GasTankTile.tsxsrc/apps/gas-tank/components/App/AppWrapper.tsxsrc/apps/gas-tank/components/Balance/GasTankBalanceCard.tsxsrc/apps/gas-tank/components/History/TransactionRow.tsxsrc/apps/gas-tank/components/History/GasTankSkeleton.tsx
📚 Learning: 2025-12-11T12:40:09.964Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/gas-tank/components/App/AppWrapper.tsxsrc/apps/gas-tank/components/App/HomeScreen.tsxsrc/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
🧬 Code graph analysis (8)
src/apps/gas-tank/components/App/AppWrapper.tsx (3)
src/apps/pulse/hooks/useGasTankBalance.ts (1)
useGasTankBalance(22-114)src/apps/key-wallet/utils/blockchain.ts (1)
chains(32-34)src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)
src/apps/gas-tank/hooks/useGasTankHistory.ts (1)
src/apps/gas-tank/types/gasTank.ts (4)
UseGasTankHistoryReturn(67-72)ProcessedGasTankTransaction(52-62)GasTankHistoryResponse(45-47)GasTankHistoryTransaction(28-40)
src/apps/gas-tank/components/Balance/GasTankBalanceCard.tsx (1)
src/apps/gas-tank/types/gasTank.ts (1)
ProcessedGasTankTransaction(52-62)
src/apps/gas-tank/components/History/TransactionRow.tsx (2)
src/apps/gas-tank/types/gasTank.ts (1)
ProcessedGasTankTransaction(52-62)src/utils/blockchain.ts (2)
getLogoForChainId(165-199)getBlockScan(225-246)
src/apps/gas-tank/components/History/GasTankHistoryCard.tsx (3)
src/apps/gas-tank/types/gasTank.ts (1)
ProcessedGasTankTransaction(52-62)src/apps/gas-tank/components/History/GasTankSkeleton.tsx (1)
SkeletonTransactionRow(8-32)src/apps/gas-tank/components/History/TransactionRow.tsx (1)
TransactionRow(14-95)
src/apps/gas-tank/index.tsx (1)
src/apps/gas-tank/components/App/AppWrapper.tsx (1)
AppWrapper(18-136)
src/apps/gas-tank/components/App/HomeScreen.tsx (5)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/services/tokensData.ts (1)
PortfolioToken(31-34)src/apps/gas-tank/hooks/useGasTankHistory.ts (1)
useGasTankHistory(14-145)src/apps/gas-tank/components/Balance/GasTankBalanceCard.tsx (1)
GasTankBalanceCard(16-100)src/apps/gas-tank/components/History/GasTankHistoryCard.tsx (1)
GasTankHistoryCard(26-252)
src/apps/pillarx-app/index.tsx (1)
src/types/api.ts (1)
Projection(223-236)
🪛 Biome (2.1.2)
src/apps/gas-tank/components/History/GasTankHistoryCard.tsx
[error] 64-64: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 65-65: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (20)
src/apps/pulse/components/Search/Search.tsx (1)
469-472: Verify that this change belongs in the Gas Tank PR.This change to the Pulse search component appears unrelated to the PR objectives, which focus on adding a Gas Tank page feature. Please confirm whether this change should be included in this PR or submitted separately.
src/apps/gas-tank/types/gasTank.ts (1)
1-72: LGTM! Well-structured type definitions.The type hierarchy is well-designed with a clear separation between raw API data (
GasTankHistoryTransactionwith flexible types) and display-ready data (ProcessedGasTankTransactionwith normalized types). This pattern provides good flexibility for handling API variations while ensuring type safety at the UI layer.src/types/api.ts (1)
11-11: LGTM! Consistent enum extension.The addition of
GAS_TANKto theApiLayoutenum follows the established pattern and properly integrates with the tile mapping system inconfigComponent.ts.src/apps/gas-tank/manifest.json (1)
1-5: LGTM! Clean manifest configuration.The manifest provides clear app metadata with an appropriate title and description. The empty translations object is acceptable for the initial implementation and can be populated when internationalization is needed.
src/apps/gas-tank/index.tsx (1)
1-7: LGTM! Clean app entry point.The entry point correctly re-exports the
AppWrappercomponent with clear documentation. This follows the established pattern for app modules in the codebase.src/apps/pillarx-app/utils/configComponent.ts (1)
14-14: LGTM! Proper tile integration.The
GasTankTileimport and mapping follow the established pattern for tile components in this configuration. The mapping correctly connects theApiLayout.GAS_TANKenum value to theGasTankTilecomponent, enabling it to be rendered in the PillarX home feed.Also applies to: 30-30
src/apps/pillarx-app/components/GasTankTile/GasTankTile.tsx (1)
33-110: LGTM!The component structure is well-organized with clear separation of left (balance) and right (history) panels. Loading and empty states are handled correctly, and the responsive layout using Tailwind breakpoints is appropriate.
src/apps/gas-tank/hooks/useGasTankHistory.ts (2)
67-132: LGTM on transaction processing logic.The
processTransactionsfunction handles both Deposit and TransactionRepayment types correctly, with appropriate amount formatting and swap data overrides.
90-91: USDC decimal handling is correct.The code properly assigns 18 decimals for BSC (chainId 56) and 6 decimals for other chains, which aligns with actual USDC token specifications on these networks.
src/apps/gas-tank/components/Balance/GasTankBalanceCard.tsx (1)
27-98: LGTM!The component structure is clean with proper loading states, and the conditional rendering of the "Total Spend" badge when
totalSpend > 0is a nice UX touch.src/apps/gas-tank/components/History/GasTankSkeleton.tsx (1)
37-85: LGTM!The skeleton components are well-structured with consistent styling. Good use of component composition with
SkeletonHistoryCardreusingSkeletonTransactionRow.src/apps/gas-tank/components/App/AppWrapper.tsx (3)
26-36: LGTM on smart loading state pattern.The
hasInitialLoadpattern correctly prevents UI flickering during refetches by only showing the loading state on the initial fetch.
74-78: Verify edge case when portfolio loads with zero tokens.The condition
portfolioTokens.length > 0 || (!walletPortfolioLoading && walletPortfolioData)handles empty portfolios, but ifwalletPortfolioDataexists without valid data,hasPortfolioLoadedmight be set prematurely. This seems intentional but worth verifying the expected behavior.
111-135: LGTM!The component correctly follows the established pattern from Pulse AppWrapper, with comprehensive prop passing to HomeScreen. Based on learnings, Transaction Kit providers are set up at container level, which this component correctly consumes.
src/apps/gas-tank/components/History/GasTankHistoryCard.tsx (2)
142-157: LGTM on infinite scroll implementation.The IntersectionObserver is correctly set up with proper cleanup. The threshold of 0.1 provides smooth loading experience.
213-249: LGTM!The content rendering with proper state handling (error, loading, empty, data) is well-implemented. The retry functionality for errors and infinite scroll loader sentinel are good UX additions.
src/apps/gas-tank/components/History/TransactionRow.tsx (1)
68-92: LGTM!The token column with the overlaid chain badge is a nice UI pattern. The fallback to USDC logo for missing token logos is appropriate.
src/apps/gas-tank/components/App/HomeScreen.tsx (2)
88-88: Verify if emptymarkOnboardingCompletecallback is intentional.The
markOnboardingCompleteprop is set to an empty function. Confirm whether TopUpScreen invokes this callback whenshowCloseButton={true}, or if this is a placeholder that should trigger additional logic (e.g., analytics, state updates).
101-139: LGTM! Main display layout is well-structured.The conditional rendering logic provides a smooth user experience:
- Skeleton components display only during initial loads
- When refreshing with existing data, the populated cards remain visible (line 126:
isHistoryLoading && transactions.length === 0)- Responsive layout with proper spacing and styling
src/components/AppIcon.tsx (1)
10-11: > Likely an incorrect or invalid review comment.
| onBack={() => { | ||
| setOnboardingScreen(null); | ||
| refetchGasTankBalance(); | ||
| }} |
There was a problem hiding this comment.
Refetch history after top-up to show new transactions.
When returning from TopUpScreen, only refetchGasTankBalance() is called. If the user completed a top-up, the transaction history should also be refreshed to display the new transaction.
🔎 Suggested fix
onBack={() => {
setOnboardingScreen(null);
refetchGasTankBalance();
+ refetchHistory();
}}🤖 Prompt for AI Agents
In @src/apps/gas-tank/components/App/HomeScreen.tsx around lines 76-79, The
onBack handler only calls setOnboardingScreen(null) and refetchGasTankBalance();
update it to also refresh the transaction history after returning from
TopUpScreen by calling the transaction-refresh function (e.g.,
refetchTransactionHistory or refetchGasTankTransactions) in the same block so
the new top-up appears in the history; modify the onBack callback where
setOnboardingScreen and refetchGasTankBalance are called to invoke that refetch
function as well.
| } | ||
|
|
||
|
|
||
| // ... existing code ... |
There was a problem hiding this comment.
Remove misleading comment.
The comment references "existing code" but this is a new file. Remove or update the comment to avoid confusion.
🔎 Suggested fix
-
-// ... existing code ...
-Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/apps/gas-tank/components/App/HomeScreen.tsx around line 98, The
top-of-file comment that reads "// ... existing code ..." in the HomeScreen
component is misleading because this is a new file; remove or replace that
comment with a clear, accurate description (or delete it entirely). Locate the
placeholder comment in src/apps/gas-tank/components/App/HomeScreen.tsx around
the HomeScreen component declaration and either delete the "// ... existing code
..." line or replace it with a concise comment describing the component's
purpose.
| // Reset displayed limit when transactions change | ||
| useMemo(() => { | ||
| setDisplayedLimit(10); | ||
| }, [transactions]); |
There was a problem hiding this comment.
Incorrect use of useMemo for side effects.
useMemo is used here to call setDisplayedLimit, which is a side effect. This should be a useEffect instead.
🔎 Proposed fix
- // Reset displayed limit when transactions change
- useMemo(() => {
- setDisplayedLimit(10);
- }, [transactions]);
+ // Reset displayed limit when transactions change
+ useEffect(() => {
+ setDisplayedLimit(10);
+ }, [transactions]);📝 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.
| // Reset displayed limit when transactions change | |
| useMemo(() => { | |
| setDisplayedLimit(10); | |
| }, [transactions]); | |
| // Reset displayed limit when transactions change | |
| useEffect(() => { | |
| setDisplayedLimit(10); | |
| }, [transactions]); |
🤖 Prompt for AI Agents
In @src/apps/gas-tank/components/History/GasTankHistoryCard.tsx around lines
40-43, The component incorrectly uses useMemo to perform a side effect—resetting
state—so replace the useMemo block with a useEffect that watches transactions
and calls setDisplayedLimit(10); specifically change the existing useMemo(() =>
{ setDisplayedLimit(10); }, [transactions]) to useEffect(() => {
setDisplayedLimit(10); }, [transactions]) so the state reset runs as an effect
when the transactions array changes.
| case 'amount': | ||
| // Sort algebraically: Top-up is positive, Spend is negative | ||
| const aAmount = parseFloat(a.usdcAmount); | ||
| const bAmount = parseFloat(b.usdcAmount); | ||
| aValue = a.type === 'Top-up' ? aAmount : -aAmount; | ||
| bValue = b.type === 'Top-up' ? bAmount : -bAmount; | ||
| break; | ||
| case 'token': | ||
| aValue = 'USDC'; // All transactions are in USDC | ||
| bValue = 'USDC'; | ||
| break; | ||
| default: | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Wrap switch case declarations in a block to prevent scope leakage.
As flagged by static analysis, const declarations inside switch cases without block scope can be accessed erroneously from other cases.
🔎 Proposed fix
case 'amount':
+ {
// Sort algebraically: Top-up is positive, Spend is negative
const aAmount = parseFloat(a.usdcAmount);
const bAmount = parseFloat(b.usdcAmount);
aValue = a.type === 'Top-up' ? aAmount : -aAmount;
bValue = b.type === 'Top-up' ? bAmount : -bAmount;
break;
+ }📝 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.
| case 'amount': | |
| // Sort algebraically: Top-up is positive, Spend is negative | |
| const aAmount = parseFloat(a.usdcAmount); | |
| const bAmount = parseFloat(b.usdcAmount); | |
| aValue = a.type === 'Top-up' ? aAmount : -aAmount; | |
| bValue = b.type === 'Top-up' ? bAmount : -bAmount; | |
| break; | |
| case 'token': | |
| aValue = 'USDC'; // All transactions are in USDC | |
| bValue = 'USDC'; | |
| break; | |
| default: | |
| return 0; | |
| } | |
| case 'amount': | |
| { | |
| // Sort algebraically: Top-up is positive, Spend is negative | |
| const aAmount = parseFloat(a.usdcAmount); | |
| const bAmount = parseFloat(b.usdcAmount); | |
| aValue = a.type === 'Top-up' ? aAmount : -aAmount; | |
| bValue = b.type === 'Top-up' ? bAmount : -bAmount; | |
| break; | |
| } | |
| case 'token': | |
| aValue = 'USDC'; // All transactions are in USDC | |
| bValue = 'USDC'; | |
| break; | |
| default: | |
| return 0; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 64-64: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 65-65: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In @src/apps/gas-tank/components/History/GasTankHistoryCard.tsx around lines
62-75, The switch in GasTankHistoryCard (the sort comparator handling cases
'amount' and 'token') declares consts inside cases which can leak scope; fix by
wrapping each affected case body in its own block (e.g., case 'amount': { /*
compute aAmount, bAmount, aValue, bValue */ break; }) or alternatively declare
aAmount/bAmount/aValue/bValue before the switch and assign inside cases so no
block-scoped declarations cross case boundaries; update the cases 'amount' and
'token' accordingly.
| /** | ||
| * Skeleton loading components for gas tank | ||
| */ | ||
|
|
||
| /** | ||
| * Skeleton row for transaction history loading state | ||
| */ | ||
| export const SkeletonTransactionRow: React.FC = () => ( |
There was a problem hiding this comment.
Missing React import for React.FC type annotation.
The file uses React.FC but doesn't import React. This will cause a TypeScript/runtime error.
🔎 Proposed fix
/**
* Skeleton loading components for gas tank
*/
+import React from 'react';
/**
* Skeleton row for transaction history loading state
*/
export const SkeletonTransactionRow: React.FC = () => (📝 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.
| /** | |
| * Skeleton loading components for gas tank | |
| */ | |
| /** | |
| * Skeleton row for transaction history loading state | |
| */ | |
| export const SkeletonTransactionRow: React.FC = () => ( | |
| /** | |
| * Skeleton loading components for gas tank | |
| */ | |
| import React from 'react'; | |
| /** | |
| * Skeleton row for transaction history loading state | |
| */ | |
| export const SkeletonTransactionRow: React.FC = () => ( |
🤖 Prompt for AI Agents
In @src/apps/gas-tank/components/History/GasTankSkeleton.tsx around lines 1-8,
The file uses the React.FC type for the SkeletonTransactionRow component but
never imports React; add an import for React (e.g., import React from "react")
at the top of the file so the React.FC type is defined and the component
compiles and runs correctly.
| // Inject Gas Tank Tile | ||
| const gasTankTile: Projection = { | ||
| id: 'gas-tank-tile', | ||
| layout: ApiLayout.GAS_TANK, | ||
| meta: { | ||
| display: { title: 'Gas Tank' }, | ||
| }, | ||
| data: {} as any, | ||
| }; |
There was a problem hiding this comment.
Fix type safety: provide proper data or use undefined instead of as any.
The data field is cast to any to bypass type checking. According to the Projection type definition, data is optional and can be TokenData[] | Advertisement | MediaGridData | Points | TokensMarketData | AlgoInsightsData. Either provide the appropriate data structure for a GAS_TANK tile or omit the field entirely (since it's optional).
🔎 Proposed fix
// Inject Gas Tank Tile
const gasTankTile: Projection = {
id: 'gas-tank-tile',
layout: ApiLayout.GAS_TANK,
meta: {
display: { title: 'Gas Tank' },
},
- data: {} as any,
};📝 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.
| // Inject Gas Tank Tile | |
| const gasTankTile: Projection = { | |
| id: 'gas-tank-tile', | |
| layout: ApiLayout.GAS_TANK, | |
| meta: { | |
| display: { title: 'Gas Tank' }, | |
| }, | |
| data: {} as any, | |
| }; | |
| // Inject Gas Tank Tile | |
| const gasTankTile: Projection = { | |
| id: 'gas-tank-tile', | |
| layout: ApiLayout.GAS_TANK, | |
| meta: { | |
| display: { title: 'Gas Tank' }, | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In @src/apps/pillarx-app/index.tsx around lines 130-138, The gasTankTile
Projection currently forces type-safety off by casting data to any; instead
remove the data property or set it to undefined, or supply a correctly-typed
value matching Projection's allowed types (e.g., TokenData[] | Advertisement |
MediaGridData | Points | TokensMarketData | AlgoInsightsData) for the GAS_TANK
tile; update the gasTankTile object (id: 'gas-tank-tile', layout:
ApiLayout.GAS_TANK) to omit data or assign a properly typed structure rather
than using "as any".
| setOnboardingScreen: Dispatch<SetStateAction<'welcome' | 'topup' | null>>; | ||
| markOnboardingComplete: () => void; | ||
| isPortfolioLoading?: boolean; | ||
| hasPortfolioData?: boolean; // True if portfolio has been loaded at least once |
There was a problem hiding this comment.
Remove unused prop hasPortfolioData.
The hasPortfolioData prop is declared and destructured but never used in the component. This adds unnecessary complexity to the component's interface.
🔎 Proposed fix
interface TopUpScreenProps {
onBack: () => void;
initialBalance: number;
setSearching: () => void;
selectedToken: SelectedToken | null;
portfolioTokens: PortfolioToken[];
setOnboardingScreen: Dispatch<SetStateAction<'welcome' | 'topup' | null>>;
markOnboardingComplete: () => void;
isPortfolioLoading?: boolean;
- hasPortfolioData?: boolean; // True if portfolio has been loaded at least once
showCloseButton?: boolean; // Show ESC button instead of back arrow
} export default function TopUpScreen(props: TopUpScreenProps) {
const {
onBack,
initialBalance,
setSearching,
selectedToken,
portfolioTokens,
setOnboardingScreen,
markOnboardingComplete,
isPortfolioLoading = false,
- hasPortfolioData = false,
showCloseButton = false,
} = props;Also applies to: 78-78
🤖 Prompt for AI Agents
In @src/apps/pulse/components/Onboarding/TopUpScreen.tsx around line 64, The
prop hasPortfolioData is declared in the component props and destructured in the
TopUpScreen component but never used; remove it from the Props/interface/type
definition and from the function parameter destructuring in TopUpScreen (and
from any defaultProps or prop spread in that component) so the component
signature no longer exposes hasPortfolioData; also update any local references
or tests that expect this prop.
| if (appId === 'gas-tank') { | ||
| setIconSrc(GasTankIcon); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove hardcoded special case in favor of standard icon loading pattern.
The hardcoded check for 'gas-tank' bypasses the standard icon loading mechanism used for other internal apps. Since gas-tank is defined as an internal app (type: 'app') in AllowedAppsProvider, it should follow the standard pattern of dynamically importing ../apps/gas-tank/icon.png (line 34) rather than requiring special-case handling.
🔎 Recommended refactor to use standard pattern
Remove the special case and ensure gas-tank has an icon.png file like other internal apps:
-// assets
-import GasTankIcon from '../apps/pulse/assets/gas-tank-icon.svg';
-
const AppIcon = ({
app,
appId,
@@ -23,11 +20,6 @@
useEffect(() => {
const loadIconSrc = async () => {
- if (appId === 'gas-tank') {
- setIconSrc(GasTankIcon);
- return;
- }
-
if ((app as ApiAllowedApp).type === 'app-external') {
setIconSrc((app as ApiAllowedApp).logo);
} else {Then ensure src/apps/gas-tank/icon.png exists in the repository.
📝 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.
| if (appId === 'gas-tank') { | |
| setIconSrc(GasTankIcon); | |
| return; | |
| } | |
| const AppIcon = ({ | |
| app, | |
| appId, | |
| // ... rest of props | |
| }: AppIconProps) => { | |
| const [iconSrc, setIconSrc] = useState<string>(''); | |
| useEffect(() => { | |
| const loadIconSrc = async () => { | |
| if ((app as ApiAllowedApp).type === 'app-external') { | |
| setIconSrc((app as ApiAllowedApp).logo); | |
| } else { | |
| // Rest of the standard icon loading pattern continues... | |
| } | |
| }; | |
| loadIconSrc(); | |
| }, [app, appId]); | |
| return <img src={iconSrc} alt={appId} />; | |
| }; |
🤖 Prompt for AI Agents
In @src/components/AppIcon.tsx around lines 26-29, Remove the hardcoded
special-case branch that checks appId === 'gas-tank' inside the AppIcon
component (the block that calls setIconSrc(GasTankIcon) and returns); instead
let the existing standard icon-loading flow (the dynamic import logic that loads
../apps/${appId}/icon.png and sets icon via setIconSrc) handle gas-tank the same
as other internal apps, and ensure an icon.png is added at
src/apps/gas-tank/icon.png so the dynamic import resolves successfully.
| // Default apps that are always available | ||
| const defaultApps: ApiAllowedApp[] = [ | ||
| { | ||
| id: 'gas-tank', | ||
| type: 'app', | ||
| appId: 'gas-tank', | ||
| title: 'Gas Tank', | ||
| name: 'Gas Tank', | ||
| shortDescription: 'Universal Gas Tank', | ||
| longDescription: | ||
| 'Manage your gas balance across all networks with the PillarX Gas Tank', | ||
| logo: '../apps/gas-tank/assets/gas-tank-icon.svg', | ||
| }, | ||
| ]; | ||
|
|
||
| // Combine default apps with API-fetched apps | ||
| const allApps = data?.length ? [...defaultApps, ...data] : defaultApps; | ||
| setAllowed(allApps.map((app: ApiAllowedApp) => app)); | ||
| } catch (e) { | ||
| console.warn('Error calling PillarX apps API', e); | ||
| // Still provide default apps even if API fails | ||
| if (!expired) { | ||
| const defaultApps: ApiAllowedApp[] = [ | ||
| { | ||
| id: 'gas-tank', | ||
| type: 'app', | ||
| appId: 'gas-tank', | ||
| title: 'Gas Tank', | ||
| name: 'Gas Tank', | ||
| shortDescription: 'Universal Gas Tank', | ||
| longDescription: | ||
| 'Manage your gas balance across all networks with the PillarX Gas Tank', | ||
| logo: '../apps/gas-tank/assets/gas-tank-icon.svg', | ||
| }, | ||
| ]; | ||
| setAllowed(defaultApps); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated defaultApps definition to eliminate code duplication.
The defaultApps array is defined identically in both the success path (lines 90-102) and the error handler (lines 111-123). This duplication violates DRY principles and increases maintenance burden.
🔎 Proposed refactor to extract constant
+const DEFAULT_APPS: ApiAllowedApp[] = [
+ {
+ id: 'gas-tank',
+ type: 'app',
+ appId: 'gas-tank',
+ title: 'Gas Tank',
+ name: 'Gas Tank',
+ shortDescription: 'Universal Gas Tank',
+ longDescription:
+ 'Manage your gas balance across all networks with the PillarX Gas Tank',
+ logo: '../apps/gas-tank/assets/gas-tank-icon.svg',
+ },
+];
+
const AllowedAppsProvider = ({ children }: { children: React.ReactNode }) => {
const [isLoading, setIsLoading] = React.useState<boolean>(true);
const [isAnimated, setIsAnimated] = React.useState<boolean>(false);
@@ -87,20 +99,7 @@
}
- // Default apps that are always available
- const defaultApps: ApiAllowedApp[] = [
- {
- id: 'gas-tank',
- type: 'app',
- appId: 'gas-tank',
- title: 'Gas Tank',
- name: 'Gas Tank',
- shortDescription: 'Universal Gas Tank',
- longDescription:
- 'Manage your gas balance across all networks with the PillarX Gas Tank',
- logo: '../apps/gas-tank/assets/gas-tank-icon.svg',
- },
- ];
// Combine default apps with API-fetched apps
- const allApps = data?.length ? [...defaultApps, ...data] : defaultApps;
+ const allApps = data?.length ? [...DEFAULT_APPS, ...data] : DEFAULT_APPS;
setAllowed(allApps.map((app: ApiAllowedApp) => app));
} catch (e) {
console.warn('Error calling PillarX apps API', e);
// Still provide default apps even if API fails
if (!expired) {
- const defaultApps: ApiAllowedApp[] = [
- {
- id: 'gas-tank',
- type: 'app',
- appId: 'gas-tank',
- title: 'Gas Tank',
- name: 'Gas Tank',
- shortDescription: 'Universal Gas Tank',
- longDescription:
- 'Manage your gas balance across all networks with the PillarX Gas Tank',
- logo: '../apps/gas-tank/assets/gas-tank-icon.svg',
- },
- ];
- setAllowed(defaultApps);
+ setAllowed(DEFAULT_APPS);
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/providers/AllowedAppsProvider.tsx around lines 89-125, Extract the
duplicated defaultApps array into a single constant (e.g., DEFAULT_ALLOWED_APPS)
outside the try/catch in AllowedAppsProvider.tsx and reuse it in both the
success and error paths; update the allApps computation to use
DEFAULT_ALLOWED_APPS (const allApps = data?.length ? [...DEFAULT_ALLOWED_APPS,
...data] : DEFAULT_ALLOWED_APPS) and call setAllowed(DEFAULT_ALLOWED_APPS) in
the catch branch (when !expired), ensuring the type ApiAllowedApp is preserved.
| const defaultApps: ApiAllowedApp[] = [ | ||
| { | ||
| id: 'gas-tank', | ||
| type: 'app', | ||
| appId: 'gas-tank', | ||
| title: 'Gas Tank', | ||
| name: 'Gas Tank', | ||
| shortDescription: 'Universal Gas Tank', | ||
| longDescription: | ||
| 'Manage your gas balance across all networks with the PillarX Gas Tank', | ||
| logo: '../apps/gas-tank/assets/gas-tank-icon.svg', | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the gas-tank icon file
echo "=== Searching for gas-tank icon file ==="
fd -t f "gas-tank-icon.svg" 2>/dev/null || echo "File not found with fd"
# Also try a broader search
echo "=== Broader search for gas-tank icon ==="
find . -name "*gas-tank*" -type f 2>/dev/null | head -20
# Check relative path structure from the provider file
echo "=== Checking from src/providers directory ==="
ls -la ../apps/gas-tank/assets/ 2>/dev/null || echo "Path not found from providers dir"Repository: pillarwallet/x
Length of output: 493
🏁 Script executed:
#!/bin/bash
# Check the AppIcon component to see lines 26-29
echo "=== Examining AppIcon.tsx (lines 20-40) ==="
fd -t f -e tsx "AppIcon" | head -5 | xargs -I {} bash -c 'echo "File: {}"; head -50 "{}"'Repository: pillarwallet/x
Length of output: 1481
🏁 Script executed:
#!/bin/bash
# Check how many apps use logo property
echo "=== Checking logo usage in AllowedAppsProvider ==="
fd -t f "AllowedAppsProvider.tsx" | xargs rg -n "logo:" -A 1
# Check if there are other apps defined
echo -e "\n=== Checking all apps defined in provider ==="
fd -t f "AllowedAppsProvider.tsx" | xargs rg -n "id:" -B 2Repository: pillarwallet/x
Length of output: 594
🏁 Script executed:
#!/bin/bash
# Compare the two icon files
echo "=== Comparing gas-tank icon files ==="
echo "File 1: src/apps/gas-tank/assets/gas-tank-icon.svg"
wc -c src/apps/gas-tank/assets/gas-tank-icon.svg
echo "File 2: src/apps/pulse/assets/gas-tank-icon.svg"
wc -c src/apps/pulse/assets/gas-tank-icon.svg
# Check if they're identical
if cmp -s src/apps/gas-tank/assets/gas-tank-icon.svg src/apps/pulse/assets/gas-tank-icon.svg; then
echo "Files are IDENTICAL"
else
echo "Files are DIFFERENT"
fi
# Show first few lines of each
echo -e "\n=== First 20 chars of gas-tank icon ==="
head -c 100 src/apps/gas-tank/assets/gas-tank-icon.svg
echo -e "\n=== First 20 chars of pulse icon ==="
head -c 100 src/apps/pulse/assets/gas-tank-icon.svgRepository: pillarwallet/x
Length of output: 647
🏁 Script executed:
#!/bin/bash
# Look for context around the special case handling
echo "=== Examining AppIcon.tsx context around gas-tank special case ==="
rg -n "gas-tank" src/components/AppIcon.tsx -B 5 -A 5
# Search for any comments or context about why this special case exists
echo -e "\n=== Searching for gas-tank references in entire codebase ==="
rg -l "gas-tank" src --type tsx --type ts | head -10Repository: pillarwallet/x
Length of output: 896
🏁 Script executed:
#!/bin/bash
# Check if the logo property is actually used anywhere for gas-tank
echo "=== Checking if logo property is referenced in AppIcon ==="
rg -n "\.logo" src/components/AppIcon.tsx
# Check how other apps handle icons
echo -e "\n=== Checking AllowedAppsProvider for all app definitions ==="
sed -n '90,130p' src/providers/AllowedAppsProvider.tsxRepository: pillarwallet/x
Length of output: 1640
Remove the unused logo property; it's overridden by AppIcon's hardcoded special case.
The logo path in the provider ('../apps/gas-tank/assets/gas-tank-icon.svg') is never used. AppIcon's special case for appId === 'gas-tank' (lines 26-28) ignores the logo property and instead imports the icon from ../apps/pulse/assets/gas-tank-icon.svg. This creates dead code and a confusing mismatch between the declared path and actual import. The icon files at both locations exist but contain different content.
Either:
- Remove the
logoproperty from the gas-tank app definition and rely solely on AppIcon's special case, or - Align the special case to use the declared path, or
- Migrate gas-tank to the standard pattern used by other internal apps (dynamic
icon.pngimport)
🤖 Prompt for AI Agents
In @src/providers/AllowedAppsProvider.tsx around lines 90-102, The gas-tank
entry in AllowedAppsProvider's defaultApps includes an unused logo property
which is ignored by AppIcon's special-case for appId === 'gas-tank'; remove the
dead `logo` field from the gas-tank object in defaultApps (or alternatively
update AppIcon's special-case to import the same path declared there or refactor
gas-tank to the standard dynamic icon pattern) so the provider and AppIcon are
consistent; locate the gas-tank object in defaultApps inside AllowedAppsProvider
and remove the `logo: '../apps/gas-tank/assets/gas-tank-icon.svg'` key (or
change AppIcon’s special-case to use that path or migrate gas-tank to dynamic
icon loading).
Deploying pillarx-debug with
|
| Latest commit: |
0889185
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8a2711f0.pillarx-debug.pages.dev |
| Branch Preview URL: | https://pro-3924-gastankpage.pillarx-debug.pages.dev |
Deploying x with
|
| Latest commit: |
0889185
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://adb32bcd.x-e62.pages.dev |
| Branch Preview URL: | https://pro-3924-gastankpage.x-e62.pages.dev |
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.