feat: map explore page #29
Conversation
…on using EXIF data
- Implemented GeoJsonLayer component for rendering GeoJSON data on the map. - Created MapBackButton for easy navigation back to the gallery. - Added MapControls component for user geolocation and navigation controls. - Developed MapInfoPanel to display information about markers and bounds. - Introduced MapLoadingState to show loading indicators while fetching data. - Built Mapbox component to integrate Mapbox GL with custom markers and popups. - Created PhotoMarkerPin for displaying photo markers on the map. - Developed PhotoPopup for showing detailed information about selected photos. - Added PureMapbox component for a simplified Mapbox integration. - Updated index files to export new components and hooks. - Implemented hooks for managing map bounds, marker selection, and loading photo markers. - Created utility functions for handling GPS coordinates and calculating map bounds. - Built MapSection and MapboxContainer components to manage map state and interactions. - Updated Explory page to utilize new map components and improve loading/error handling. - Defined TypeScript types for map-related data structures.
|
Thanks for your wonderful work 🚀. However, I was thinking that maybe we could abstract the map functionality into providers, rather than tying it specifically to Mapbox. This way, developers could easily switch to another map provider if needed. After all, using Mapbox requires registration and obtaining a token, and it may also incur fees if usage is high. 🤔 |
- MapLibre dependencies - abstracted MapProvider to choose MapLibre/Mapbox - config control specific provider/priority
Thank you for your advice. After 1.
|
|
I believe the functionality for the standalone map page is now complete. To keep this PR focused, I'd like to avoid adding more complexity here. Could the maintainers please review and consider merging? @MaxtuneLee @Innei |
|
Very cool work! 🔥 However, I noticed a small issue — when two photos are located at the same (or very close) spot, it's not possible to click on the one behind. Maybe the interaction could be improved when photos are too close together? ⬆️ There are actually two photos here. ⬆️ When zoomed in, the two images overlap so tightly that I can't click the one underneath. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new “map explore” feature to display photo locations using Mapbox and MapLibre.
- Defines a
mapconfiguration insite.config.tsand updatesconfig.example.json - Adds localization keys for the map UI in all supported languages
- Implements a generic map provider system (MapProvider, adapters, types, utilities) and integrates it into a new
/explorypage and gallery action
Reviewed Changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| site.config.ts | Added MapConfig type and optional map field |
| config.example.json | Provided example map settings |
| locales/app/*.json | Added action.map.explore and explory.* keys |
| apps/web/src/types/map/*.ts | Defined map-related types and provider interfaces |
| apps/web/src/modules/map/ | Implemented MapProvider, adapters, utils, and UI |
| apps/web/src/pages/(main)/explory/index.tsx | Created map exploration page component |
| apps/web/src/modules/gallery/ActionGroup.tsx | Added “map explore” button to gallery actions |
Comments suppressed due to low confidence (3)
apps/web/src/lib/map-utils.ts:101
- Consider adding unit tests for
convertPhotosToMarkersFromEXIF(and related GPS parsing utilities) to ensure correct behavior with various EXIF data formats and edge cases.
export function convertPhotosToMarkersFromEXIF(
locales/app/en.json:4
- [nitpick] The label 'Map Explore' reads awkwardly; consider using 'Explore Map' for better clarity and consistency with other UI text.
"action.map.explore": "Map Explore",
apps/web/src/modules/gallery/ActionGroup.tsx:320
- The Button component is used here but not imported; please add
import { Button } from '~/components/ui/button'.
<Button
| * Hook to get the current map adapter from the context. | ||
| */ | ||
| export const useMapAdapter = () => { | ||
| const context = use(MapContext) |
There was a problem hiding this comment.
The hook use(MapContext) is incorrect for consuming React context; please use React's useContext(MapContext) instead of use().
| const context = use(MapContext) | |
| const context = useContext(MapContext) |
Copilot uses AI. Check for mistakes.
|
|
||
| const value = useMemo(() => ({ adapter }), [adapter]) | ||
|
|
||
| return <MapContext value={value}>{children}</MapContext> |
There was a problem hiding this comment.
Context provider usage is incorrect; wrap children in <MapContext.Provider value={value}> rather than using MapContext directly as a component.
| return <MapContext value={value}>{children}</MapContext> | |
| return <MapContext.Provider value={value}>{children}</MapContext.Provider> |
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| // Simple clustering algorithm for small datasets | ||
| function clusterMarkers(markers: PhotoMarker[], zoom: number): ClusterPoint[] { |
There was a problem hiding this comment.
The simple distance-based clustering algorithm is O(n²) and may not scale well for large marker sets. Consider using a spatial index or a library like Supercluster for more efficient clustering.
Copilot uses AI. Check for mistakes.
…teractions - Introduced a new HoverCard component for displaying additional information on photo markers. - Implemented a GlassButton component for improved button aesthetics and interactions. - Updated Map components to utilize the new HoverCard and GlassButton, enhancing user experience. - Added new styles and refactored existing components for better visual consistency. Signed-off-by: Innei <tukon479@gmail.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Added zoom in, zoom out, compass, and geolocation buttons to Mapbox and MapLibre components for better user interaction. - Refactored MapInfoPanel to improve layout and styling, including enhanced coordinate display and coverage area calculation. - Updated animations and styles for a more cohesive visual experience across map components. Signed-off-by: Innei <tukon479@gmail.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ared utilities - Introduced clustering functionality for photo markers, improving the display of grouped photos on the map. - Added shared components for GeoJSON layers, cluster markers, and photo marker pins to streamline code and enhance reusability. - Updated Mapbox and MapLibre components to utilize new shared utilities, improving maintainability and consistency across the application. - Enhanced styling and interactions for cluster markers and photo markers, providing a better user experience. Signed-off-by: Innei <tukon479@gmail.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Innei <tukon479@gmail.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Added autoFitBounds prop to MapLibre and GenericMap components to automatically adjust the map view to fit all markers. - Updated MapInfoPanel to include collapsible sections for better organization of coordinate and coverage area information. - Introduced a new MapLibreStyle.json for improved map styling. - Refactored MapControls to update the location icon for better visual consistency. Signed-off-by: Innei <tukon479@gmail.com>
There was a problem hiding this comment.
Bug: ClusterPhotoGrid Fails Date Validation
The ClusterPhotoGrid component's date range calculation is flawed. It uses DateTimeOriginal EXIF data without validating date string formats. Invalid date strings result in new Date() creating 'Invalid Date' objects, which return NaN for getTime(). This leads to unpredictable sorting and incorrect display of the earliest and latest dates.
apps/web/src/components/ui/map/ClusterPhotoGrid.tsx#L127-L133
afilmory/apps/web/src/components/ui/map/ClusterPhotoGrid.tsx
Lines 127 to 133 in 68823c9
apps/web/src/lib/map-utils.ts#L126-L132
afilmory/apps/web/src/lib/map-utils.ts
Lines 126 to 132 in 68823c9
Was this report helpful? Give feedback by reacting with 👍 or 👎
|
I removed mapbox support because I didn't have the energy to maintain the UI/UX for both maps. I redesigned the UI/UX for the current map, thanks for your contribution! |
This PR introduces a new map feature for displaying photo locations using Mapbox. If this feature isn't welcomed to afilmory, just close this PR directly and I'd respect your decision.
TODO
sync selected marker status to url likeafilmory/apps/web/src/pages/(main)/layout.tsx
Line 93 in fcd9c31
open in amap => show in map component