Feat/enhance frontend - #23
Conversation
…mprove error handling, and refactor geocoding logic in utils.py
…tate management and layer updates for improved user interaction
… selected mountain highlighting and legend toggle functionality
…n tracking and current location button
…er onboarding experience
… handling, and UI updates across components
… enhancing visual representation and interaction
…tes management in hooks, and UI updates across components
There was a problem hiding this comment.
Pull Request Overview
This pull request adds several new features to the PeakSight application: a favorites system for mountains, a tutorial for first-time users, and bear sighting information overlay on the map. The changes also include performance optimizations through caching for bear data imports and adjustments to tile caching duration.
- Added client-side favorites management using localStorage with toggle functionality in mountain details
- Implemented an interactive tutorial system that shows on first visit and can be dismissed permanently
- Integrated bear sighting data from NHK news articles with LLM-based analysis and geocoding, displayed as custom markers on the map
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/hooks/useFavorites.ts | New hook for managing mountain favorites in localStorage |
| frontend/src/components/Tutorial.tsx | New tutorial component with multi-slide walkthrough |
| frontend/src/components/FavoritesModal.tsx | New modal for viewing and accessing favorite mountains |
| frontend/src/components/PanelContent.tsx | Added favorite toggle button and bear sighting details panel |
| frontend/src/components/MapTerrain.tsx | Implemented custom markers for mountains and bears with selection states, clustering logic for overlapping bears, and favorites filtering |
| frontend/src/components/Map.tsx | Added collapsible legend with bear sightings and favorites filter toggle |
| frontend/src/components/Header.tsx | Added buttons for favorites modal and tutorial with count badge |
| frontend/src/components/ContextPanel.tsx | Props updated to pass through favorites and bear handlers |
| frontend/src/components/BottomSheet.tsx | Props updated to pass through favorites and bear handlers |
| frontend/src/app/page.tsx | Integrated favorites hook, tutorial state, bear data fetching, and favorites filtering logic |
| frontend/next.config.ts | Added NHK image domain to remote patterns (needs correction) |
| frontend/src/app/api/proxy/tiles/[...path]/route.ts | Changed cache duration from 1 year to 30 days with Redis expiration |
| frontend/src/app/api/proxy/maps/[...style]/route.ts | Changed cache duration from 1 year to 30 days with Redis expiration |
| backend/commons/utils.py | Enhanced geocoding to support prefecture-only queries with Japanese comments |
| backend/commons/import_bear_sight.py | Added LLM and DB result caching to reduce API calls and improve performance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| latitude=coordinates[0] if coordinates else 0.0, | ||
| longitude=coordinates[1] if coordinates else 0.0, | ||
| summary=llm_result.summary or "", | ||
| **{k: v for k, v in sighting_data.items() if k != "reported_at"}, |
There was a problem hiding this comment.
This dictionary comprehension filter {k: v for k, v in sighting_data.items() if k != "reported_at"} is fragile. If sighting_data structure changes, this could silently include unwanted fields. Consider explicitly listing the fields to unpack:
sighting = BearSighting(
prefecture=sighting_data["prefecture"],
city=sighting_data["city"],
latitude=sighting_data["latitude"],
longitude=sighting_data["longitude"],
summary=sighting_data["summary"],
image_url=sighting_data["image_url"],
source_url=url,
reported_at=reported_at,
)| **{k: v for k, v in sighting_data.items() if k != "reported_at"}, | |
| prefecture=sighting_data["prefecture"], | |
| city=sighting_data["city"], | |
| latitude=sighting_data["latitude"], | |
| longitude=sighting_data["longitude"], | |
| summary=sighting_data["summary"], | |
| image_url=sighting_data["image_url"], |
|
|
||
| if (mountainsHash !== "empty" && (isFirstLoad || hasChanged)) { | ||
| // showOnlyFavoritesがtrueの場合は常に更新 | ||
| if (displayMountainsHash !== "empty" && (isFirstLoad || hasChanged || showOnlyFavorites)) { |
There was a problem hiding this comment.
The condition showOnlyFavorites in the dependency check causes mountains to be updated on every render when favorites filter is enabled. This is inefficient. The condition should check if hasChanged is true, not add an OR condition with showOnlyFavorites:
if (displayMountainsHash !== "empty" && (isFirstLoad || hasChanged)) {The displayMountainsHash already accounts for showOnlyFavorites state (line 1118), so the explicit OR is redundant and causes unnecessary updates.
| if (displayMountainsHash !== "empty" && (isFirstLoad || hasChanged || showOnlyFavorites)) { | |
| if (displayMountainsHash !== "empty" && (isFirstLoad || hasChanged)) { |
| await redis.set(cacheKey, dataBuffer, { | ||
| expiration: { | ||
| type: "EX", | ||
| value: 2592000, | ||
| }, |
There was a problem hiding this comment.
Redis expiration and HTTP Cache-Control max-age are now aligned (both 30 days), but this contradicts the immutable directive. Immutable resources should have longer cache durations. This change appears across multiple cache configuration points (lines 30, 68, 78). Consider using a constant to ensure consistency:
const CACHE_MAX_AGE = 2592000; // 30 days in seconds| const bearsEventHandlers = useRef<{ | ||
| handleClick?: ( | ||
| e: maplibregl.MapMouseEvent & { | ||
| features?: maplibregl.MapGeoJSONFeature[]; | ||
| }, | ||
| ) => void; | ||
| handleMouseEnter?: ( | ||
| e: maplibregl.MapMouseEvent & { | ||
| features?: maplibregl.MapGeoJSONFeature[]; | ||
| }, | ||
| ) => void; | ||
| handleMouseLeave?: () => void; | ||
| }>({}); | ||
|
|
There was a problem hiding this comment.
Unused variable bearsEventHandlers.
| const bearsEventHandlers = useRef<{ | |
| handleClick?: ( | |
| e: maplibregl.MapMouseEvent & { | |
| features?: maplibregl.MapGeoJSONFeature[]; | |
| }, | |
| ) => void; | |
| handleMouseEnter?: ( | |
| e: maplibregl.MapMouseEvent & { | |
| features?: maplibregl.MapGeoJSONFeature[]; | |
| }, | |
| ) => void; | |
| handleMouseLeave?: () => void; | |
| }>({}); |
| const bearsGeoJSON = useMemo((): GeoJSON.FeatureCollection => { | ||
| const features = bears | ||
| .filter( | ||
| bear => | ||
| bear.longitude !== null && | ||
| bear.longitude !== undefined && | ||
| bear.latitude !== null && | ||
| bear.latitude !== undefined, | ||
| ) | ||
| .map(bear => ({ | ||
| type: "Feature" as const, | ||
| properties: { | ||
| id: bear.id, | ||
| prefecture: bear.prefecture, | ||
| city: bear.city, | ||
| summary: bear.summary, | ||
| }, | ||
| geometry: { | ||
| type: "Point" as const, | ||
| coordinates: [bear.longitude, bear.latitude] as [number, number], | ||
| }, | ||
| })); | ||
| return { | ||
| type: "FeatureCollection", | ||
| features, | ||
| }; | ||
| }, [bears]); |
There was a problem hiding this comment.
Unused variable bearsGeoJSON.
| const bearsGeoJSON = useMemo((): GeoJSON.FeatureCollection => { | |
| const features = bears | |
| .filter( | |
| bear => | |
| bear.longitude !== null && | |
| bear.longitude !== undefined && | |
| bear.latitude !== null && | |
| bear.latitude !== undefined, | |
| ) | |
| .map(bear => ({ | |
| type: "Feature" as const, | |
| properties: { | |
| id: bear.id, | |
| prefecture: bear.prefecture, | |
| city: bear.city, | |
| summary: bear.summary, | |
| }, | |
| geometry: { | |
| type: "Point" as const, | |
| coordinates: [bear.longitude, bear.latitude] as [number, number], | |
| }, | |
| })); | |
| return { | |
| type: "FeatureCollection", | |
| features, | |
| }; | |
| }, [bears]); |
…sModal, enhance UI interactions in MapTerrain, and streamline state management in hooks
…fe in HomePage, BottomSheet, ContextPanel, MapTerrain, and useFavorites
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const removeFavorite = (mountainId: number) => { | ||
| setFavorites(prev => { | ||
| const updated = prev.filter(m => m.id !== mountainId); | ||
| localStorage.setItem(FAVORITES_KEY, JSON.stringify(updated)); |
There was a problem hiding this comment.
Missing window check before accessing localStorage in removeFavorite. The addFavorite function correctly checks typeof window !== 'undefined' on line 28, but removeFavorite directly accesses localStorage on line 39 without this check. This will cause errors during SSR.
| localStorage.setItem(FAVORITES_KEY, JSON.stringify(updated)); | |
| if (typeof window !== "undefined") { | |
| localStorage.setItem(FAVORITES_KEY, JSON.stringify(updated)); | |
| } |
| const stored = localStorage.getItem(FAVORITES_KEY); | ||
| if (stored) { | ||
| try { | ||
| setFavorites(JSON.parse(stored)); | ||
| } catch (error) { | ||
| console.error("Failed to parse favorites:", error); |
There was a problem hiding this comment.
Missing window check before accessing localStorage in useEffect. During SSR, localStorage is not available and will throw a ReferenceError. Add if (typeof window !== 'undefined') check before accessing localStorage.getItem.
| const stored = localStorage.getItem(FAVORITES_KEY); | |
| if (stored) { | |
| try { | |
| setFavorites(JSON.parse(stored)); | |
| } catch (error) { | |
| console.error("Failed to parse favorites:", error); | |
| if (typeof window !== "undefined") { | |
| const stored = localStorage.getItem(FAVORITES_KEY); | |
| if (stored) { | |
| try { | |
| setFavorites(JSON.parse(stored)); | |
| } catch (error) { | |
| console.error("Failed to parse favorites:", error); | |
| } |
| } | ||
| // 座標を丸めてグループ化するヘルパー関数 | ||
| const roundCoordinate = (coord: number, precision: number = 6): number => { | ||
| const factor = Math.pow(10, precision); |
There was a problem hiding this comment.
[nitpick] Use Math.pow(10, precision) instead of the more modern exponentiation operator. Consider using 10 ** precision for consistency with modern JavaScript practices.
| const factor = Math.pow(10, precision); | |
| const factor = 10 ** precision; |
| class CachedResult: | ||
| def __init__(self, data): | ||
| self.is_sighting = data.get("is_sighting", False) | ||
| self.prefecture = data.get("prefecture") | ||
| self.city = data.get("city") | ||
| self.summary = data.get("summary") |
There was a problem hiding this comment.
[nitpick] Local class definition inside a function reduces code clarity. Consider defining CachedResult at module level or using a dataclass/NamedTuple for better type safety and reusability.
| : response.data.results || []; | ||
| setBears(bearsData); | ||
| } else { | ||
| console.error("Failed to fetch bears:", response); |
There was a problem hiding this comment.
Error message lacks detail about what failed. The console.error on line 70 should include the status code and error details for better debugging: console.error('Failed to fetch bears:', response.status, response.error)
| console.error("Failed to fetch bears:", response); | |
| console.error("Failed to fetch bears:", response.status, response.error); |
This pull request introduces major improvements to both the backend and frontend of the application, focusing on enhanced caching for bear sighting data, integration of bear sightings into the frontend map and UI, and optimizations for cache management and geocoding. The backend now implements persistent caching for LLM analysis and DB results, reducing redundant API calls and improving efficiency. The frontend is updated to display bear sightings on the map, allow user interaction with these sightings, and provide new features like favorites and tutorials. Additionally, cache durations for map data are adjusted for better cache control.
Backend Enhancements:
Bear Sighting Data Caching and Processing:
bears_cache/llmandbears_cache/db), minimizing repeated LLM and geocoding calls for the same article URLs. This includes new helper functions for loading and saving cache entries, and logic inmain()to utilize these caches before making external calls. [1] [2] [3] [4]Geocoding Utility Improvements:
Frontend Enhancements:
Bear Sighting Integration and UI Improvements:
BottomSheet,ContextPanel, andMapPageClientcomponents to support bear sighting selection and display, in addition to mountains and paths. [1] [2] [3]Cache Duration Adjustments:
Image Domain Configuration:
imgu.web.nhkin Next.js config to support bear sighting images from NHK.