-
-
Notifications
You must be signed in to change notification settings - Fork 279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(suite-native): persist favourite trade assets #17169
base: develop
Are you sure you want to change the base?
Conversation
…rite lists exclusive
🚀 Expo preview is ready!
|
WalkthroughThis pull request introduces modifications across multiple modules. In the component responsible for displaying tradeable assets, the logic for determining favorite assets has been refactored. The import of helper functions and the selector used for favorites has been updated, and the asset list is now dynamically categorized into favorites and non-favorites using a reduction over the assets array. In the trading slice, a helper function for generating favorite asset keys has been changed from private to exported, allowing broader reuse. Additionally, the state management has been enhanced with the introduction of a persisted reducer for trading, ensuring that the favorites state is maintained across sessions by persisting it under a defined key and version. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
suite-native/module-trading/src/components/general/TradeableAssetsSheet/TradeableAssetsSheet.tsx (1)
81-93
: Consider extracting the reducer function for better reusability.While the current implementation is efficient, the reducer function could be extracted to improve reusability and testability.
+const categorizeAssets = (favourites: Record<string, TradeableAsset>) => ( + acc: { favouriteData: TradeableAsset[]; allData: TradeableAsset[] }, + asset: TradeableAsset, +) => { + const key = getTradeableAssetFavouriteKey(asset); + if (favourites[key]) { + acc.favouriteData.push(asset); + } else { + acc.allData.push(asset); + } + return acc; +}; const listData = useMemo(() => { - const { favouriteData, allData } = mockAssets.reduce( - (acc, a) => { - const key = getTradeableAssetFavouriteKey(a); - if (favourites[key]) { - acc.favouriteData.push(a); - } else { - acc.allData.push(a); - } - - return acc; - }, - { favouriteData: [] as TradeableAsset[], allData: [] as TradeableAsset[] }, - ); + const { favouriteData, allData } = mockAssets.reduce( + categorizeAssets(favourites), + { favouriteData: [] as TradeableAsset[], allData: [] as TradeableAsset[] }, + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
suite-native/module-trading/src/components/general/TradeableAssetsSheet/TradeableAssetsSheet.tsx
(2 hunks)suite-native/module-trading/src/tradingSlice.ts
(1 hunks)suite-native/state/src/reducers.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
suite-native/module-trading/src/components/general/TradeableAssetsSheet/TradeableAssetsSheet.tsx (2)
Learnt from: jbazant
PR: trezor/trezor-suite#16668
File: suite-native/module-trading/src/components/TradeableAssetsSheet/TradeableAssetsSheet.tsx:0-0
Timestamp: 2025-02-04T13:18:21.530Z
Learning: The mock data arrays in `suite-native/module-trading/src/components/TradeableAssetsSheet/TradeableAssetsSheet.tsx` are intentionally used as visual stubs and will be replaced with real data fetching in future commits.
Learnt from: jbazant
PR: trezor/trezor-suite#16668
File: suite-native/module-trading/src/components/general/TradeableAssetsSheet/TradeableAssetsFilterTabs.tsx:17-28
Timestamp: 2025-02-04T13:18:46.084Z
Learning: The mock data in TradeableAssetsFilterTabs.tsx is part of a visual stub and will be replaced with dynamic data in future commits.
🔇 Additional comments (3)
suite-native/state/src/reducers.ts (1)
78-83
: LGTM! Well-structured persistence implementation.The implementation follows the established pattern of other persisted reducers in the file, correctly specifying the persisted keys and version, and properly integrating with the existing wallet reducers.
Also applies to: 94-94
suite-native/module-trading/src/tradingSlice.ts (1)
34-35
: LGTM! Clean and efficient key generation function.The exported function follows a consistent pattern for generating unique keys, combining the asset's symbol and contract address when available. The implementation is simple, efficient, and reusable across components.
suite-native/module-trading/src/components/general/TradeableAssetsSheet/TradeableAssetsSheet.tsx (1)
7-7
: LGTM! Efficient data processing implementation.The refactored implementation efficiently categorizes assets using reduce, maintaining a clean separation between favorites and all assets. The useMemo dependency is correctly updated to reflect the changes.
Also applies to: 78-112
Related Issue
Followup to #16991
Screenshots:
Simulator.Screen.Recording.-.iPhone.16.-.2025-02-22.at.16.27.34.mp4