feat: Added bookmark section in Earth Globe View#129
Conversation
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
📝 WalkthroughWalkthroughChangesGlobe Bookmark System
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GlobeUser
participant EarthTwin
participant useBookmarks
participant localStorage
participant CesiumCamera
GlobeUser->>EarthTwin: Save current view
EarthTwin->>useBookmarks: Add bookmark with camera state
useBookmarks->>localStorage: Persist bookmark
GlobeUser->>EarthTwin: Open bookmark or shared link
EarthTwin->>useBookmarks: Resolve bookmark
EarthTwin->>CesiumCamera: Fly to saved camera state
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
frontend/src/hooks/useBookmarkStorage.ts (2)
30-60: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMerging the entire stored object over the preset risks masking future preset content updates.
defaultsWithSavedPreferencesspreads the whole stored record over the preset, not just the user-mutable fields (isFavorite,lastOpenedAt,updatedAt). This is harmless today only because default bookmarks can't be edited elsewhere, but the moment a preset'sname/description/altitude/etc. is updated in source, any user who previously favorited/opened that preset will keep seeing the stale cached copy fromlocalStorageforever, since it fully overrides the new default.♻️ Proposed fix — merge only user-mutable fields
const defaultsWithSavedPreferences = DEFAULT_BOOKMARKS.map( - (preset) => ({ - ...preset, - ...storedById.get(preset.id), - isDefault: true, - }) + (preset) => { + const stored = storedById.get(preset.id); + return { + ...preset, + ...(stored && { + isFavorite: stored.isFavorite, + lastOpenedAt: stored.lastOpenedAt, + updatedAt: stored.updatedAt, + }), + isDefault: true, + }; + } );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/useBookmarkStorage.ts` around lines 30 - 60, Update mergeWithDefaultBookmarks so defaultsWithSavedPreferences preserves preset-owned fields from DEFAULT_BOOKMARKS and overlays only the user-mutable fields isFavorite, lastOpenedAt, and updatedAt from the stored record. Keep custom bookmark handling unchanged, ensuring source updates to default name, description, altitude, and other preset metadata are not masked by localStorage.
91-287: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated localStorage persistence logic across every mutator.
addBookmark,updateBookmark,deleteBookmark,toggleFavorite, andmarkAsOpenedeach reimplement the samelocalStorage.setItem+ try/catch block thatsaveBookmarks(lines 99-116) already provides —saveBookmarksis only used byreplaceBookmarks. Beyond the duplication, embedding the persistence side effect directly inside thesetBookmarksfunctional updater violates React's contract that updater functions must be pure (Strict Mode intentionally double-invokes them in development to catch exactly this).♻️ Proposed fix — extract a shared persist helper
+function persistBookmarks(bookmarks: Bookmark[]): void { + try { + localStorage.setItem(STORAGE_KEY, JSON.stringify(bookmarks)); + } catch (error) { + console.error('Failed to save bookmarks to localStorage:', error); + } +} + const addBookmark = useCallback((bookmark: Bookmark) => { setBookmarks((currentBookmarks) => { const updatedBookmarks = [...currentBookmarks, bookmark]; - try { - localStorage.setItem(STORAGE_KEY, JSON.stringify(updatedBookmarks)); - } catch (error) { - console.error('Failed to save bookmark:', error); - } + persistBookmarks(updatedBookmarks); return updatedBookmarks; }); }, []);Apply the same substitution to
updateBookmark,deleteBookmark,toggleFavorite, andmarkAsOpened.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/useBookmarkStorage.ts` around lines 91 - 287, Refactor the mutators addBookmark, updateBookmark, deleteBookmark, toggleFavorite, and markAsOpened to compute updated bookmarks without side effects inside setBookmarks functional updaters, then persist through the shared saveBookmarks helper. Remove each duplicated localStorage.setItem try/catch block while preserving the existing bookmark transformations and error handling.frontend/src/components/EarthTwin.tsx (2)
304-419: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCamera→bookmark field extraction is duplicated.
saveCurrentViewand the create branch ofhandleBookmarkSubmitboth independently derivelatitude/longitude/altitudefromviewer.scene.globe.ellipsoid.cartesianToCartographic(viewer.camera.positionWC)plusheading/pitch/rollfrom the camera. Extracting a smallgetCameraBookmarkFields(viewer)helper would remove the duplication and prevent the two call sites from drifting apart later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/EarthTwin.tsx` around lines 304 - 419, Extract the shared camera-to-bookmark field derivation from saveCurrentView and the create branch of handleBookmarkSubmit into a getCameraBookmarkFields(viewer) helper. Have the helper return the converted latitude, longitude, altitude, heading, pitch, and roll, while preserving the existing null/invalid-camera handling at both call sites.
341-375: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduced-motion + flyTo logic is duplicated between
restoreBookmarkand the mount effect.
restoreBookmarkand the shared-bookmark-on-mount block both independently computeprefersReducedMotionviawindow.matchMediaand callviewer.camera.flyTowith the same destination/orientation/duration shape. Consider extracting a sharedflyToBookmark(viewer, bookmark)helper used by both call sites.Also applies to: 487-510
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/EarthTwin.tsx` around lines 341 - 375, Extract the duplicated bookmark camera transition logic into a shared flyToBookmark(viewer, bookmark) helper, including reduced-motion detection and the existing destination, orientation, and duration values. Update both restoreBookmark and the shared-bookmark-on-mount effect to call this helper, preserving their current validation and state-management behavior.frontend/src/components/GlobeBookmarks/BookmarkModal.tsx (1)
37-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMissing focus trap in the modal dialog.
Tab/Shift+Tab can move focus out of the dialog into the rest of the page while it's open, which conflicts with the PR's stated goal of accessible, keyboard-friendly dialogs. Consider trapping focus within the
<section role="dialog">while open (e.g. cycling Tab between the first/last focusable elements, or using a small focus-trap utility).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/GlobeBookmarks/BookmarkModal.tsx` around lines 37 - 53, Implement a focus trap for the open modal’s <section role="dialog"> so Tab and Shift+Tab cycle only through its focusable elements. Update the modal keyboard-handling logic near handleSubmit, preserve normal dialog interactions, and ensure focus cannot move to the page outside the dialog while it is open.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/EarthTwin.tsx`:
- Around line 823-829: Remove the redundant BookmarkModal render block near the
bookmark dialog section, keeping only the correctly wired instance that supplies
initialBookmark and key. Ensure the standalone “SAVE VIEW” opening flow resets
editingBookmark before setting isBookmarkModalOpen so it always starts a create
operation without a stale edit target.
- Line 8: Update the BookmarkModal import in EarthTwin.tsx to use the exact
filename casing, BookmarkModal, so it resolves correctly on case-sensitive
filesystems.
In `@frontend/src/hooks/useBookmarks.ts`:
- Around line 383-417: Update the bookmark type guard in the imported-bookmark
filter to require Number.isFinite for every numeric field: latitude, longitude,
altitude, heading, pitch, and roll. Keep the existing string checks and object
validation unchanged, aligning with isValidSharedBookmark in bookmarkHelpers.ts
before replaceBookmarks persists the data.
In `@frontend/src/utils/bookmarkHelpers.ts`:
- Around line 168-218: Update isValidSharedBookmark to validate that
bookmark.description is a string before accepting the shared data. Keep the
existing validation for all other fields unchanged so crafted bookmark URL
values cannot pass the type guard with a non-string description.
---
Nitpick comments:
In `@frontend/src/components/EarthTwin.tsx`:
- Around line 304-419: Extract the shared camera-to-bookmark field derivation
from saveCurrentView and the create branch of handleBookmarkSubmit into a
getCameraBookmarkFields(viewer) helper. Have the helper return the converted
latitude, longitude, altitude, heading, pitch, and roll, while preserving the
existing null/invalid-camera handling at both call sites.
- Around line 341-375: Extract the duplicated bookmark camera transition logic
into a shared flyToBookmark(viewer, bookmark) helper, including reduced-motion
detection and the existing destination, orientation, and duration values. Update
both restoreBookmark and the shared-bookmark-on-mount effect to call this
helper, preserving their current validation and state-management behavior.
In `@frontend/src/components/GlobeBookmarks/BookmarkModal.tsx`:
- Around line 37-53: Implement a focus trap for the open modal’s <section
role="dialog"> so Tab and Shift+Tab cycle only through its focusable elements.
Update the modal keyboard-handling logic near handleSubmit, preserve normal
dialog interactions, and ensure focus cannot move to the page outside the dialog
while it is open.
In `@frontend/src/hooks/useBookmarkStorage.ts`:
- Around line 30-60: Update mergeWithDefaultBookmarks so
defaultsWithSavedPreferences preserves preset-owned fields from
DEFAULT_BOOKMARKS and overlays only the user-mutable fields isFavorite,
lastOpenedAt, and updatedAt from the stored record. Keep custom bookmark
handling unchanged, ensuring source updates to default name, description,
altitude, and other preset metadata are not masked by localStorage.
- Around line 91-287: Refactor the mutators addBookmark, updateBookmark,
deleteBookmark, toggleFavorite, and markAsOpened to compute updated bookmarks
without side effects inside setBookmarks functional updaters, then persist
through the shared saveBookmarks helper. Remove each duplicated
localStorage.setItem try/catch block while preserving the existing bookmark
transformations and error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 695d21de-8f0b-4393-9c05-13a01a7bb1d6
📒 Files selected for processing (8)
frontend/src/components/EarthTwin.tsxfrontend/src/components/GlobeBookmarks/BookmarkCard.tsxfrontend/src/components/GlobeBookmarks/BookmarkModal.tsxfrontend/src/components/GlobeBookmarks/BookmarkSearch.tsxfrontend/src/components/GlobeBookmarks/BookmarkSidebar.tsxfrontend/src/hooks/useBookmarkStorage.tsfrontend/src/hooks/useBookmarks.tsfrontend/src/utils/bookmarkHelpers.ts
|
Hi @khedkaravani-rgb, your PR deployment has failed on Vercel. Could you please check the deployment logs and fix the issue? Thanks!
|
|
@krishkhinchi I looked into the issue. I would need access to the deployment url because I cant see the error logs from my end. |

User description
Description
Adds a Globe Bookmark System for Cesium camera views.
This PR includes:
Related Issue
Fixes #122
Checklist
Screenshots / Screen Recordings
Screen.Recording.2026-07-22.223946.1.mp4
Breaking Changes
No breaking changes.
ECSoC26 Submission
ECSoC26-L1– BeginnerECSoC26-L2– IntermediateECSoC26-L3– AdvancedCodeAnt-AI Description
Add saved globe views with search, sharing, and import/export
What Changed
Impact
✅ Faster return to saved globe views✅ Easier sharing of exact camera positions✅ Fewer lost bookmarks after refresh💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Greptile Summary
This PR adds saved camera views to the Cesium globe. The main changes are:
Confidence Score: 2/5
The frontend build and core bookmark dialog flow are broken and need fixes before merging.
frontend/src/components/EarthTwin.tsx and frontend/src/hooks/useBookmarks.ts
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Globe camera] --> B[Create bookmark] B --> C[Bookmark hooks] C --> D[(Local storage)] D --> E[Bookmark sidebar] E --> F[Restore camera] E --> G[Import or export JSON] E --> H[Create share URL] H --> FPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: Added bookmark section in Earth Gl..." | Re-trigger Greptile