feat(map): enable adding new points via drag and double-click on the map #67
Conversation
This commit introduces new interactive features for adding map points, including drag-and-drop functionality and double-click selection. The changes enhance the user interface by adding visual markers and tooltips to guide users in placing points on the map.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update enhances map interaction by enabling users to add and reposition a point on the map via dragging or double-clicking. The form for adding locations now synchronizes with the map marker, removing manual latitude/longitude entry. Store logic shifts to manage and animate the map based on the added point's state. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MapComponent
participant MapStore
participant AddForm
User->>MapComponent: Double-click or drag marker
MapComponent->>MapStore: Update addedPoint coordinates
MapStore->>MapComponent: Watch addedPoint, fly to new location
MapStore->>AddForm: Sync addedPoint with form fields
AddForm->>User: Display updated coordinates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 (3)
components/app/map.client.vue (3)
16-21: Remove unnecessary semicolon after function closing brace.function updatedAddedPoint(location: LngLat) { if (mapStore.addedPoint) { mapStore.addedPoint.lat = location.lat; mapStore.addedPoint.long = location.lng; } -}; +}
23-28: Remove unnecessary semicolon after function closing brace.function onDoubleClick(mglEvent: MglEvent<"dblclick">) { if (mapStore.addedPoint) { mapStore.addedPoint.lat = mglEvent.event.lngLat.lat; mapStore.addedPoint.long = mglEvent.event.lngLat.lng; } -}; +}
49-55: Consider making the tooltip conditional to improve user experience.The tooltip is always open with the
tooltip-openclass, which might be distracting. Consider showing it only initially or on hover.- <div class="tooltip tooltip-top hover:cursor-pointer tooltip-open" data-tip="Drag to your desired location"> + <div class="tooltip tooltip-top hover:cursor-pointer" :class="{ 'tooltip-open': showAddedPointTooltip }" data-tip="Drag to your desired location">You would need to add a
showAddedPointTooltipref that could be controlled based on user interaction or a timer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/app/map.client.vue(4 hunks)pages/dashboard.vue(1 hunks)pages/dashboard/add.vue(4 hunks)stores/map.ts(2 hunks)
🔇 Additional comments (4)
pages/dashboard.vue (1)
82-83: LGTM! Layout adjustments support the new map interaction feature.The conditional flex direction based on the route and the background color addition are appropriate changes that complement the new interactive map features.
components/app/map.client.vue (1)
70-71: Good simplification of mouse event handlers!The direct assignment to
mapStore.selectedPointis cleaner than using separate handler functions.pages/dashboard/add.vue (2)
131-142: Clear and helpful UI instructions!The instructions for dragging the marker or double-clicking the map, along with the real-time coordinate display, provide excellent user guidance.
61-69: Verify hardcodedidassignment on new map pointsFile: pages/dashboard/add.vue
Lines: 61–69onMounted(() => { mapStore.addedPoint = { long: (CENTER_USA as [number, number])[0], lat: (CENTER_USA as [number, number])[1], description: "", name: "Added Point", id: 1, }; });We weren’t able to locate a
MapPointtype or any downstream logic that relies on.id—absence of evidence isn’t evidence of absence. Please confirm:
- Is the
idfield actually required for new points?- Could a static
id: 1collide with other points (e.g., in persistence or comparisons)?- Should you generate a unique identifier (UUID, nanoid, etc.) or defer ID assignment until the point is saved?
Consider omitting the
idfor purely temporary points or switching to a dynamic ID strategy to avoid clashes.
| function formatNumber(value?: number) { | ||
| if (!value) | ||
| return 0; | ||
| return value.toFixed(5); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve null/undefined handling in formatNumber.
The current implementation returns 0 for falsy values, which might not be appropriate for coordinates.
function formatNumber(value?: number) {
- if (!value)
- return 0;
+ if (value === undefined || value === null)
+ return '';
return value.toFixed(5);
}This change ensures that undefined/null values are displayed as empty strings rather than "0.00000", which is clearer for users.
📝 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.
| function formatNumber(value?: number) { | |
| if (!value) | |
| return 0; | |
| return value.toFixed(5); | |
| } | |
| function formatNumber(value?: number) { | |
| if (value === undefined || value === null) | |
| return ''; | |
| return value.toFixed(5); | |
| } |
🤖 Prompt for AI Agents
In pages/dashboard/add.vue around lines 25 to 29, the formatNumber function
returns 0 for any falsy value, which incorrectly converts null or undefined to
"0.00000". Update the function to explicitly check for null or undefined and
return an empty string in those cases, while still formatting valid numbers to
five decimal places. This will ensure clearer display of missing coordinate
values.
| watch(addedPoint, (newValue, oldValue) => { | ||
| if (newValue && !oldValue) { | ||
| map.map?.flyTo({ | ||
| center: [newValue.long, newValue.lat], | ||
| speed: 0.8, | ||
| zoom: 6, | ||
| }); | ||
| } | ||
| }, { | ||
| immediate: true, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refining the watcher logic for better control over fly-to animations.
The current implementation has a few potential issues:
- With
immediate: true, ifaddedPointis already set when the store initializes, it will trigger an unnecessary fly-to animation - The condition
newValue && !oldValueonly triggers on null-to-value transitions, missing updates when dragging an existing point
Consider this improved implementation:
- watch(addedPoint, (newValue, oldValue) => {
- if (newValue && !oldValue) {
- map.map?.flyTo({
- center: [newValue.long, newValue.lat],
- speed: 0.8,
- zoom: 6,
- });
- }
- }, {
- immediate: true,
- });
+ watch(addedPoint, (newValue, oldValue) => {
+ // Only fly to if we're adding a new point (not just updating coordinates)
+ if (newValue && !oldValue && map.map) {
+ map.map.flyTo({
+ center: [newValue.long, newValue.lat],
+ speed: 0.8,
+ zoom: 6,
+ });
+ }
+ });This removes the immediate option and adds a null check for map.map to prevent potential runtime errors.
📝 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.
| watch(addedPoint, (newValue, oldValue) => { | |
| if (newValue && !oldValue) { | |
| map.map?.flyTo({ | |
| center: [newValue.long, newValue.lat], | |
| speed: 0.8, | |
| zoom: 6, | |
| }); | |
| } | |
| }, { | |
| immediate: true, | |
| }); | |
| watch(addedPoint, (newValue, oldValue) => { | |
| // Only fly to if we're adding a new point (not just updating coordinates) | |
| if (newValue && !oldValue && map.map) { | |
| map.map.flyTo({ | |
| center: [newValue.long, newValue.lat], | |
| speed: 0.8, | |
| zoom: 6, | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In stores/map.ts around lines 28 to 38, the watcher on addedPoint uses
immediate: true causing an unnecessary fly-to animation on initialization and
only triggers on null-to-value changes, missing updates when the point changes.
Remove the immediate option and update the watcher condition to trigger on any
newValue change, adding a null check for map.map before calling flyTo to avoid
runtime errors.
This PR introduces new interactive features for adding map points, including drag-and-drop functionality and double-click selection. The changes enhance the user interface by adding visual markers and tooltips to guide users in placing points on the map.
closes #30
Summary by CodeRabbit
New Features
Improvements