-
Notifications
You must be signed in to change notification settings - Fork 1
restore 3d state from url on app load #295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
60e3e7a
to
8957753
Compare
a47f350
to
06518c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors various mapping engine utilities and components to support restoring a 3D camera state from the URL on app load while cleaning up legacy URL state handling across 2D and 3D modes. Key changes include re-exporting new Cesium types and utilities, updating hash routing logic in URL state management, and integrating new hooks (e.g. useCesiumInitialCameraFromSearchParams, useSyncToken, useAppConfig) in several geoportal components.
Reviewed Changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
libraries/mapping/engines/cesium/src/index.ts | Re-exports new Cesium types and utilities. |
libraries/appframeworks/portals/src/lib/utils/routing.ts | Adjusts the hash-param replacement logic and parameter typing. |
envirometrics/wuppertal/floodingmap/src/* | Adds defensive checks against destroyed viewer instances in various hooks and components. |
apps/geoportal/src/app/hooks/* | Introduces new hooks to manage URL state, config fetching, layer management, and keyboard shortcuts. |
apps/geoportal/src/app/components/GeoportalMap/* | Updates map components to use new Cesium hooks and camera limiter options while integrating URL state updates. |
apps/geoportal/src/app/App.tsx | Cleans up imports and integrates the new hooks and constants for config and responsive behavior. |
Files not reviewed (2)
- libraries/collaboration/carma-pecher-collab/pecher-collab-submodule: Language not supported
- libraries/collaboration/carma-wuppertal-collab/wuppertal-collab-submodule: Language not supported
apps/geoportal/src/app/components/GeoportalMap/GeoportalMap.tsx
Outdated
Show resolved
Hide resolved
apps/geoportal/src/app/components/GeoportalMap/GeoportalMap.tsx
Outdated
Show resolved
Hide resolved
aee63ff
to
1d62820
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR aims to enhance the restoration of the 3d state from URL parameters on app load while cleaning up related URL state handling and ensuring a smooth transition between 2d and 3d modes.
- Updates Cesium engine exports and hook types to support new camera handling options.
- Refines URL hash parameter handling across routing and mapping components.
- Introduces additional checks for viewer destruction and improves 3d state restoration in both floodingmap and geoportal apps.
Reviewed Changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
libraries/mapping/engines/cesium/src/index.ts | Added type exports and new camera hash codec exports for enhanced camera state restoration. |
libraries/appframeworks/portals/src/lib/utils/routing.ts | Adjusted the routing utility by making the state property optional and streamlining hash parameter processing. |
envirometrics/wuppertal/floodingmap/src/* | Introduced viewer.isDestroyed() checks and retry logic improvements in terrain initialization and event handling. |
apps/geoportal/src/app/* | Integrated new hooks (useCesiumInitialCameraFromSearchParams, useSyncToken, useManageLayers, useCesiumSearchParams, useKeyboardShortcuts) and updated mapping components to utilize camera limiter options and enhanced URL state handling. |
apps/geoportal/src/app/components/GeoportalMap/* | Updated 3d viewer initialization to conditionally render based on a valid initial camera view and replaced camera options with camera limiter options for consistency. |
Files not reviewed (2)
- libraries/collaboration/carma-pecher-collab/pecher-collab-submodule: Language not supported
- libraries/collaboration/carma-wuppertal-collab/wuppertal-collab-submodule: Language not supported
apps/geoportal/src/app/components/GeoportalMap/GeoportalMap.tsx
Outdated
Show resolved
Hide resolved
…cesium library, cesium specific helper method is still with the library, just it stops making assumptions where those parameters come from, WIP
…ts initialized too often
1d62820
to
cdb7879
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request restores the 3D state from the URL on application load while cleaning up URL state management and related transition logic. Key changes include:
- Exporting new types and helper functions (e.g. InitialCameraView, getClearCesiumCameraParams) in the Cesium engine.
- Updating routing and hash parameter utilities to support 3D/2D mode transitions.
- Refactoring components and hooks in both the Envirometrics and Geoportal apps to use URL-driven state (including camera restoration and terrain provider retry logic).
Reviewed Changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
libraries/mapping/engines/cesium/src/index.ts | Exports new types and utilities for camera restoration and hash parameter management. |
libraries/appframeworks/portals/src/lib/utils/routing.ts | Updates the hash parameter utilities to clean and combine URL parameters. |
envirometrics/wuppertal/floodingmap/src/hooks/useHGKCesiumTerrain.ts | Refactors the terrain initialization using retries and viewer readiness checks. |
apps/geoportal/src/app/components/GeoportalMap/GeoportalMap.tsx | Integrates the new camera restoration hook and renames camera configuration prop. |
apps/geoportal/src/app/App.tsx | Replaces inline URL config handling with dedicated hooks (useAppConfig, useManageLayers, etc.) for improved modularity. |
Files not reviewed (2)
- libraries/collaboration/carma-pecher-collab/pecher-collab-submodule: Language not supported
- libraries/collaboration/carma-wuppertal-collab/wuppertal-collab-submodule: Language not supported
Comments suppressed due to low confidence (3)
apps/geoportal/src/app/App.tsx:171
- [nitpick] Consider renaming or clarifying the mode check variable in the onCesiumSceneChange callback (e.g. use an 'is3dMode' flag) to improve readability and better reflect the 3D state derived from the URL.
replaceHashRoutedHistory(e, "/", "app/hgk");
apps/geoportal/src/app/components/GeoportalMap/GeoportalMap.tsx:558
- [nitpick] Ensure consistent naming for camera configuration props across the application; if transitioning from 'cameraOptions' to 'cameraLimiterOptions', update related documentation and usage in connected components for clarity.
cameraLimiterOptions={CESIUM_CONFIG.camera}
libraries/appframeworks/portals/src/lib/utils/routing.ts:36
- [nitpick] If the commented-out formatting logic is no longer needed, consider removing it to reduce confusion and improve code clarity.
//const formattedHash = combinedHash.replace(/=&/g, "&").replace(/=$/, ""); // remove empty values
Progress looks good, needs some cleanup and adjustments still for transition related issues:
on reload and load
is3d=1
tag now is source of truth for showing 3d mode.adds restoring fov as prerequisite for oblique mode
restoring a non-visible 3d view heading/pitch from a 2d view is currently out of scope