Consistent UI #15
Consistent UI #15
Conversation
WalkthroughAdds Android deep-link intents for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Navbar as UniversalNavbar
participant Wallet as WalletProvider
participant Dialog as WalletConnectDialog
User->>Navbar: Tap wallet button
alt isConnecting
Navbar->>Wallet: forceReconnect()
Wallet-->>Navbar: URI or error
else not connected
Navbar->>Wallet: connectWallet()
Wallet-->>Navbar: URI or error
end
alt URI available
Navbar->>Dialog: Show dialog with URI
else No URI / error
Navbar-->>User: No dialog shown
end
sequenceDiagram
autonumber
participant UI as BaseScaffold
participant Wallet as WalletProvider
participant Chain as ChainCheck
UI->>Wallet: Read isConnecting / isConnected
alt Connecting
UI-->>UI: Show spinner + "Connecting"
else Not connected
UI-->>UI: Show WalletNotConnectedWidget
else Connected
UI->>Chain: isCorrectChain?
alt Wrong chain
UI-->>UI: Show WrongChainWidget
else Correct chain
UI-->>UI: Render body in SafeArea
end
end
sequenceDiagram
autonumber
participant Android as Android System
participant App as MainActivity
Android->>App: Intent VIEW (scheme=treeplantingprotocol)
App-->>Android: Handled by intent-filter
Android->>Android: `<queries>` allows `wc` scheme queries
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
lib/pages/mint_nft/submit_nft_page.dart (1)
187-223: Error card uses the same “error” color for background, icon, and text → unreadable in dark/light themesUse an error container + contrasting text/icon (onError/onErrorContainer). Example fix below uses theme tokens with safe fallbacks.
- decoration: BoxDecoration( - color: getThemeColors(context)['error']!, - borderRadius: BorderRadius.circular(8), - border: Border.all(color: getThemeColors(context)['error']!), - ), + final colors = getThemeColors(context); + decoration: BoxDecoration( + color: colors['errorContainer'] ?? Theme.of(context).colorScheme.errorContainer, + borderRadius: BorderRadius.circular(8), + border: Border.all(color: colors['error'] ?? Theme.of(context).colorScheme.error), + ), ... - Icon(Icons.error_outline, - color: getThemeColors(context)['error']!), + Icon( + Icons.error_outline, + color: colors['error'] ?? Theme.of(context).colorScheme.error, + ), ... - style: TextStyle( - color: getThemeColors(context)['error']!, - fontSize: 14, - ), + style: TextStyle( + color: colors['onErrorContainer'] ?? Theme.of(context).colorScheme.onErrorContainer, + fontSize: 14, + ),lib/providers/wallet_provider.dart (4)
1-15: Missing import for web3dart types used below (ContractAbi, Web3Client, etc.)This file references web3dart classes but doesn’t import them. Add the import or remove usages.
import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +import 'package:web3dart/web3dart.dart' + show Web3Client, DeployedContract, ContractAbi, EthereumAddress;
43-43: Stray getterglooks accidental
bool get g => _isConnecting;is unclear and pollutes the API surface. Remove it.- bool get g => _isConnecting;
421-425: switchChain sets UI state without actually switching chains; message uses wrong map
- Status message reads from
chainInfo['name'](entire map), not the target chain’s entry._currentChainIdis set optimistically without an RPC; consider invokingwallet_switchEthereumChainor deferring state until confirmed.- _updateStatus('Switching to ${chainInfo['name']}...'); - _currentChainId = newChainId; + final name = _chainInfo[newChainId]?['name'] ?? newChainId; + _updateStatus('Switching to $name...'); + // TODO: request wallet_switchEthereumChain then update _currentChainId on success
285-286:userAddressgetter returns nullExpose
_currentAddressor remove this getter to avoid confusion.- get userAddress => null; + String? get userAddress => _currentAddress;lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart (1)
45-45: Fix formatting issues before merge.The pipeline is failing due to formatting issues. Run
dart formatto fix these issues.#!/bin/bash # Verify which files need formatting dart format --set-exit-if-changed --output=none lib/
🧹 Nitpick comments (29)
android/app/src/main/AndroidManifest.xml (2)
25-28: wc intent query added — OK. Consider consolidating queries blocks and verify necessity.
- This intent query for scheme wc is correct for package visibility. If you don’t call PackageManager.queryIntentActivities (or similar) for wc links, the intent and explicit entries may be unnecessary; startActivity with ACTION_VIEW doesn’t require it. Please confirm usage and remove if not needed.
- You now have two top-level blocks (Lines 20–29 and 79–84). Prefer a single block for maintainability.
Apply this diff to remove duplication (move the intent under the existing bottom block):
- <queries> - <package android:name="io.metamask"/> - <package android:name="com.wallet.crypto.trustapp"/> - <package android:name="io.gnosis.safe"/> - <package android:name="me.rainbow"/> - <intent> - <action android:name="android.intent.action.VIEW" /> - <data android:scheme="wc" /> - </intent> - </queries>Then add this under the existing near Lines 79–84:
<!-- also allow querying WalletConnect deep link handlers --> <intent> <action android:name="android.intent.action.VIEW" /> <data android:scheme="wc" /> </intent>
61-66: Deep link filter for treeplantingprotocol — correct. Add Flutter deep linking metadata and validate URIs.
- Filter is well-formed (VIEW + DEFAULT + BROWSABLE). Keep android:exported="true" as you have.
- If you rely on Flutter’s built-in deep linking (Navigator 2.0), add flutter_deeplinking_enabled to ensure routing receives links cleanly.
- Runtime: Validate the incoming URI (scheme, expected path/params) before acting; avoid performing sensitive actions directly from the link.
Minimal addition inside the activity:
<meta-data android:name="io.flutter.embedding.android.NormalTheme" android:resource="@style/NormalTheme" /> + <!-- Enable Flutter deep linking route parsing if used --> + <meta-data + android:name="flutter_deeplinking_enabled" + android:value="true" />Optional hardening (if you want stricter matches):
<!-- e.g., restrict to a path prefix --> <data android:scheme="treeplantingprotocol" android:pathPrefix="/v1/" />lib/pages/organisations_pages/user_organisations_page.dart (1)
1-2: Empty placeholder file — either implement or removeShip either a minimal scaffold or delete this file to avoid dead code and accidental route wiring to an empty page.
I can generate a minimal theming-ready page if you want.
lib/pages/mint_nft/submit_nft_page.dart (2)
108-116: Guard substring on transaction hash to prevent RangeError
substring(0, N)will throw if the hash is shorter than N. Clamp the end index.- 'Transaction hash: ${result.transactionHash!.substring(0, 10)}...\n\n' + final hash = result.transactionHash!; + 'Transaction hash: ${hash.substring(0, hash.length >= 10 ? 10 : hash.length)}...\n\n' ... - "Hash: ${lastTransactionHash!.substring(0, 20)}...", + "Hash: ${lastTransactionHash!.substring(0, lastTransactionHash!.length >= 20 ? 20 : lastTransactionHash!.length)}...",Also applies to: 158-163
132-141: Primary color as a card background might be too aggressivePrefer a “container/surface” token for legibility (e.g., primaryContainer/surfaceVariant) while keeping icons/border on primary.
lib/widgets/nft_display_utils/user_nfts_widget.dart (1)
42-101: Deduplicate integer conversion with shared toInt utilityThis file’s private
_toIntduplicates lib/utils/services/conversion_functions.dart:toInt. Prefer the shared helper.+ import 'package:tree_planting_protocol/utils/services/conversion_functions.dart'; ... - static int _toInt(dynamic value) { - if (value is BigInt) return value.toInt(); - if (value is int) return value; - return int.tryParse(value.toString()) ?? 0; - } + // Use toInt(value) from conversion_functions.dartAnd replace
_toInt(...)calls withtoInt(...).lib/utils/services/conversion_functions.dart (1)
1-5: Handle hex strings and numeric-like values more robustlyAdd support for "0x…" hex strings and num inputs; keep current fallbacks.
-int toInt(dynamic value) { - if (value is BigInt) return value.toInt(); - if (value is int) return value; - return int.tryParse(value.toString()) ?? 0; -} +int toInt(dynamic value) { + if (value is int) return value; + if (value is BigInt) return value.toInt(); + if (value is num) return value.toInt(); + final s = value.toString().trim(); + if (s.startsWith('0x') || s.startsWith('0X')) { + return int.tryParse(s.substring(2), radix: 16) ?? 0; + } + return int.tryParse(s) ?? 0; +}If you expect very large BigInt values (beyond int range), consider returning BigInt or documenting truncation behavior.
lib/widgets/nft_display_utils/tree_nft_view_details_with_map.dart (1)
63-73: Complete the theming pass for this widgetThere are still hard-coded colors (e.g.,
Colors.green,Colors.grey.shade50/200). Replace with theme tokens to avoid clashes in dark mode.I can send a follow-up diff if you share intended tokens for borders/surfaces.
Also applies to: 154-157
lib/providers/wallet_provider.dart (4)
708-711: Log message is fine; consider including session/topic to aid debuggingOptional: include session topic or current address in the log for traces.
-logger.d("Tried to disconnect"); +logger.d("Attempting disconnect (addr: $_currentAddress, chain: $_currentChainId)");
114-115: No-op string literal in _onSessionConnect
('Session connected: ...');does nothing. Replace with a logger call or remove.- ('Session connected: ${event.session.topic}'); + logger.d('Session connected: ${event.session.topic}');
399-404: Logic nit:if (!sessions.isNotEmpty)Use
if (sessions.isEmpty)for readability and correctness.- if (!sessions.isNotEmpty) { + if (sessions.isEmpty) {
610-616: Ineffective normalization of deep link base
replaceAll('://', '://')is a no-op. Append the scheme when missing.- if (!deepLinkBase.endsWith('://')) { - deepLinkBase = deepLinkBase.replaceAll('://', '://'); - } + if (!deepLinkBase.endsWith('://')) { + deepLinkBase = '$deepLinkBase://'; + }lib/widgets/profile_widgets/profile_section_widget.dart (1)
266-268: Remove unnecessary formatting changesThese lines only have formatting changes (removing
constfrom Offset) without any functional changes. Consider reverting to maintain cleaner diff.- color: Colors.black12, - blurRadius: 6, - offset: Offset(0, 3), + color: Colors.black12, // shadow color + blurRadius: 6, // shadow softness + offset: const Offset(0, 3), // shadow positionlib/utils/constants/ui/color_constants.dart (2)
5-6: Consider using const for color constantsThe color constants should be
constfor better performance and to indicate they're compile-time constants.-Color primaryYellowColor = Color.fromARGB(255, 251, 251, 99); -Color primaryGreenColor = Color.fromARGB(255, 28, 211, 129); +const Color primaryYellowColor = Color.fromARGB(255, 251, 251, 99); +const Color primaryGreenColor = Color.fromARGB(255, 28, 211, 129);
8-54: Consider caching theme colors to avoid repeated Provider lookupsThe function creates a new Map on every call and performs Provider lookup each time. For better performance, consider caching the result when theme doesn't change.
You could optimize this by either:
- Making the Map a computed property in ThemeProvider itself
- Using a memoization pattern to cache results per theme mode
Example approach with ThemeProvider:
// In ThemeProvider class Map<String, Color> get themeColors => _isDarkMode ? _darkThemeColors : _lightThemeColors; static const Map<String, Color> _lightThemeColors = { 'primary': Color.fromARGB(255, 28, 211, 129), // ... other colors }; static const Map<String, Color> _darkThemeColors = { 'primary': Color.fromARGB(255, 0, 128, 70), // ... other colors };lib/widgets/map_widgets/flutter_map_widget.dart (2)
283-285: Inconsistent theming - hardcoded color remainsLine 279 still uses a hardcoded
Colors.bluefor the container background while the text color is themed. This creates inconsistency in the theming implementation.decoration: BoxDecoration( - color: Colors.blue, + color: getThemeColors(context)['primary'] ?? Colors.blue, borderRadius: BorderRadius.circular(4), ),
461-465: StaticDisplayMap not fully themedThe
StaticDisplayMapwidget still uses hardcoded colors (e.g., red location pin, white/black in various places) whileCoordinatesMapis themed. This creates visual inconsistency.Consider applying the same theming approach to
StaticDisplayMapfor consistency:- child: const Icon( + child: Icon( Icons.location_pin, - color: Colors.red, + color: getThemeColors(context)['marker'] ?? Colors.red, size: 40, ),lib/pages/register_user_page.dart (1)
437-437: Missing context parameter documentationThe
_buildFormFieldfunction now requires aBuildContextparameter but this isn't documented. Consider adding a doc comment explaining the parameter's purpose.Add documentation above the function:
/// Builds a form field with themed styling. /// /// The [context] is required to access theme colors via Provider. Widget _buildFormField({ // ... parameters })lib/pages/mint_nft/mint_nft_details.dart (3)
63-65: Consider using consistent theme color keys for similar UI states.The snackbar background uses
'secondaryButton'for error and'primaryButton'for success. This naming suggests button colors rather than state colors. Consider using more semantic keys like'error'and'success'for better clarity.- backgroundColor: isError - ? getThemeColors(context)['secondaryButton'] - : getThemeColors(context)['primaryButton'], + backgroundColor: isError + ? getThemeColors(context)['error'] ?? getThemeColors(context)['secondaryButton'] + : getThemeColors(context)['success'] ?? getThemeColors(context)['primaryButton'],
272-272: Hard-coded color remains in themed widget.While most colors have been migrated to the theme system, this label still uses a hard-coded green color.
- color: Color(0xFF1CD381), + color: getThemeColors(context)['primary'],
283-284: Hard-coded border colors not fully themed.The border uses hard-coded colors that should be managed through the theme system.
- border: Border.all( - color: const Color(0xFFFAEB96), + border: Border.all( + color: getThemeColors(context)['secondary'] ?? const Color(0xFFFAEB96),lib/pages/mint_nft/mint_nft_coordinates.dart (2)
212-212: Inconsistent snackbar theming.The error snackbar still uses hard-coded colors while the success case uses themed colors. This should be consistent with the theme system.
- backgroundColor: - isError ? Colors.red.shade400 : const Color(0xFF1CD381), + backgroundColor: isError + ? getThemeColors(context)['error'] ?? Colors.red.shade400 + : getThemeColors(context)['primary'] ?? const Color(0xFF1CD381),
548-549: Mixed theming approach in coordinate field decorations.Some elements use hard-coded colors while others use theme colors within the same component.
- color: const Color(0xFF1CD381), + color: getThemeColors(context)['primary'],- color: Color(0xFF1CD381), + color: getThemeColors(context)['primary'],Also applies to: 563-563
lib/pages/tree_details_page.dart (2)
141-141: Inconsistent color usage in detail cards.Some containers still use hard-coded colors while others are themed.
- color: Color.fromARGB(255, 251, 251, 99), + color: getThemeColors(context)['secondary'] ?? primaryYellowColor,- color: const Color(0xFFFF4E63), + color: getThemeColors(context)['accent'] ?? const Color(0xFFFF4E63),Also applies to: 164-164
372-372: Hard-coded snackbar colors should use theme system.Several snackbars still use hard-coded colors instead of the theme system.
- backgroundColor: Colors.blue.shade600, + backgroundColor: getThemeColors(context)['info'] ?? Colors.blue.shade600,- backgroundColor: Colors.green.shade600, + backgroundColor: getThemeColors(context)['success'] ?? Colors.green.shade600,Also applies to: 398-398, 605-605
lib/pages/mint_nft/mint_nft_images.dart (1)
133-133: Hard-coded color in button style.The "Remove All" button still uses a hard-coded red color.
- style: TextButton.styleFrom(foregroundColor: Colors.red), + style: TextButton.styleFrom( + foregroundColor: getThemeColors(context)['error'] ?? Colors.red + ),lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart (3)
248-248: Hard-coded colors in clipboard feedback.The snackbar for clipboard feedback uses hard-coded colors.
- backgroundColor: Colors.green.shade600, + backgroundColor: getThemeColors(context)['success'] ?? Colors.green.shade600,Also applies to: 250-250
518-518: Inconsistent theming in detail view.Several UI elements still use hard-coded colors from Material's color palette.
- color: Colors.grey.shade500, + color: getThemeColors(context)['textSecondary'] ?? Colors.grey.shade500,- color: Colors.grey.shade100, - child: Icon( - Icons.broken_image, - color: Colors.grey.shade400, + color: getThemeColors(context)['backgroundSecondary'] ?? Colors.grey.shade100, + child: Icon( + Icons.broken_image, + color: getThemeColors(context)['iconSecondary'] ?? Colors.grey.shade400,Also applies to: 757-761
1016-1016: Hard-coded error color in removal dialog.The remove button uses a hard-coded red color.
- backgroundColor: Colors.red.shade600, + backgroundColor: getThemeColors(context)['error'] ?? Colors.red.shade600,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
android/app/src/main/AndroidManifest.xml(2 hunks)lib/components/universal_navbar.dart(8 hunks)lib/pages/mint_nft/mint_nft_coordinates.dart(16 hunks)lib/pages/mint_nft/mint_nft_details.dart(13 hunks)lib/pages/mint_nft/mint_nft_images.dart(12 hunks)lib/pages/mint_nft/submit_nft_page.dart(6 hunks)lib/pages/organisations_pages/user_organisations_page.dart(1 hunks)lib/pages/register_user_page.dart(15 hunks)lib/pages/tree_details_page.dart(25 hunks)lib/providers/wallet_provider.dart(2 hunks)lib/utils/constants/ui/color_constants.dart(1 hunks)lib/utils/services/conversion_functions.dart(1 hunks)lib/widgets/basic_scaffold.dart(2 hunks)lib/widgets/map_widgets/flutter_map_widget.dart(12 hunks)lib/widgets/map_widgets/static_map_display_widget.dart(1 hunks)lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart(34 hunks)lib/widgets/nft_display_utils/tree_nft_view_details_with_map.dart(4 hunks)lib/widgets/nft_display_utils/tree_nft_view_widget.dart(2 hunks)lib/widgets/nft_display_utils/user_nfts_widget.dart(3 hunks)lib/widgets/profile_widgets/profile_section_widget.dart(3 hunks)lib/widgets/wallet_not_connected_widget.dart(1 hunks)lib/widgets/wrong_chain_widget.dart(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Flutter CI
lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart
[error] Dart format check failed (exit code 1). Reformatted 45 files; please review and commit the changes. Run 'dart format' again if needed.
🔇 Additional comments (16)
lib/pages/mint_nft/submit_nft_page.dart (1)
136-139: Verify theme map keys exist (‘primary’, ‘primaryBorder’, ‘icon’)These reads mix null-asserted and nullable access. If any key is absent, you’ll crash at runtime or get unexpected nulls.
Would you confirm color_constants.dart defines: primary, primaryBorder, icon, error, errorContainer, onErrorContainer? If not, I can update the helper and call sites.
Also applies to: 145-145
lib/widgets/map_widgets/static_map_display_widget.dart (1)
37-37: LGTM — cosmetic change onlyComment removal has no functional impact.
lib/widgets/nft_display_utils/user_nfts_widget.dart (2)
387-392: LGTM — themed error colorUsing getThemeColors for the error icon aligns with the PR’s theming goals.
413-417: LGTM — themed empty-state iconGood move to theme the empty-state icon.
lib/widgets/nft_display_utils/tree_nft_view_details_with_map.dart (1)
4-4: LGTM — theme adoption is consistentGood use of getThemeColors for background, shadow, and text.
Also applies to: 65-69, 147-147, 163-163
lib/providers/wallet_provider.dart (2)
210-223: Stricter connect guards — good, but note API change for callersThrowing on “already connected/in progress” is safer, but it changes behavior. Ensure UI call sites handle exceptions and present user-friendly messages.
147-156: accountsChanged payload handling may set a non-address valueMany providers send full CAIP-10 account strings or hex addresses. You’re assigning
accounts.firstdirectly without parsing. This can break downstream code expecting a hex address.If
accounts.firstcontainsnamespace:chainId:address, split by:and take the last segment (as done elsewhere).- final newAddress = accounts.first.toString(); + final raw = accounts.first.toString(); + final parts = raw.split(':'); + final newAddress = parts.isNotEmpty ? parts.last : raw;lib/widgets/wallet_not_connected_widget.dart (1)
9-21: Resolved — Connect affordance still present in UniversalNavbarUniversalNavbar renders its connect button when the wallet is disconnected, and BaseScaffold uses UniversalNavbar as the appBar, so screens using BaseScaffold already expose a connect CTA (lib/components/universal_navbar.dart, lib/widgets/basic_scaffold.dart). Re-add an inline "Connect Wallet" button to buildWalletNotConnectedWidget only if you need a body-level CTA for screens that don't use BaseScaffold.
lib/widgets/profile_widgets/profile_section_widget.dart (1)
490-491: Safe theme color access implementationGood use of null-safe operators when accessing theme colors, properly handling potential missing keys.
Also applies to: 498-498
lib/widgets/basic_scaffold.dart (1)
35-72: Well-structured wallet state handlingThe refactored code properly handles different wallet states (connecting, not connected, wrong chain) with appropriate UI responses. Good use of Consumer pattern for reactive updates.
lib/components/universal_navbar.dart (3)
47-49: Clean implementation of optional leading widgetThe conditional rendering of the leading widget is well implemented using spread operator syntax.
369-397: Comprehensive wallet connection state handlingExcellent implementation of visual feedback for different connection states (connecting, connected, disconnected) with appropriate colors and icons. The retry functionality with
forceReconnectis a good UX improvement.Also applies to: 421-437
66-66: Confirmed: asset paths existassets/tree-navbar-images/logo.png and the other referenced images are present; no change required.
lib/pages/mint_nft/mint_nft_coordinates.dart (1)
140-152: Good error handling for geohash generation!The addition of try-catch for geohash generation with proper logging and user feedback is a solid improvement for error handling.
lib/pages/tree_details_page.dart (1)
92-95: Good documentation for coordinate conversion!The comments clearly explain the contract data transformation, which is helpful for maintainability.
lib/pages/mint_nft/mint_nft_images.dart (1)
164-169: Good migration to theme-aware text styling!The text styling now properly uses the theme's color scheme, improving consistency.
| borderRadius: BorderRadius.circular(16), | ||
| border: Border.all( | ||
| color: const Color(0xFFFAEB96), | ||
| color: getThemeColors(context)['secondaryBorder']!, |
There was a problem hiding this comment.
Undefined theme key 'secondaryBorder'
Another undefined key being accessed from the theme colors Map.
Fix by using an existing key or adding the missing one:
- color: getThemeColors(context)['secondaryBorder']!,
+ color: getThemeColors(context)['primaryBorder'] ?? Colors.grey,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| color: getThemeColors(context)['secondaryBorder']!, | |
| color: getThemeColors(context)['primaryBorder'] ?? Colors.grey, |
🤖 Prompt for AI Agents
In lib/pages/register_user_page.dart around line 309, the code accesses a
non-existent theme map key 'secondaryBorder'; fix by either replacing that key
with an existing key from getThemeColors (e.g., use the established
border/secondary key your project uses) or add 'secondaryBorder' to the theme
colors map/provider (update the map literal and its type/interface where
getThemeColors is defined), and guard usage with a safe fallback (e.g., use the
map value or a default color) to satisfy null-safety.
| child: Icon( | ||
| icon, | ||
| color: const Color(0xFF1CD381), | ||
| color: getThemeColors(context)['primary'], |
There was a problem hiding this comment.
Multiple non-null assertions on theme colors
Consistent with other files, there are multiple unsafe uses of ! when accessing Map values.
Apply defensive null-checking:
- color: getThemeColors(context)['primary'],
+ color: getThemeColors(context)['primary'] ?? Colors.green,- color: getThemeColors(context)['primary']!,
+ color: getThemeColors(context)['primary'] ?? Colors.green,- color: getThemeColors(context)['border']!,
+ color: getThemeColors(context)['border'] ?? Colors.grey,- color: getThemeColors(context)['shadow']!,
+ color: getThemeColors(context)['shadow'] ?? Colors.black12,- color: getThemeColors(context)['textPrimary']!,
+ color: getThemeColors(context)['textPrimary'] ?? Colors.black,- color: getThemeColors(context)['textSecondary']!,
+ color: getThemeColors(context)['textSecondary'] ?? Colors.grey,Also applies to: 462-462, 473-473, 478-478, 490-490, 496-496
🤖 Prompt for AI Agents
In lib/pages/register_user_page.dart around lines 452, 462, 473, 478, 490 and
496, the code uses multiple non-null assertions (!) when accessing values from
getThemeColors(context); replace these unsafe accesses by first assigning the
map to a local variable, then defensively retrieving each color with null-aware
fallbacks (e.g., final theme = getThemeColors(context); final primary =
theme?['primary'] ?? Theme.of(context).colorScheme.primary; and similarly for
other keys), and use those local variables where needed so you avoid runtime
exceptions if map entries are absent.
| borderRadius: BorderRadius.circular(16), | ||
| border: Border.all( | ||
| color: const Color(0xFFFAEB96), | ||
| color: getThemeColors(context)['border']!, |
There was a problem hiding this comment.
Undefined theme key 'border'
The code uses getThemeColors(context)['border'] but the getThemeColors function doesn't define a 'border' key. It defines 'primaryBorder' and 'BNWBorder' instead.
Fix the key name:
- color: getThemeColors(context)['border']!,
+ color: getThemeColors(context)['primaryBorder'] ?? Colors.grey,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| color: getThemeColors(context)['border']!, | |
| color: getThemeColors(context)['primaryBorder'] ?? Colors.grey, |
🤖 Prompt for AI Agents
In lib/pages/register_user_page.dart around line 473, the code accesses
getThemeColors(context)['border'] which is undefined; replace the 'border' key
with one of the defined keys (e.g., 'primaryBorder' or 'BNWBorder') — most
likely change to getThemeColors(context)['primaryBorder'] to match the existing
theme map (and keep the null-assertion if appropriate).
| boxShadow: [ | ||
| BoxShadow( | ||
| color: const Color(0xFF1CD381), | ||
| color: getThemeColors(context)['shadow']!, |
There was a problem hiding this comment.
Undefined theme key 'shadow'
The code uses getThemeColors(context)['shadow'] but this key is not defined in the getThemeColors function.
Either add the 'shadow' key to the theme colors Map or use an existing color:
- color: getThemeColors(context)['shadow']!,
+ color: Colors.black.withOpacity(0.1),Or add to getThemeColors:
'shadow': themeProvider.isDarkMode
? Colors.black.withOpacity(0.3)
: Colors.black.withOpacity(0.1),🤖 Prompt for AI Agents
In lib/pages/register_user_page.dart around line 478 the code accesses
getThemeColors(context)['shadow'] which is undefined; fix this by either
updating getThemeColors to include a 'shadow' entry (returning an appropriate
Color that respects dark/light mode) or replace the lookup with an existing
defined key (e.g., 'divider' or 'overlay') so the map access is valid and
null-safe; ensure any added color follows the app's theme conventions and use
null-aware access if needed.
| Future<String?> forceReconnect() async { | ||
| _updateStatus("Forcing reconnection..."); | ||
| _isConnected = false; | ||
| _isConnecting = false; | ||
| _currentAddress = null; | ||
| _currentChainId = null; | ||
| notifyListeners(); | ||
|
|
||
| try { | ||
| await disconnectWallet(); | ||
| final uri = await connectWallet(); // Return the URI | ||
| return uri; | ||
| } catch (e) { | ||
| _updateStatus("Force reconnect failed: ${e.toString()}"); | ||
| _isConnecting = false; | ||
| notifyListeners(); | ||
| rethrow; | ||
| } | ||
| } |
There was a problem hiding this comment.
forceReconnect never disconnects the live session (state reset happens before early-return check)
You set _isConnected = false then call disconnectWallet(), which immediately returns if not connected. Result: stale WalletConnect session remains.
Future<String?> forceReconnect() async {
- _updateStatus("Forcing reconnection...");
- _isConnected = false;
- _isConnecting = false;
- _currentAddress = null;
- _currentChainId = null;
- notifyListeners();
-
try {
- await disconnectWallet();
+ _updateStatus("Forcing reconnection...");
+ await disconnectWallet(); // actually disconnect the session
+ // Reset local state after disconnect
+ _isConnected = false;
+ _isConnecting = false;
+ _currentAddress = null;
+ _currentChainId = null;
+ notifyListeners();
final uri = await connectWallet();
return uri;
} catch (e) {
_updateStatus("Force reconnect failed: ${e.toString()}");
_isConnecting = false;
notifyListeners();
rethrow;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Future<String?> forceReconnect() async { | |
| _updateStatus("Forcing reconnection..."); | |
| _isConnected = false; | |
| _isConnecting = false; | |
| _currentAddress = null; | |
| _currentChainId = null; | |
| notifyListeners(); | |
| try { | |
| await disconnectWallet(); | |
| final uri = await connectWallet(); // Return the URI | |
| return uri; | |
| } catch (e) { | |
| _updateStatus("Force reconnect failed: ${e.toString()}"); | |
| _isConnecting = false; | |
| notifyListeners(); | |
| rethrow; | |
| } | |
| } | |
| Future<String?> forceReconnect() async { | |
| try { | |
| _updateStatus("Forcing reconnection..."); | |
| await disconnectWallet(); // actually disconnect the session | |
| // Reset local state after disconnect | |
| _isConnected = false; | |
| _isConnecting = false; | |
| _currentAddress = null; | |
| _currentChainId = null; | |
| notifyListeners(); | |
| final uri = await connectWallet(); | |
| return uri; | |
| } catch (e) { | |
| _updateStatus("Force reconnect failed: ${e.toString()}"); | |
| _isConnecting = false; | |
| notifyListeners(); | |
| rethrow; | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/providers/wallet_provider.dart around lines 189 to 207, the method resets
connection state (_isConnected/_isConnecting/_currentAddress/_currentChainId)
before calling disconnectWallet(), which makes disconnectWallet() return early
and leaves the live WalletConnect session intact; move the disconnect call to
occur before any state resets (or change disconnectWallet to accept/handle a
force flag and bypass its early-return when forceReconnect is invoked) and only
update/reset local state after disconnectWallet completes (or after confirming
the session is terminated), then proceed to connectWallet and return the new
URI.
| 'BNWBorder': themeProvider.isDarkMode | ||
| ? const Color.fromARGB(255, 1, 135, 12) | ||
| : const Color.fromARGB(255, 28, 211, 129), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent theme key naming: 'BNWBorder' vs other keys
The key 'BNWBorder' doesn't follow the camelCase convention used by other keys like 'primaryBorder', 'textPrimary', etc.
- 'BNWBorder': themeProvider.isDarkMode
+ 'bnwBorder': themeProvider.isDarkModeAlso update all usages of this key throughout the codebase.
Also applies to: 473-473
🤖 Prompt for AI Agents
In lib/utils/constants/ui/color_constants.dart around lines 20-22, the theme key
'BNWBorder' uses inconsistent naming compared to camelCase keys like
'primaryBorder' and 'textPrimary'; rename the key to follow camelCase (e.g.,
'bnwBorder' or better 'primarySuccessBorder' depending on semantic intent) and
update the map entry accordingly, then search the entire codebase for all usages
of 'BNWBorder' and replace them with the new key name to keep naming consistent
and avoid runtime lookup failures.
| decoration: BoxDecoration( | ||
| border: Border.all(color: Colors.grey), | ||
| border: | ||
| Border.all(color: getThemeColors(context)['primaryBorder']!), |
There was a problem hiding this comment.
Unsafe Map access with non-null assertions
Multiple instances of unsafe access to theme color Map values that could cause runtime crashes.
Apply null-safe access pattern:
- Border.all(color: getThemeColors(context)['primaryBorder']!),
+ Border.all(color: getThemeColors(context)['primaryBorder'] ?? Colors.green),- color: getThemeColors(context)['marker'],
+ color: getThemeColors(context)['marker'] ?? Colors.red,- color: getThemeColors(context)['BNWBorder']!,
+ color: getThemeColors(context)['bnwBorder'] ?? Colors.grey,Also applies to: 149-149, 200-200
🤖 Prompt for AI Agents
In lib/widgets/map_widgets/flutter_map_widget.dart around lines 42, 149, and
200, there are unsafe non-null assertions when accessing entries from the theme
color Map (e.g., getThemeColors(context)['primaryBorder']!) that can crash if
the key is missing or null; replace each usage with a null-safe access pattern:
read the map into a local variable, use map['key'] ?? <fallbackColor> or
map['key'] ?? Theme.of(context).colorScheme.<suitableColor> (or
Colors.transparent) to provide a safe default, and avoid the trailing !; ensure
the resulting value is the correct Color type before passing it to Border.all or
other widgets.
| decoration: BoxDecoration( | ||
| border: Border.all(color: Colors.green, width: 2), | ||
| border: Border.all( | ||
| color: getThemeColors(context)['primaryBorder']!, width: 2), |
There was a problem hiding this comment.
Potential null dereference risk with theme colors
The code uses non-null assertion (!) when accessing theme colors without verifying that the keys exist in the returned Map. If getThemeColors doesn't return the expected keys, this will cause a runtime crash.
Consider adding null-safe access:
- color: getThemeColors(context)['primaryBorder']!, width: 2),
+ color: getThemeColors(context)['primaryBorder'] ?? Colors.grey, width: 2),- color: getThemeColors(context)['background']!,
+ color: getThemeColors(context)['background'] ?? Colors.white,Also applies to: 25-25
🤖 Prompt for AI Agents
In lib/widgets/nft_display_utils/tree_nft_view_widget.dart around lines 23 and
25, the code uses a non-null assertion when accessing
getThemeColors(context)['primaryBorder'] which can throw if the key is missing;
change it to a null-safe access and provide a fallback (e.g. use
getThemeColors(context)['primaryBorder'] ?? Theme.of(context).dividerColor or
another sensible default) or explicitly handle the null by checking containsKey
and branching to a default color before using it; update both occurrences to
avoid the '!' operator and ensure the widget does not crash when the key is
absent.
| color: getThemeColors(context)['background'], | ||
| border: Border.all( | ||
| color: Colors.black, | ||
| color: getThemeColors(context)['primaryBorder']!, |
There was a problem hiding this comment.
Non-null assertions on theme colors without validation
Similar to other files, using ! on Map access is risky if the keys aren't guaranteed to exist.
Apply defensive programming:
- color: getThemeColors(context)['primaryBorder']!,
+ color: getThemeColors(context)['primaryBorder'] ?? Colors.green,- color: getThemeColors(context)['icon'],
+ color: getThemeColors(context)['icon'] ?? Colors.grey,- getThemeColors(context)['primary']!,
+ getThemeColors(context)['primary'] ?? Colors.green,Also applies to: 24-24, 42-42
🤖 Prompt for AI Agents
In lib/widgets/wrong_chain_widget.dart around lines 14, 24 and 42, the code uses
non-null assertion (!) on Map lookups from getThemeColors(context) which can
throw if keys are missing; replace each usage with a null-safe fallback:
retrieve the color with getThemeColors(context)['key'] ??
Theme.of(context).<appropriateColor> (e.g., primaryColor, dividerColor) or a
sensible constant like Colors.grey, and if necessary add an assert or guard that
logs/throws a clear error when the map is unexpectedly null; ensure required
imports (Theme and Colors) are available and keep behavior consistent across all
three lines.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart (2)
256-266: Null-safety: avoid treeDetails!.id dereference.Current code will throw if treeDetails is null. Either assert non-null at the callsite or guard in the callee before use.
-Future<void> _removeVerifier(Verifier verifier, BuildContext context, - Tree? treeDetails, Function loadTreeDetails) async { +Future<void> _removeVerifier(Verifier verifier, BuildContext context, + Tree? treeDetails, Function loadTreeDetails) async { + if (treeDetails == null) { + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: const Text("Tree details are unavailable."), + backgroundColor: getThemeColors(context)['error'], + behavior: SnackBarBehavior.floating, + ), + ); + } + return; + } @@ - treeId: treeDetails!.id, + treeId: treeDetails.id,Optionally, make treeDetails non-nullable and update the call site to pass a non-null value.
Also applies to: 1017-1019
791-813: Status chip icon is same color as its background (invisible).When isActive, both container and icon use primary; the icon blends into the background.
- decoration: BoxDecoration( - color: verifier.isActive - ? getThemeColors(context)['primary']! - : Colors.red.shade50, + decoration: BoxDecoration( + color: verifier.isActive + ? getThemeColors(context)['primaryLight']! + : getThemeColors(context)['error']!.withOpacity(0.08), @@ - color: verifier.isActive - ? getThemeColors(context)['primary']! - : Colors.red.shade200, + color: verifier.isActive + ? getThemeColors(context)['primary']! + : getThemeColors(context)['error']!.withOpacity(0.3), @@ - color: verifier.isActive - ? getThemeColors(context)['primary'] - : Colors.red.shade600, + color: verifier.isActive + ? getThemeColors(context)['icon'] + : getThemeColors(context)['error'],
🧹 Nitpick comments (7)
lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart (7)
244-253: Eliminate hard-coded Colors.*; use theme tokens for full consistency.There are several remaining direct Colors.* usages that bypass the new theme system (snackbars, cards, icons, greys, error states). Replace them with tokens from getThemeColors(context) (or add missing tokens like textSecondary, onPrimary, onError, surface, errorLight, errorBorder if needed).
Apply representative fixes below; repeat the pattern for the other occurrences called out in the line ranges:
@@ _copyToClipboard - const Icon(Icons.check_circle, color: Colors.white, size: 16), + Icon(Icons.check_circle, color: getThemeColors(context)['icon'], size: 16), @@ - backgroundColor: Colors.green.shade600, + backgroundColor: getThemeColors(context)['primary'],@@ _buildVerifierCard - color: Colors.white, + color: themeColors['background'], @@ - border: Border.all(color: primaryYellowColor), + border: Border.all(color: themeColors['primaryBorder']!), @@ - child: const Icon( + child: Icon( Icons.verified_user, - color: Colors.white, + color: themeColors['icon'], size: 20, ), @@ - Icon( + Icon( Icons.copy, size: 12, - color: Colors.black, + color: themeColors['icon'], ),@@ remove button chip - boxShadow: [ - BoxShadow( - color: Colors.black, + boxShadow: [ + BoxShadow( + color: themeColors['primaryBorder']!, blurRadius: 6, offset: const Offset(0, 2), ), ], @@ - border: Border.all(width: 2), + border: Border.all(width: 2, color: themeColors['primaryBorder']!),@@ Image error and skeleton - color: Colors.grey.shade100, + color: getThemeColors(context)['background'], @@ - color: Colors.grey.shade400, + color: getThemeColors(context)['icon'],@@ Remove dialog (address pill + destructive button) - color: Colors.white, + color: getThemeColors(context)['background'], @@ - backgroundColor: Colors.red.shade600, - foregroundColor: Colors.white, + backgroundColor: getThemeColors(context)['error'], + // TODO: switch to getThemeColors(context)['onError'] once available + foregroundColor: Colors.white,Also replace the remaining greys (e.g., grey.shade500/600/700) with theme tokens (e.g., textSecondary, iconMuted) to keep contrast consistent across light/dark.
Also applies to: 417-421, 434-447, 477-477, 519-519, 535-535, 564-564, 759-763, 796-801, 812-812, 930-931, 947-951, 979-979, 988-988, 1011-1012, 1021-1022, 1042-1043
383-385: Avoid using primary as body text color; use a text token.Primary-colored body copy can fail contrast. Prefer textSecondary or textPrimary with reduced opacity.
- style: TextStyle( - color: themeColors['primary']!, - fontSize: 12, - ), + style: TextStyle( + color: themeColors['textPrimary']!.withOpacity(0.7), + fontSize: 12, + ),
551-553: Unify approach: pass BuildContext, not theme map.Mixing a Map parameter in some helpers and BuildContext in others adds inconsistency. Compute themeColors inside the helper.
-void _showVerifierDetailsModal( - Verifier verifier, BuildContext context, Map<String, Color> themeColors) { +void _showVerifierDetailsModal(Verifier verifier, BuildContext context) { + final themeColors = getThemeColors(context); @@ - _showVerifierDetailsModal(verifier, context, themeColors); + _showVerifierDetailsModal(verifier, context);Also applies to: 585-586
363-363: Avoid primaryYellowColor; route via theme tokens.Hard-coding an accent undermines dark mode. Use a themed accent/border token.
- color: primaryYellowColor, + color: themeColors['icon'], @@ - border: Border.all(color: primaryYellowColor), + border: Border.all(color: themeColors['primaryBorder']!),If you need a dedicated accent, add it to getThemeColors (e.g., 'accent').
Also applies to: 419-419
55-57: Guard shortAddress against unexpectedly short strings.Substring will throw if address length < 10. Defensive handling avoids runtime errors during partial loads or test data.
- return "${address.substring(0, 6)}...${address.substring(address.length - 4)}"; + if (address.length <= 10) return address; + return "${address.substring(0, 6)}...${address.substring(address.length - 4)}";
50-53: Consider locale-aware timestamp formatting.Manual string assembly can produce inconsistent formats; prefer intl DateFormat with locale.
// final date = DateTime.fromMillisecondsSinceEpoch(timestamp * 1000); // return DateFormat('yyyy-MM-dd HH:mm').format(date);
239-247: User-facing strings should be localized.Multiple hard-coded strings in widgets/dialogs/snackbars. Extract to your i18n layer to enable translations.
Also applies to: 273-283, 293-297, 326-340, 368-374, 707-717, 719-733, 837-842, 905-916, 1010-1028
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart(34 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart (1)
5-5: Good move to centralized theming via getThemeColors.Consistently sourcing colors from getThemeColors(context) improves maintainability and sets you up for light/dark modes. Keep going in this direction across the remaining hard-coded colors noted below.
Also applies to: 304-304, 352-355, 371-371, 401-401, 615-621, 631-639, 664-665, 677-682, 698-699, 731-731, 770-778, 795-801, 810-811, 830-842, 860-868, 877-877, 903-903, 921-924, 950-951, 999-999
|
|
||
| logger.d( | ||
| "Rendering verifiers section with ${treeDetails.verifiers.length} verifiers"); | ||
| final isOwner = treeDetails.owner == loggedInUser; |
There was a problem hiding this comment.
Owner check should be case-insensitive for addresses.
Checksum-cased Ethereum addresses will fail strict equality and block legitimate owners from removal.
- final isOwner = treeDetails.owner == loggedInUser;
+ final isOwner = treeDetails.owner.toLowerCase() == (loggedInUser ?? '').toLowerCase();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/widgets/nft_display_utils/tree_nft_details_verifiers_widget.dart around
line 345, the owner check uses strict equality (final isOwner =
treeDetails.owner == loggedInUser;) which fails for differently-cased Ethereum
addresses; change it to a case-insensitive comparison by normalizing both sides
(e.g. compare treeDetails.owner?.trim().toLowerCase() to
loggedInUser?.trim().toLowerCase()), and handle nulls safely so the check
returns false if either value is null.
| onPressed: () => Navigator.pop(context), | ||
| style: ElevatedButton.styleFrom( | ||
| backgroundColor: themeColors['primary'], | ||
| backgroundColor: getThemeColors(context)['background'], | ||
| foregroundColor: Colors.white, | ||
| padding: const EdgeInsets.symmetric(vertical: 12), | ||
| shape: RoundedRectangleBorder( | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| ), | ||
| child: const Text("Close"), | ||
| child: Text( | ||
| "Close", | ||
| style: TextStyle( | ||
| color: getThemeColors(context)['textPrimary']), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Close button styling conflicts and likely poor contrast.
foregroundColor is set to white while the child Text sets a different color; background uses a “background” token (often light), reducing contrast.
- style: ElevatedButton.styleFrom(
- backgroundColor: getThemeColors(context)['background'],
- foregroundColor: Colors.white,
+ style: ElevatedButton.styleFrom(
+ backgroundColor: getThemeColors(context)['primary'],
padding: const EdgeInsets.symmetric(vertical: 12),
shape: RoundedRectangleBorder(
borderRadius: BorderRadius.circular(8),
),
),
- child: Text(
- "Close",
- style: TextStyle(
- color: getThemeColors(context)['textPrimary']),
- ),
+ child: const Text("Close"),Consider centralizing ElevatedButtonTheme at the app level to avoid per-button styling.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onPressed: () => Navigator.pop(context), | |
| style: ElevatedButton.styleFrom( | |
| backgroundColor: themeColors['primary'], | |
| backgroundColor: getThemeColors(context)['background'], | |
| foregroundColor: Colors.white, | |
| padding: const EdgeInsets.symmetric(vertical: 12), | |
| shape: RoundedRectangleBorder( | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| ), | |
| child: const Text("Close"), | |
| child: Text( | |
| "Close", | |
| style: TextStyle( | |
| color: getThemeColors(context)['textPrimary']), | |
| ), | |
| ), | |
| onPressed: () => Navigator.pop(context), | |
| style: ElevatedButton.styleFrom( | |
| backgroundColor: getThemeColors(context)['primary'], | |
| padding: const EdgeInsets.symmetric(vertical: 12), | |
| shape: RoundedRectangleBorder( | |
| borderRadius: BorderRadius.circular(8), | |
| ), | |
| ), | |
| child: const Text("Close"), | |
| ), |
Added
Summary by CodeRabbit
New Features
Style
Bug Fixes
Chores