Conversation
- Replace hardcoded Colors.grayLight with adaptive systemGray5 background - Handle nil image URL to avoid infinite loading spinner for NFTs without images (e.g. ENS domains) - Add NftImagePlaceholderView with proportional circle + photo icon that clearly communicates "no image" - Show NFT name on placeholder in detail view only - Extract placeholder into separate reusable component
- Extract reusable VerifiedBadgeView component - Use consistent white checkmark styling across grid and detail views
Added isImageLoaded to track if image failed to download
…ifiers - Remove unused imports: Photos, ImageGalleryService, Foundation, Style, Localization - Remove dead `description` property that shadows CustomStringConvertible - Move private members to private extension: contractValue, contractExplorerUrl, enabledChainTypes - Make enabledChainTypes static since it's constant across instances
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 several enhancements and refactorings across the NFT collection and collectible views. It focuses on improving the user interface, standardizing UI components, and streamlining the underlying view models by moving logic into computed properties and private extensions. 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
The pull request refactors NFT-related UI components and view models, introducing reusable VerifiedBadgeView and NftImagePlaceholderView components for standardized UI elements and improved image loading states. The NftImageView now uses NftImagePlaceholderView for empty/failed states and binds an isImageLoaded flag to the CollectibleViewModel, which in turn controls the visibility of the image context menu. Additionally, the PR includes code cleanup by removing unused imports and reorganizing properties within CollectibleViewModel. A review comment suggests improving the NftImagePlaceholderView by extracting magic numbers used in layout calculations into named constants for better readability and maintainability.
| let showName = size > 250 | ||
| let circleSize = size * (showName ? 0.3 : 0.35) | ||
| let iconSize = circleSize * 0.45 |
There was a problem hiding this comment.
These layout calculations use several magic numbers. To improve readability and maintainability, it's best to extract these values into named constants. Please replace these lines with the suggestion and add the following constants struct inside NftImagePlaceholderView:
private struct Layout {
static let nameVisibilityThreshold: CGFloat = 250
static let circleSizeRatioWithText: CGFloat = 0.3
static let circleSizeRatioDefault: CGFloat = 0.35
static let iconSizeRatio: CGFloat = 0.45
}| let showName = size > 250 | |
| let circleSize = size * (showName ? 0.3 : 0.35) | |
| let iconSize = circleSize * 0.45 | |
| let showName = size > Layout.nameVisibilityThreshold | |
| let circleSize = size * (showName ? Layout.circleSizeRatioWithText : Layout.circleSizeRatioDefault) | |
| let iconSize = circleSize * Layout.iconSizeRatio |
2 participants
closes #1814
NftImagePlaceholderViewas reusable component with proportional sizingVerifiedBadgeViewwith consistent white checkmark on blue seal across grid and detail viewscontentMarginsfor top spacing, align grid horizontal padding with toolbar buttonimages: