feat: add map view functionality with tree markers and user location#26
feat: add map view functionality with tree markers and user location#26LogicalGuy77 wants to merge 5 commits intoStabilityNexus:mainfrom
Conversation
WalkthroughAdds a new map feature: route and bottom-nav entry, MapViewPage UI with wallet/user-location gating, MapProvider state, map UI widgets, and TreeMapService to fetch/filter tree NFTs by bounding box or geohash. Changes
Sequence DiagramsequenceDiagram
actor User
participant MapViewPage
participant LocationService
participant WalletProvider
participant MapProvider
participant TreeMapService
participant ContractReadFunctions
User->>MapViewPage: Open /map
MapViewPage->>LocationService: request user location
LocationService-->>MapViewPage: return coords
MapViewPage->>MapProvider: setUserLocation(coords)
MapViewPage->>WalletProvider: isConnected?
alt wallet connected
MapViewPage->>MapProvider: getBoundingBox()
MapViewPage->>TreeMapService: getTreesInBoundingBox(bounds, max=100)
TreeMapService->>ContractReadFunctions: fetch recent tree entries
ContractReadFunctions-->>TreeMapService: raw entries
TreeMapService-->>MapViewPage: parsed & filtered trees
MapViewPage->>MapProvider: setLoadedTrees(trees)
MapViewPage->>User: render markers & controls
else wallet not connected
MapViewPage-->>User: prompt to connect wallet
end
User->>MapViewPage: tap marker
MapViewPage->>MapViewPage: select tree, show TreeInfoCard
User->>MapViewPage: "View Details"
MapViewPage->>Router: navigate to /trees/{id}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 10
🧹 Nitpick comments (7)
lib/utils/services/tree_map_service.dart (1)
26-30: Consider making the fetch limit configurable or documenting the limitation.Both
getTreesInBoundingBoxandgetTreesNearLocationfetch only 50 trees from the contract, then filter client-side. For large map areas, this may miss many trees. Consider either:
- Making the limit configurable via a parameter
- Implementing pagination to fetch all matching trees
- Documenting this limitation clearly
static Future<List<Tree>> getTreesInBoundingBox({ required WalletProvider walletProvider, required double minLat, required double maxLat, required double minLng, required double maxLng, int maxTrees = 100, + int fetchLimit = 50, // Backend fetch limit before filtering }) async {Also applies to: 92-96
lib/pages/map_view_page.dart (3)
147-149: Eliminate code duplication by reusingTreeMapService._convertCoordinate.This method duplicates the logic from
TreeMapService._convertCoordinate. Since the service method is private, consider either:
- Making
TreeMapService._convertCoordinatepublic- Creating a shared utility function
In
tree_map_service.dart:- static double _convertCoordinate(int coordinate) { + static double convertCoordinate(int coordinate) {Then in this file:
- double _convertCoordinate(int coordinate) { - return (coordinate / 1000000.0) - 90.0; - }And update usages:
- final lat = _convertCoordinate(tree.latitude); - final lng = _convertCoordinate(tree.longitude); + final lat = TreeMapService.convertCoordinate(tree.latitude); + final lng = TreeMapService.convertCoordinate(tree.longitude);
87-89: Consider using a different UI pattern for empty results.Setting an error message when no trees are found treats a valid empty state as an error. Consider showing a separate informational message or a different visual indicator instead of using the error banner.
if (trees.isEmpty) { - mapProvider.setError("No trees found in this area. Try moving the map or zooming out."); + // Consider adding a separate info message state in MapProvider + // mapProvider.setInfoMessage("No trees found in this area. Try moving the map or zooming out."); }
291-291:withOpacityis deprecated in newer Flutter versions.Consider using
Color.fromARGBorwithValuesfor better performance and forward compatibility.- color: Colors.black.withOpacity(0.2), + color: Colors.black.withValues(alpha: 0.2),Also applies to: 328-328
lib/providers/map_provider.dart (2)
99-112:getBoundingBox()should be derived from actual visible bounds, not zoom-only heuristics.
0.05 / zoomis unlikely to match the map widget viewport and will vary across devices/aspect ratios. Prefer feeding the provider the real bounds from the map (MapCamera.visibleBounds/ controller bounds) and returning that.Minimal API change idea:
- Map<String, double> getBoundingBox() { - final latDelta = 0.05 / _currentZoom; - final lngDelta = 0.05 / _currentZoom; - return { ... }; - } + Map<String, double> getBoundingBoxFromBounds(LatLngBounds bounds) { + return { + 'minLat': bounds.south, + 'maxLat': bounds.north, + 'minLng': bounds.west, + 'maxLng': bounds.east, + }; + }(Then have the page pass bounds into the fetch call.)
35-67: Optional: reduce redundant rebuilds by only notifying on actual state changes.
setCurrentCenter,setCurrentZoom,setLoading, etc. alwaysnotifyListeners(). Consider early-return when the value is unchanged (especially if map emits frequent camera updates).lib/widgets/map_widgets/tree_map_widgets.dart (1)
7-45: Nice: marker widgets are simple and composable; considerSemantics/ larger tap target.If these are used inside map markers, consider giving the container a fixed
width/height(e.g., 36–44) and aSemantics(label: ...)for accessibility/screen readers.Also applies to: 47-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/main.dart(4 hunks)lib/pages/map_view_page.dart(1 hunks)lib/providers/map_provider.dart(1 hunks)lib/utils/constants/bottom_nav_constants.dart(1 hunks)lib/utils/constants/route_constants.dart(1 hunks)lib/utils/services/tree_map_service.dart(1 hunks)lib/widgets/map_widgets/tree_map_widgets.dart(1 hunks)
🔇 Additional comments (5)
lib/utils/services/tree_map_service.dart (2)
165-171: Verify decodeGeohash output order matches GeoHasher.decode.The
decodeGeohashmethod assumesdecoded[0]is latitude anddecoded[1]is longitude. Given thatencodetakes(longitude, latitude), the decode output order should be verified to ensure consistency.
81-82: Parameter order is correct. Thedart_geohashlibrary'sGeoHasher.encodemethod expects(longitude, latitude)order, and the code correctly uses this sequence.lib/utils/constants/bottom_nav_constants.dart (1)
32-37: LGTM!The new Map navigation item is correctly implemented with appropriate icons and route reference, consistent with the existing navigation items pattern.
lib/utils/constants/route_constants.dart (1)
4-4: LGTM!The route constants follow the established pattern in this file, maintaining consistency with existing route definitions.
Also applies to: 12-12
lib/main.dart (1)
6-6: LGTM!The map view integration is well-structured:
- Route definition follows the existing pattern
MapProvideris correctly registered in theMultiProvider- Imports are properly organized
Also applies to: 21-21, 157-163, 177-177
- Fix longitude conversion to use correct offset (180 vs 90) - Replace inaccurate custom Math class with dart:math library - Split _convertCoordinate into separate lat/lng functions - Add null guards for getThemeColors to prevent runtime errors - Remove 88 lines of unreliable Taylor series approximations This fixes critical bugs in distance calculations and coordinate display.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/widgets/map_widgets/tree_map_widgets.dart (1)
127-150: Remove the extra vertical gap whenhasUserLocation == false(unconditionalSizedBox).- // Center on user location - if (hasUserLocation) - Container( + // Center on user location + if (hasUserLocation) ...[ + Container( decoration: BoxDecoration( color: Colors.white, borderRadius: BorderRadius.circular(8), boxShadow: [ BoxShadow( color: Colors.black.withValues(alpha: 0.2), blurRadius: 4, offset: const Offset(0, 2), ), ], ), child: IconButton( icon: const Icon(Icons.my_location, color: Colors.blue), onPressed: onCenterUser, tooltip: 'Center on Me', ), ), - const SizedBox(height: 8), + const SizedBox(height: 8), + ],
🧹 Nitpick comments (1)
lib/widgets/map_widgets/tree_map_widgets.dart (1)
196-205: Consider deduplicating lat/lng decoding (service + card implement the same codec).A small shared utility (e.g.,
CoordinateCodec.decodeLatitudeMicro(int)/decodeLongitudeMicro(int)) reduces drift risk.Also applies to: 279-284
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/pages/map_view_page.dart(1 hunks)lib/utils/services/tree_map_service.dart(1 hunks)lib/widgets/map_widgets/tree_map_widgets.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/pages/map_view_page.dart
🔇 Additional comments (2)
lib/utils/services/tree_map_service.dart (1)
132-153: Core geo helpers look solid (haversine + correct lat/lng fixed-point decoding + prefix match).Also applies to: 199-209, 211-234
lib/widgets/map_widgets/tree_map_widgets.dart (1)
151-178: Theme color fallback looks good; loading/disabled states are wired correctly.Also applies to: 302-313
- Fix planting_proposals_tab.dart: was missing offsets entirely - Fix recent_trees_widget.dart: was using lat offset for longitude - Ensures all tree locations display correctly in Trees list and proposals - Brings all coordinate conversions in line with blockchain encoding This fixes coordinate display bugs in non-map components that would show trees at incorrect GPS locations by 90-180 degrees.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/widgets/nft_display_utils/recent_trees_widget.dart (1)
96-106: Duplicate coordinate conversion logic.These functions are identical to those in
planting_proposals_tab.dart(lines 122-132). Please extract them to a shared utility module as suggested in the review comment for that file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/widgets/nft_display_utils/recent_trees_widget.dart(2 hunks)lib/widgets/organisation_details_page/tabs/planting_proposals_tab.dart(2 hunks)
🔇 Additional comments (2)
lib/widgets/organisation_details_page/tabs/planting_proposals_tab.dart (1)
210-211: LGTM!The coordinate conversion usage is correct, and displaying 6 decimal places provides appropriate precision (~0.11 meters) for tree locations.
lib/widgets/nft_display_utils/recent_trees_widget.dart (1)
263-263: LGTM!The coordinate display is correctly implemented with appropriate precision.
- Fix map tree details navigation: /tree-details -> /trees to match router - Update fetch limits to respect maxTrees parameter in both bounding box and geohash queries - Ensures at least maxTrees can be returned after filtering (was hard-capped at 50)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
lib/utils/services/tree_map_service.dart (1)
22-24: Don’t log precise location bounds (PII) in app logs.Those debug logs can leak sensitive user location context if collected remotely; sanitize (round/coarsen) or gate behind debug-only.
+import 'package:flutter/foundation.dart'; ... - logger.d("Fetching trees in bounding box: " - "lat[$minLat, $maxLat], lng[$minLng, $maxLng]"); + if (!kReleaseMode) { + logger.d("Fetching trees in bounding box"); + }lib/pages/map_view_page.dart (1)
23-34: DisposeMapControllerindispose().class _MapViewPageState extends State<MapViewPage> { final MapController _mapController = MapController(); Tree? _selectedTree; bool _isInitialized = false; @@ void initState() { @@ } + + @override + void dispose() { + _mapController.dispose(); + super.dispose(); + }
🧹 Nitpick comments (3)
lib/utils/services/tree_map_service.dart (1)
176-197: Harden_parseTreeFromMapagainst partial/null contract fields.Casting
treeData['species'] as String(etc.) will throw if the field is null/missing; you catch and skip the tree, which can amplify “no trees found” for minor data issues. Consider defaults or more targeted validation.lib/pages/map_view_page.dart (2)
147-155: Deduplicate coordinate conversion helpers.
_convertLatitude/_convertLongitudeare duplicated here and inTreeMapService; consider a shared util (or re-useTreeMapServicehelpers) to avoid future drift.Also applies to: 194-207
100-110: UseMapEventMoveEndinstead ofMapEventMoveto avoid excessive Provider updates.
MapEventMovefires continuously while dragging the map, causingsetCurrentCenter/Zoomto notify listeners on every event and potentially causing jank. PreferMapEventMoveEnd, which fires once when the drag completes. If you need live updates during dragging, debounceMapEventMovewith a Timer (300ms duration is typical) or userxdart.debounceTime().
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/pages/map_view_page.dart(1 hunks)lib/utils/services/tree_map_service.dart(1 hunks)
| Future<void> _initializeMap() async { | ||
| if (_isInitialized) return; | ||
|
|
||
| final mapProvider = Provider.of<MapProvider>(context, listen: false); | ||
|
|
||
| try { | ||
| // Get user location | ||
| final location = await LocationService().getCurrentLocation(); | ||
| if (location.isValid) { | ||
| mapProvider.setUserLocation(LatLng(location.latitude!, location.longitude!)); | ||
| logger.d("User location set: ${location.latitude}, ${location.longitude}"); | ||
| } | ||
|
|
||
| _isInitialized = true; | ||
|
|
||
| // Load initial trees | ||
| await _loadTreesInArea(); | ||
| } catch (e) { | ||
| logger.e("Error initializing map: $e"); | ||
| mapProvider.setError("Failed to initialize map: $e"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid requesting precise location before the user opts in / wallet is connected.
Right now _initializeMap() calls getCurrentLocation() even when you’re rendering the “Connect Your Wallet” prompt; this can trigger permissions unexpectedly and is privacy-sensitive.
Future<void> _initializeMap() async {
if (_isInitialized) return;
final mapProvider = Provider.of<MapProvider>(context, listen: false);
+ final walletProvider = Provider.of<WalletProvider>(context, listen: false);
try {
+ if (!walletProvider.isConnected) {
+ _isInitialized = true;
+ return;
+ }
// Get user location
final location = await LocationService().getCurrentLocation();Also consider gating the log line that prints exact lat/lng.
Also applies to: 63-66
| logger.d("User location set: ${location.latitude}, ${location.longitude}"); | ||
| } |
There was a problem hiding this comment.
Don’t log exact user location / bounds in production logs.
(At minimum, gate behind debug-only or remove the coordinates.)
Also applies to: 74-75
🤖 Prompt for AI Agents
In lib/pages/map_view_page.dart around lines 46-47 (and similarly at 74-75) the
code logs exact user latitude/longitude which must not be emitted in production;
either remove the coordinate values from logs or gate the logs behind a
debug-only check (e.g., use Flutter's kDebugMode) so they only run in
development, or else redact/round the coordinates to coarse values (city-level
or use buckets) before logging; update the logging calls accordingly and add the
necessary import for the debug constant if you choose the kDebugMode guard.
| // Fetch a large batch of recent trees | ||
| // In future, this could be optimized with backend or contract-level filtering | ||
| final result = await ContractReadFunctions.getRecentTreesPaginated( | ||
| walletProvider: walletProvider, | ||
| offset: 0, | ||
| limit: math.max(50, maxTrees), // Fetch enough trees to satisfy maxTrees after filtering | ||
| ); |
There was a problem hiding this comment.
Single “recent trees” page can miss nearby trees; consider paging until enough matches.
Right now you only fetch offset: 0 and filter client-side; if relevant trees aren’t in the first page, the map may show “no trees” even when they exist nearby.
- final result = await ContractReadFunctions.getRecentTreesPaginated(
- walletProvider: walletProvider,
- offset: 0,
- limit: math.max(50, maxTrees),
- );
+ final int pageSize = math.max(50, maxTrees);
+ final List<dynamic> treesData = [];
+ for (var offset = 0; treesData.length < pageSize * 10; offset += pageSize) {
+ final result = await ContractReadFunctions.getRecentTreesPaginated(
+ walletProvider: walletProvider,
+ offset: offset,
+ limit: pageSize,
+ );
+ if (!result.success || result.data == null) break;
+ final page = (result.data['trees'] ?? []) as List<dynamic>;
+ if (page.isEmpty) break;
+ treesData.addAll(page);
+ // Optionally: early-exit if you already found enough matches while parsing/filtering.
+ }(Apply similarly in getTreesNearLocation.)
Also applies to: 92-98
| /// Get geohash for a coordinate | ||
| static String getGeohash(double latitude, double longitude, {int precision = 6}) { | ||
| return _geoHasher.encode(longitude, latitude, precision: precision); | ||
| } | ||
|
|
||
| /// Get neighboring geohashes | ||
| static List<String> getNeighboringGeohashes(String geohash) { | ||
| return _geoHasher.neighbors(geohash).values.toList(); | ||
| } | ||
|
|
||
| /// Decode geohash to coordinates | ||
| static Map<String, double> decodeGeohash(String geohash) { | ||
| final decoded = _geoHasher.decode(geohash); | ||
| return { | ||
| 'latitude': decoded[0], | ||
| 'longitude': decoded[1], | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd pubspec.yaml -type fRepository: StabilityNexus/Treee
Length of output: 234
🏁 Script executed:
grep -A 20 "dart_geohash" pubspec.yaml 2>/dev/null | head -30Repository: StabilityNexus/Treee
Length of output: 711
🌐 Web query:
dart_geohash 2.1.0 GeoHasher encode decode parameter order coordinate order documentation
💡 Result:
In dart_geohash (including v2.1.0) GeoHasher.encode expects (longitude, latitude) — i.e. lon first, lat second. The package examples and API docs show calls like geoHasher.encode(-98, 38) and decode returns [longitude, latitude]. [1][2]
Sources:
[1] dart_geohash package page (examples).
[2] dart_geohash example / API docs (encode/decode usage).
Fix inverted coordinate order in decodeGeohash().
The dart_geohash library's decode() method returns [longitude, latitude], but the code assigns them backwards:
'latitude': decoded[0], // decoded[0] is longitude
'longitude': decoded[1], // decoded[1] is latitude
This silently swaps coordinates and will misplace all markers. Correct to:
'latitude': decoded[1],
'longitude': decoded[0],
🤖 Prompt for AI Agents
In lib/utils/services/tree_map_service.dart around lines 155 to 172 the
decodeGeohash method assigns decoded values in the wrong order:
dart_geohash.decode() returns [longitude, latitude] but the code maps decoded[0]
to 'latitude' and decoded[1] to 'longitude'. Swap the assignments so 'latitude'
gets decoded[1] and 'longitude' gets decoded[0], keeping the return map keys the
same.
1 participant
Fixes: #24
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.