feat(map): add map popup content and dynamic styling #66
Conversation
- Added popup component with point name and description - Implemented dark mode styling for map popups - Added hover effects and selected point state management - Improved map marker interactions and visual feedback BREAKING CHANGE: MapPoint type properties changed (label -> name, added description)
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update enhances the map and sidebar interactivity by synchronizing hover and selection states between map markers, sidebar items, and dashboard cards. It introduces map popups that display location names and descriptions, adds dynamic styling for dark mode, and refines store types and logic to support these features and interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant DashboardCard
participant MapClient
participant MapStore
User->>Sidebar: Hover/click sidebar item
Sidebar->>MapStore: Set selectedPoint
MapStore->>MapClient: Notify selectedPoint change
MapClient->>Map: Highlight marker, show popup
User->>DashboardCard: Hover/click card
DashboardCard->>MapStore: Set selectedPoint
MapStore->>MapClient: Notify selectedPoint change
MapClient->>Map: Highlight marker, show popup
User->>MapClient: Hover/click marker
MapClient->>MapStore: Set selectedPoint
MapClient->>Map: Show popup, highlight marker
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
assets/css/main.css (1)
11-37: Consider consolidating repetitive tip styling.The dark theme popup styling is comprehensive and handles all anchor positions correctly. However, the tip styling could be optimized.
Consider consolidating the repetitive border color assignments:
.maplibregl-popup-tip { - @apply border-t-gray-800; + @apply border-gray-800; } - .maplibregl-popup-anchor-bottom .maplibregl-popup-tip, - .maplibregl-popup-anchor-bottom-right .maplibregl-popup-tip, - .maplibregl-popup-anchor-bottom-left .maplibregl-popup-tip { - @apply border-t-gray-800; - } - - .maplibregl-popup-anchor-top .maplibregl-popup-tip, - .maplibregl-popup-anchor-top-right .maplibregl-popup-tip, - .maplibregl-popup-anchor-top-left .maplibregl-popup-tip { - @apply border-b-gray-800; - } - - .maplibregl-popup-anchor-left .maplibregl-popup-tip { - @apply border-r-gray-800; - } - - .maplibregl-popup-anchor-right .maplibregl-popup-tip { - @apply border-l-gray-800; - }This approach sets a base border color and relies on MapLibre's default tip styling for directional borders.
stores/map.ts (1)
32-46: LGTM! Well-designed animation control logic.The selected point navigation logic effectively manages fly-to animations and provides appropriate fallback behavior. The separation of concerns between bounds fitting and point selection is well-implemented.
The TODO comment at line 34 indicates a future enhancement for configurable animations. Would you like me to help implement a user preference system for controlling map animations, or should this be tracked in a separate issue?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
assets/css/main.css(1 hunks)components/app/map.client.vue(1 hunks)components/sidebar-button.vue(2 hunks)lib/types.ts(1 hunks)pages/dashboard.vue(2 hunks)pages/dashboard/index.vue(2 hunks)stores/locations.ts(1 hunks)stores/map.ts(1 hunks)stores/sidebar.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
stores/sidebar.ts (1)
lib/types.ts (1)
MapPoint(14-18)
stores/map.ts (1)
lib/types.ts (1)
MapPoint(14-18)
🔇 Additional comments (16)
stores/sidebar.ts (2)
1-1: LGTM: Clean import for MapPoint type.The import is properly structured and necessary for the new location property in SidebarItem.
8-8: LGTM: Well-designed optional location property.The optional
locationproperty withMapPoint | nulltype provides the necessary link between sidebar items and map points while maintaining backward compatibility.lib/types.ts (1)
16-17: LGTM: Breaking change properly implemented.The property rename from "label" to "name" and the addition of "description" are well-designed for the popup enhancement. The nullable description type provides appropriate flexibility.
assets/css/main.css (1)
8-10: LGTM: Appropriate close button styling.The padding utility class provides better visual spacing for the popup close button.
components/sidebar-button.vue (2)
7-7: LGTM: Well-typed iconColor prop.The optional prop with specific string literal types provides type safety while maintaining backward compatibility.
24-28: LGTM: Clean implementation of dynamic icon styling.The CSS class binding properly applies the iconColor prop to provide visual feedback for selected states.
stores/locations.ts (2)
17-17: LGTM: Enhanced sidebar item with location association.Adding the full location object to sidebar items properly supports the new map integration functionality.
19-19: Verify external/api/locationsresponse matches updated MapPoint typeI wasn’t able to locate a local handler for
/api/locationsin this repo, which suggests you’re calling an external service or a backend not checked in here. Before assigning the fulldata.valuearray tomapStore.mapPoints, please confirm that the endpoint returns objects shaped like:
id: number|stringname: stringdescription: stringlat: numberlong: numberRecommended steps:
- Review the API contract or OpenAPI/OpenAPI-like spec for
/api/locations.- Manually test or write an integration test (e.g. with a curl call or via your test suite) to ensure
nameanddescriptionare always present.- (Optional) Add runtime schema validation (e.g. Zod, io-ts) around your
useFetchcall to catch mismatches early.components/app/map.client.vue (3)
32-40: LGTM! Well-implemented interactive marker behavior.The tooltip implementation correctly handles the breaking change from
point.labeltopoint.name, adds appropriate cursor styling, and properly manages hover states without triggering unwanted map animations through theselectedPointWithoutFlyTomethod.
44-44: LGTM! Clear visual feedback for selected markers.The dynamic icon color implementation provides excellent visual feedback by highlighting selected markers with the accent color.
48-55: LGTM! Well-structured popup component.The popup implementation effectively displays location information with proper semantic HTML structure and conditional rendering for optional descriptions. The styling classes are appropriate for the content hierarchy.
pages/dashboard/index.vue (1)
4-4: LGTM! Excellent integration of map store selection state.The dynamic card styling provides clear visual feedback when locations are selected, and the border colors appropriately highlight the connection between dashboard cards and map markers.
Also applies to: 22-26
pages/dashboard.vue (1)
5-5: LGTM! Clean store integration.stores/map.ts (3)
1-2: LGTM! Well-structured state additions.The new state variables and imports are properly typed and support the enhanced map interaction functionality.
Also applies to: 7-8
10-13: LGTM! Essential method for controlling map animations.The
selectedPointWithoutFlyTomethod provides necessary control over map animations during hover interactions, properly setting the flag before updating the selected point.
19-30: LGTM! Proper bounds calculation with good separation of concerns.The bounds calculation correctly handles multiple map points and applies consistent padding. The scoping of the
boundsvariable outside the reactive effect enables its use in the second effect for coordinated map navigation.
| :icon-color="mapStore.selectedPoint === item.location ? 'text-accent' : undefined" | ||
| @mouseenter="mapStore.selectedPoint = item.location ?? null" | ||
| @mouseleave="mapStore.selectedPoint = null" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use selectedPointWithoutFlyTo for consistent hover behavior.
The dynamic icon color implementation is excellent, but the mouse event handlers should use selectedPointWithoutFlyTo instead of direct assignment to avoid unwanted map animations during hover interactions.
- @mouseenter="mapStore.selectedPoint = item.location ?? null"
- @mouseleave="mapStore.selectedPoint = null"
+ @mouseenter="mapStore.selectedPointWithoutFlyTo(item.location ?? null)"
+ @mouseleave="mapStore.selectedPointWithoutFlyTo(null)"📝 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.
| :icon-color="mapStore.selectedPoint === item.location ? 'text-accent' : undefined" | |
| @mouseenter="mapStore.selectedPoint = item.location ?? null" | |
| @mouseleave="mapStore.selectedPoint = null" | |
| :icon-color="mapStore.selectedPoint === item.location ? 'text-accent' : undefined" | |
| @mouseenter="mapStore.selectedPointWithoutFlyTo(item.location ?? null)" | |
| @mouseleave="mapStore.selectedPointWithoutFlyTo(null)" |
🤖 Prompt for AI Agents
In pages/dashboard.vue around lines 68 to 70, the mouse event handlers currently
assign directly to mapStore.selectedPoint, causing unwanted map animations on
hover. Update the @mouseenter and @mouseleave handlers to assign to
mapStore.selectedPointWithoutFlyTo instead, ensuring consistent hover behavior
without triggering map fly-to animations.
| @mouseenter="mapStore.selectedPoint = location" | ||
| @mouseleave="mapStore.selectedPoint = null" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use selectedPointWithoutFlyTo for consistency with hover interactions.
The direct assignment to mapStore.selectedPoint may trigger unwanted fly-to animations during hover. For consistency with the map marker hover behavior, consider using the selectedPointWithoutFlyTo method.
- @mouseenter="mapStore.selectedPoint = location"
- @mouseleave="mapStore.selectedPoint = null"
+ @mouseenter="mapStore.selectedPointWithoutFlyTo(location)"
+ @mouseleave="mapStore.selectedPointWithoutFlyTo(null)"🤖 Prompt for AI Agents
In pages/dashboard/index.vue around lines 27 to 28, replace the direct
assignment to mapStore.selectedPoint in the mouseenter and mouseleave event
handlers with calls to the selectedPointWithoutFlyTo method to prevent unwanted
fly-to animations during hover and maintain consistency with other map marker
hover interactions.
closes #29
BREAKING CHANGE: MapPoint type properties changed (label -> name, added description)
Summary by CodeRabbit
New Features
Enhancements
Style