Add fiat transactions history on buy/sell screen#1810
Conversation
- Add FiatTransactionsScene (activity list) and FiatTransactionDetailScene with asset preview headers - Add FiatTransactionService for fetching and storing fiat transaction history - Add FiatTransactionStore with GRDB persistence and observable queries - Add FiatTransactionViewModel producing ListItemModel with status tags (pending/failed) - Add FiatTransactionStatusViewModel for consistent status colors - Add FiatProviderName display names and images for providers - Add FiatAmountDisplay for fiat-only transaction headers - Pass fiatTransactionService through model chain from app target (no @Entry/@Environment hack) - Update AmountSceneViewModel and ConfirmTransferSceneViewModel to carry fiatTransactionService - Add navigation from FiatConnectNavigationView to activity list and detail screens - Gray subtitle color for failed transactions, no +/- signs on fiat amounts
Summary of ChangesHello, 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 introduces a significant new feature, allowing users to view their fiat transaction history directly within the application's buy/sell interface. It provides a dedicated screen to list all fiat transactions and a detailed view for each, enhancing transparency and user experience for fiat operations. The changes involve new UI components, a dedicated service layer for transaction management, and robust data persistence to ensure transaction history is readily available. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: a history screen for fiat transactions. The implementation is well-structured, adding new scenes, view models, services, and database components across the relevant packages. The code follows existing architectural patterns, such as using reactive queries to populate views from a local store. My feedback focuses on improving user experience through better error handling, enhancing accessibility by adjusting color usage, and general code hygiene.
| } catch { | ||
| debugLog("FiatTransactionsViewModel fetch error: \(error)") | ||
| } |
There was a problem hiding this comment.
Silently catching and logging the error here means the user receives no feedback if fetching transactions fails. They will see an empty or stale list, which can be confusing. It's better to expose this error state to the view so it can display an appropriate error message. Consider adding a state property to the view model to track fetching errors.
|
|
||
| import SwiftUI | ||
| import Store | ||
| import FiatConnect |
There was a problem hiding this comment.
This import FiatConnect statement appears to be unused in the file. It should be removed to maintain code cleanliness.
| address: address | ||
| ) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The file should end with a newline character. This is a common code style convention that many tools expect.
| private var subtitleColor: Color { | ||
| switch transaction.status { | ||
| case .failed: Colors.gray | ||
| case .pending, .complete, .unknown: Colors.black | ||
| } |
There was a problem hiding this comment.
Using Colors.gray for the subtitle when a transaction has failed reduces text contrast, which can be an accessibility issue. Since the failed status is already clearly indicated by the title tag (which is styled with a red color), it's better to ensure the amount remains clearly readable. I suggest making the amount always black for better readability and accessibility, regardless of the transaction status.
private var subtitleColor: Color {
Colors.black
}1 participant
Close: #1807