Add asset image header on Stake and Earn views#1836
Conversation
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 refactors the display of asset information in the Stake and Earn features, as well as in WalletConnector scenes, by introducing a new, reusable ListAssetHeaderView component. This change streamlines the UI code, promotes consistency in how asset headers are presented, and removes deprecated properties, leading to a cleaner and more maintainable codebase. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new ListAssetHeaderView component to standardize the display of asset headers within List views, replacing duplicated code in EarnScene, StakeScene, ConnectionProposalScene, and SignMessageScene. View models were refactored to expose AssetViewModel directly. The review suggests improving the encapsulation of ListAssetHeaderView by defining a local SubtitleLayout enum instead of exposing AssetPreviewView.SubtitleLayout in its public API.
| private let subtitleLayout: AssetPreviewView<Model>.SubtitleLayout | ||
|
|
||
| public init(model: Model, subtitleLayout: AssetPreviewView<Model>.SubtitleLayout = .horizontal) { | ||
| self.model = model | ||
| self.subtitleLayout = subtitleLayout | ||
| } |
There was a problem hiding this comment.
To improve encapsulation, consider defining a local SubtitleLayout enum for this view rather than exposing AssetPreviewView.SubtitleLayout in the public API. This decouples ListAssetHeaderView from the implementation details of AssetPreviewView.
You can then add the new enum and a private computed property to map to the AssetPreviewView.SubtitleLayout:
public enum SubtitleLayout {
case horizontal
case vertical
}
private var assetPreviewSubtitleLayout: AssetPreviewView<Model>.SubtitleLayout {
switch subtitleLayout {
case .horizontal: .horizontal
case .vertical: .vertical
}
}Then, use assetPreviewSubtitleLayout in the body.
| private let subtitleLayout: AssetPreviewView<Model>.SubtitleLayout | |
| public init(model: Model, subtitleLayout: AssetPreviewView<Model>.SubtitleLayout = .horizontal) { | |
| self.model = model | |
| self.subtitleLayout = subtitleLayout | |
| } | |
| private let subtitleLayout: SubtitleLayout | |
| public init(model: Model, subtitleLayout: SubtitleLayout = .horizontal) { | |
| self.model = model | |
| self.subtitleLayout = subtitleLayout | |
| } |
07a3d2f to
29ee438
Compare
Replace text section titles with asset image header (logo + name) on Stake and Earn views, matching the pattern used in RecipientScene.
ListAssetHeaderViewin PrimitivesComponentsSection { } header:blocks in WalletConnector scenesassetTitlepropertiesClose: #1831