feat(simulation): tap token name to view contract address#84
Merged
Conversation
📝 WalkthroughWalkthroughSafeTxSimulationScreen adds tappable token and NFT labels/logos that open an AddressDetailSheet via a modal bottom sheet; new helpers (_tokenLogo, _inlineTokenSpan, _inlineTappableLabelSpan, _showTokenDetail) centralize logo rendering and tappable, dotted-underlined inline labels. ChangesInteractive Token Details in Transaction Simulation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart`:
- Around line 303-305: Replace the non-keyboard-accessible GestureDetector
usages that wrap token/NFT rows with a Semantics + InkWell pattern so keyboard
and screen-reader users can activate _showTokenDetail; specifically, locate the
widgets that call _showTokenDetail(context, tokenAddress) and the other two
similar GestureDetector instances and wrap the row in Semantics(button: true,
onTapHint: 'Open token details') and an InkWell (or InkResponse) with onTap
calling _showTokenDetail instead of GestureDetector.onTap; ensure the InkWell
contains the same child Row and preserve any visual feedback and hit testing
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e3705a4-c108-40ea-8b2d-59c68de37148
📒 Files selected for processing (1)
lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart (1)
466-466: 💤 Low valueVerify intentional font size difference between token and NFT allowances.
Token allowance messages (lines 466, 523) use
fontSize: 14, while NFT allowance messages (lines 794, 878) remain atfontSize: 12. If this difference is intentional (e.g., NFT rows already have larger images), that's fine. Otherwise, consider unifying the font sizes for visual consistency.Also applies to: 523-523, 794-794, 878-878
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart` at line 466, The token allowance message uses TextStyle(fontSize: 14, color: Colors.grey[600], fontWeight: FontWeight.bold) while NFT allowance messages use fontSize: 12; decide which size should be the standard (14 or 12) and make them consistent: locate the TextStyle(...) occurrences used for token allowance text and the NFT allowance rows in safe_tx_simulation_screen.dart and update the fontSize value in the NFT TextStyle instances (or the token ones) so both token and NFT allowance messages use the same fontSize; ensure you update all occurrences (the shown TextStyle(...) instance and the corresponding NFT TextStyle usages) so visual consistency is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart`:
- Around line 277-292: _tokenLogo currently uses a hardcoded fontSize (17) and
lacks image error handling; update the widget so the placeholder "?" text scales
with the provided size parameter (e.g., compute fontSize as a fraction of size)
and add an errorBuilder to Image.network to show the same fallback (centered "?"
or the same icon) when the image fails to load; this change will fix overflow
when _tokenLogo is called from _inlineTokenSpan and make behavior consistent
with _buildNFTImageContent.
---
Nitpick comments:
In
`@lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart`:
- Line 466: The token allowance message uses TextStyle(fontSize: 14, color:
Colors.grey[600], fontWeight: FontWeight.bold) while NFT allowance messages use
fontSize: 12; decide which size should be the standard (14 or 12) and make them
consistent: locate the TextStyle(...) occurrences used for token allowance text
and the NFT allowance rows in safe_tx_simulation_screen.dart and update the
fontSize value in the NFT TextStyle instances (or the token ones) so both token
and NFT allowance messages use the same fontSize; ensure you update all
occurrences (the shown TextStyle(...) instance and the corresponding NFT
TextStyle usages) so visual consistency is preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eae31c68-9511-4539-9cf4-9c20d978e9a6
📒 Files selected for processing (1)
lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart
Comment on lines
+277
to
+292
| Widget _tokenLogo(String logoUri, double size) { | ||
| return SizedBox( | ||
| width: size, | ||
| height: size, | ||
| child: ClipRRect( | ||
| borderRadius: BorderRadius.circular(70), | ||
| child: logoUri == "unknown" | ||
| ? Container( | ||
| alignment: Alignment.center, | ||
| color: Colors.grey, | ||
| child: const Text("?", style: TextStyle(fontSize: 17, fontWeight: FontWeight.bold)), | ||
| ) | ||
| : Image.network(logoUri), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Fix hardcoded font size and add error handling to _tokenLogo.
Two issues:
-
Font size overflow: Line 287 hardcodes
fontSize: 17regardless of thesizeparameter. Whensize=14(called from_inlineTokenSpanline 308), the 17px text overflows the 14×14 container. The font size should scale proportionally with the container. -
Missing error handling:
Image.networkhas noerrorBuilder. If the image fails to load, the widget shows blank space. This is inconsistent with_buildNFTImageContent(lines 1366–1411), which provides a fallback icon.
🔧 Proposed fix
Widget _tokenLogo(String logoUri, double size) {
return SizedBox(
width: size,
height: size,
child: ClipRRect(
- borderRadius: BorderRadius.circular(70),
+ borderRadius: BorderRadius.circular(size / 2),
child: logoUri == "unknown"
? Container(
alignment: Alignment.center,
color: Colors.grey,
- child: const Text("?", style: TextStyle(fontSize: 17, fontWeight: FontWeight.bold)),
+ child: Text(
+ "?",
+ style: TextStyle(
+ fontSize: size * 0.68,
+ fontWeight: FontWeight.bold,
+ ),
+ ),
)
- : Image.network(logoUri),
+ : Image.network(
+ logoUri,
+ errorBuilder: (context, error, stackTrace) {
+ return Container(
+ alignment: Alignment.center,
+ color: Colors.grey,
+ child: Icon(
+ Icons.broken_image,
+ size: size * 0.6,
+ color: Colors.grey[600],
+ ),
+ );
+ },
+ ),
),
);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@lib/features/verify_safe_transaction/simulation/safe_tx_simulation_screen.dart`
around lines 277 - 292, _tokenLogo currently uses a hardcoded fontSize (17) and
lacks image error handling; update the widget so the placeholder "?" text scales
with the provided size parameter (e.g., compute fontSize as a fraction of size)
and add an errorBuilder to Image.network to show the same fallback (centered "?"
or the same icon) when the image fails to load; this change will fix overflow
when _tokenLogo is called from _inlineTokenSpan and make behavior consistent
with _buildNFTImageContent.
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit