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 introduces a standardized and reusable approach for handling context-sensitive actions related to blockchain explorers within the application. By abstracting the logic for copying values and opening explorer links into dedicated data structures and a SwiftUI view modifier, the changes streamline development, enhance consistency across different screens (including market insights, NFT details, validator selection, and transfer confirmations), and improve the overall user experience when interacting with blockchain-related data. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a standardized way to handle explorer links and copyable values across the application by introducing ExplorerContextData and CopyValue types. This refactoring impacts MarketValueViewModel, CollectibleViewModel, ValidatorSelectSceneViewModel, and ConfirmTransferSceneViewModel, centralizing the logic for displaying and interacting with blockchain explorer links and copy actions. Specifically, ChartScene was updated to use a new Action enum for market items, and CollectibleScene now uses CollectibleInfoRow and explorerContext for NFT details. The ExplorerService was extended with an nftUrl function. Feedback includes a suggestion to use the marketItemView helper in ChartScene for consistency and to enhance the ExplorerContextModifier to support custom onCopy handlers for a more consistent user experience.
| switch item.action { | ||
| case .explorer(let explorerContext): | ||
| SafariNavigationLink(url: explorerContext.explorerLink.url) { | ||
| ListItemView(title: item.title, subtitle: item.subtitle) |
There was a problem hiding this comment.
For consistency and to ensure all relevant view model properties are displayed, it's better to use the marketItemView helper function here, similar to the .info and .none cases. This will ensure that properties like titleTag, titleExtra, etc., are rendered if they exist on the item.
| ListItemView(title: item.title, subtitle: item.subtitle) | |
| marketItemView(item) |
| let explorerLink = context.explorerLink | ||
| content | ||
| .contextMenu([ | ||
| .copy(value: copyValue.rawValue), |
There was a problem hiding this comment.
The current implementation for .copy uses the default behavior, which may show a generic toast message. Other parts of the app provide a custom onCopy handler to show a more specific toast (e.g., showing the value that was copied).
To ensure a consistent user experience, consider adding support for a custom onCopy handler to this modifier. This would involve passing an optional closure to the modifier and using it in the .copy context menu item.
You could update the modifier and extension like this:
public struct ExplorerContextModifier: ViewModifier {
@State private var isPresentingUrl: URL?
let context: ExplorerContextData
let onCopy: ((String) -> Void)?
public func body(content: Content) -> some View {
// ...
.contextMenu([
.copy(value: copyValue.rawValue, onCopy: onCopy),
// ...
])
}
}
public extension View {
func explorerContext(_ context: ExplorerContextData, onCopy: ((String) -> Void)? = nil) -> some View {
modifier(ExplorerContextModifier(context: context, onCopy: onCopy))
}
}…:gemwalletcom/gem-ios into 1809-copy-view-on-explorer-for-addresses # Conflicts: # core
…-view-on-explorer-for-addresses # Conflicts: # Features/NFT/Sources/ViewModels/CollectibleViewModel.swift # core
Add shared explorer context actions for copy and view in explorer across iOS screens, including NFT token support.
Close: #1809