feat(ui): preview improvements + moving back to cesium #849
Conversation
WalkthroughThe pull request overhauls mapping and UI components. It replaces Maplibre and React-Map-GL with Cesium and Resium by updating dependencies and introducing new Cesium components. Several DataTable components now use a boolean “condensed” property instead of a fixed row height. In addition, Editor panels have been refactored with resizable layouts and updated button elements. Workspace components remove explicit row height settings, and key strings in localization files have been cleaned up. Finally, the Vite configuration is updated to integrate the new Cesium plugin. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Editor
participant CesiumViewer
participant CesiumContents
participant CesiumEngine as Cesium Engine
User->>Editor: Load Editor
Editor->>CesiumViewer: Render CesiumViewer component
CesiumViewer->>CesiumContents: Pass isLoaded state
CesiumContents->>CesiumEngine: Configure camera view and render entity
sequenceDiagram
participant User
participant ResizablePanelGroup
participant DataTable
participant CesiumViewer
User->>ResizablePanelGroup: Adjust panel sizes via handle
ResizablePanelGroup->>DataTable: Display DataTable (default 65%)
ResizablePanelGroup->>CesiumViewer: Display CesiumViewer (default 35%)
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
✅ Deploy Preview for reearth-flow ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
ui/src/features/Editor/components/LeftPanel/index.tsx (1)
137-139:⚠️ Potential issueRemove keyboard shortcut for the removed Resources tab.
The keyboard shortcut for the Resources tab should be removed since the tab itself has been removed.
- { - keyBinding: { key: "r", shiftKey: true }, - callback: () => handleTabChange("resources"), - },
🧹 Nitpick comments (9)
ui/src/features/Editor/components/BottomPanel/index.tsx (2)
64-75: Simplify the IconButton container styling.The IconButton container has explicit height and padding that could be simplified using Tailwind's flex utilities.
Consider this more concise styling:
- <div className="flex h-[29px] items-center px-1"> + <div className="flex items-center px-1"> <IconButton className="h-4/5 w-[55px]" icon={windowSize === "min" ? <CornersOut /> : <CornersIn />} tooltipPosition="top" onClick={() => windowSize === "min" ? setWindowSize("max") : setWindowSize("min") } /> </div>The parent
flexanditems-centerclasses will naturally center the IconButton without needing an explicit height.
34-43: Add type safety for window size state.Consider adding a type definition for the window size state to improve maintainability and prevent potential bugs.
Add this type definition:
type WindowSize = "min" | "max"; const { selectedId, windowSize, panelContentOptions, setWindowSize, handleSelection, } = useHooks<WindowSize>({ isOpen, onOpen, });ui/src/features/Editor/components/BottomPanel/hooks.tsx (1)
31-42: Consider adding ARIA labels for accessibility.The changes from
icontobuttonare consistent with the type changes. However, since these are interactive elements, consider adding ARIA labels for better accessibility.Apply this diff to improve accessibility:
{ id: "output-log", - button: <Terminal className="size-[20px]" weight="thin" />, + button: <Terminal className="size-[20px]" weight="thin" aria-label={t("Log")} />, title: t("Log"), }, { id: "visual-preview", - button: <Globe className="size-[20px]" weight="thin" />, + button: <Globe className="size-[20px]" weight="thin" aria-label={t("Preview")} />, title: t("Preview"), },ui/src/features/Editor/components/BottomPanel/components/DataTable/index.tsx (2)
6-7: Address the TODO comment before production deployment.The comment indicates this is placeholder code. We should plan to implement the dynamic type inference or fixed types based on the API implementation.
Would you like me to help implement the dynamic type inference system or create an issue to track this task?
82-83: Consider using overflow-auto instead of overflow-scroll.The change from
overflow-autotooverflow-scrollwill force scrollbars to always be visible, even when not needed. This might not provide the best user experience across different operating systems and browsers.- <div className="m-2 mb-1 box-border flex-1 overflow-scroll"> + <div className="m-2 mb-1 box-border flex-1 overflow-auto">ui/src/components/visualizations/Cesium/index.tsx (1)
20-26: Consider simplifying the isLoaded state logic.You can set
isLoadedonce inside auseEffectwith an empty dependency array to avoid the extra conditional:useEffect(() => { - if (isLoaded) return; setIsLoaded(true); -}, [isLoaded]); +}, []);ui/src/components/visualizations/Cesium/Contents.tsx (2)
5-9: Remove or utilize the commented-outstartingPosition.Dead code may lead to confusion. If you need a different starting position in the future, consider making it a configurable constant or reintroduce it with a clear purpose.
10-15: Consider making the entity position configurable.Hardcoded coordinates can limit flexibility. Storing them in a config file or passing them as props would make the component more reusable.
ui/src/features/Editor/components/OverlayUI/components/Infobar/index.tsx (1)
10-21: Consider reducing hover delay for better UX.The 1-second delay before showing the infobar might feel too long for users. Consider reducing it to 300-500ms for a more responsive experience while still preventing unwanted popups.
- }, 1000); + }, 300);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/yarn.lockis excluded by!**/yarn.lock,!**/*.lock,!**/*.lock
📒 Files selected for processing (26)
ui/package.json(3 hunks)ui/src/components/DataTable/index.tsx(5 hunks)ui/src/components/visualizations/Cesium/Contents.tsx(1 hunks)ui/src/components/visualizations/Cesium/index.tsx(1 hunks)ui/src/components/visualizations/Map/index.tsx(0 hunks)ui/src/components/visualizations/index.ts(1 hunks)ui/src/features/Editor/components/BottomPanel/components/BaseActionButtons/index.tsx(2 hunks)ui/src/features/Editor/components/BottomPanel/components/Contents/index.tsx(2 hunks)ui/src/features/Editor/components/BottomPanel/components/DataTable/index.tsx(1 hunks)ui/src/features/Editor/components/BottomPanel/components/Map/index.tsx(1 hunks)ui/src/features/Editor/components/BottomPanel/hooks.tsx(1 hunks)ui/src/features/Editor/components/BottomPanel/index.tsx(2 hunks)ui/src/features/Editor/components/LeftPanel/index.tsx(3 hunks)ui/src/features/Editor/components/OverlayUI/components/Infobar/index.tsx(1 hunks)ui/src/features/Editor/components/OverlayUI/index.tsx(1 hunks)ui/src/features/Editor/index.tsx(1 hunks)ui/src/features/WorkspaceDeployments/components/DeploymentsDialog.tsx(0 hunks)ui/src/features/WorkspaceDeployments/index.tsx(0 hunks)ui/src/features/WorkspaceJobs/index.tsx(0 hunks)ui/src/features/WorkspaceTriggers/index.tsx(0 hunks)ui/src/lib/i18n/locales/en.json(0 hunks)ui/src/lib/i18n/locales/es.json(0 hunks)ui/src/lib/i18n/locales/fr.json(0 hunks)ui/src/lib/i18n/locales/ja.json(0 hunks)ui/src/lib/i18n/locales/zh.json(0 hunks)ui/vite.config.ts(2 hunks)
💤 Files with no reviewable changes (10)
- ui/src/features/WorkspaceTriggers/index.tsx
- ui/src/features/WorkspaceJobs/index.tsx
- ui/src/lib/i18n/locales/en.json
- ui/src/lib/i18n/locales/zh.json
- ui/src/features/WorkspaceDeployments/index.tsx
- ui/src/lib/i18n/locales/ja.json
- ui/src/components/visualizations/Map/index.tsx
- ui/src/features/WorkspaceDeployments/components/DeploymentsDialog.tsx
- ui/src/lib/i18n/locales/fr.json
- ui/src/lib/i18n/locales/es.json
✅ Files skipped from review due to trivial changes (2)
- ui/src/features/Editor/index.tsx
- ui/src/components/visualizations/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
ui/src/features/Editor/components/BottomPanel/components/Map/index.tsx
[error] 7-7: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (19)
ui/src/features/Editor/components/BottomPanel/index.tsx (2)
47-47: LGTM! Improved border styling organization.The border styling has been appropriately moved from the main container to the inner elements for better visual hierarchy.
81-81: LGTM! Consistent styling with top edge.The added classes maintain visual consistency with the top edge section and ensure proper content contrast.
ui/src/features/Editor/components/BottomPanel/components/BaseActionButtons/index.tsx (3)
19-25: Verify visual impact of CSS changes.The changes include:
- Reduced min-width from 140px to 120px
- Removed default background color when not selected
Please ensure these changes don't cause layout issues or reduce visibility of unselected buttons.
27-27: LGTM!The change from
content.icontocontent.buttonaligns with the type changes.
5-9: Breaking change: Renamedicontobuttonin PanelContent type.This type change affects all consumers of the
PanelContenttype. Ensure that all components using this type have been updated accordingly.Run the following script to verify all usages of the
PanelContenttype:✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Find all files using PanelContent type and verify they use 'button' instead of 'icon'. # Find files importing or using PanelContent rg -l "PanelContent" --type typescript # Check for any remaining usages of 'icon' property rg "icon.*:.*React\.ReactNode" --type typescriptLength of output: 159
Let's re-run the search commands without using the unsupported file type flag and using file globs for
.ts/.tsxfiles:
🏁 Script executed:
#!/bin/bash echo "Searching for PanelContent usage in TS/TSX files..." rg -l "PanelContent" -g "*.ts" -g "*.tsx" echo "Searching for any remaining usages of 'icon' property in TS/TSX files..." rg "icon\s*:.*React\.ReactNode" -g "*.ts" -g "*.tsx"Length of output: 929
PanelContent type update is correctly applied and isolated from unrelated "icon" usages.
- The updated definition in
ui/src/features/Editor/components/BottomPanel/components/BaseActionButtons/index.tsxnow correctly usesbutton?: React.ReactNode;instead oficon.- Searches confirm that files importing and using
PanelContentdo not reference aniconwithin that type.- The remaining occurrences of
icon: React.ReactNodein other parts of the codebase belong to different components/types and are not affected by this breaking change.ui/src/features/Editor/components/BottomPanel/components/DataTable/index.tsx (1)
84-90: Verify prop types for the Table component.The component uses new props (
condensed,selectColumns,showFiltering) without explicit type definitions in this file.Let's verify the prop types are properly defined in the imported Table component:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check prop types in the Table component # Search for Table component prop types ast-grep --pattern 'interface $interface { $$$ condensed?: boolean $$$ }' # Search for Table component implementation ast-grep --pattern 'const Table = $_({ $$$ condensed, selectColumns, showFiltering $$$ })'Length of output: 184
Manual verification needed: Table component prop types
The initial search did not produce any evidence that the new props (
condensed,selectColumns, andshowFiltering) are explicitly typed in the Table component. To further investigate, please run the following additional script to search for any interface or type declarations (likeTableProps) that might include these props:If these searches do not reveal explicit type definitions for the new props, please manually verify whether the Table component relies on implicit TypeScript inference or other patterns to include these props. Adjustments may be necessary to ensure proper type definitions.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching for TableProps interface definitions with new props:" rg -n "interface\s+TableProps" . echo "Searching for 'condensed?: boolean' in the repository:" rg -n "condensed\\?:\\s*boolean" . echo "Searching for 'selectColumns?: boolean' in the repository:" rg -n "selectColumns\\?:\\s*boolean" . echo "Searching for 'showFiltering?: boolean' in the repository:" rg -n "showFiltering\\?:\\s*boolean" . echo "Searching for Table component definition:" rg -n "export (const|function)\s+Table" .Length of output: 1440
Table component prop types are properly defined.
Our investigation confirms that the new props (
condensed,selectColumns, andshowFiltering) are explicitly typed in the Table component’s prop interfaces. For example, inui/src/components/DataTable/index.tsxthe types are declared as follows:
condensed?: boolean;selectColumns?: boolean;showFiltering?: boolean;These definitions ensure that the imported Table component correctly handles the new props used in
ui/src/features/Editor/components/BottomPanel/components/DataTable/index.tsx.ui/src/features/Editor/components/BottomPanel/components/Map/index.tsx (1)
9-10: Simplified rendering approach looks good.By removing the old map modes and directly rendering
<CesiumViewer />, you've streamlined the component.ui/src/components/visualizations/Cesium/index.tsx (2)
6-7: Ensure Cesium’s required credits are displayed to comply with licensing.You’ve set up a
dummyCreditcontainer, which may hide the credits required by Cesium’s license. Verify that licensing requirements for attribution are met and consider showing these credits instead of bypassing them.
28-33: Well-structured use of resium’s Viewer.The approach of passing props and maintaining the loaded state before rendering
<CesiumContents>is clear and concise.ui/src/components/visualizations/Cesium/Contents.tsx (1)
24-35: Camera initialization logic appears correct.The
isLoadedcheck prevents repeated camera resets, ensuring the view is only set once. This implementation is clear and should behave as intended.ui/src/features/Editor/components/OverlayUI/components/Infobar/index.tsx (2)
6-6: LGTM! Type safety improvement.Making
hoveredDetailsrequired improves type safety since the parent component now handles the undefined case.
23-23: LGTM! Better visibility with primary background.The change from
bg-backgroundtobg-primaryimproves the infobar's visibility.ui/src/features/Editor/components/BottomPanel/components/Contents/index.tsx (1)
37-55: LGTM! Improved layout with resizable panels.The implementation of resizable panels enhances user experience by allowing flexible adjustment of the DataTable and Map sections.
ui/vite.config.ts (1)
11-11: LGTM! Cesium plugin integration.The addition of
vite-plugin-cesiumand its integration into the plugins array is correct and aligns with the transition to Cesium.Also applies to: 21-21
ui/src/features/Editor/components/OverlayUI/index.tsx (1)
86-86: LGTM! Safer Infobar rendering.The conditional rendering ensures Infobar only appears when hoveredDetails exists, preventing potential runtime errors.
ui/src/components/DataTable/index.tsx (2)
48-48: Good improvement: Simplified row height control with boolean flag.The change from a numeric
rowHeightto a booleancondensedprop simplifies the API and makes the component more maintainable.Also applies to: 64-64
209-209: Good addition: Cursor feedback for clickable rows.Adding
cursor-pointerprovides better user feedback for clickable rows.ui/package.json (2)
63-63: Good addition: Cesium setup with proper plugin support.The addition of Cesium and its Vite plugin is well-structured:
- Cesium core library for 3D visualization
- Vite plugin for proper asset handling
Also applies to: 137-137
82-82: Verify compatibility of Resium beta version.Using Resium v1.19.0-beta.1 in production could be risky. Please verify:
- Compatibility with Cesium v1.126.0
- Known issues or breaking changes in the beta release
✅ Verification successful
🌐 Web query:
What is the latest stable version of Resium? Are there any known issues with version 1.19.0-beta.1?💡 Result:
The latest stable version of Resium is v1.18.x, while v1.19.0-beta.1 remains an unreleased beta version as of February 2025[10]. Based on available documentation:
Key Details
- Stable releases follow semantic versioning with clear changelogs (e.g., v1.14.x included Cesium v1.85 support and bug fixes)[1][4].
- Beta versions like v1.19.0-beta.x are pre-release builds for testing new features or major changes[10].
Known Issues in Beta Versions
While no specific issues are documented for v1.19.0-beta.x, beta versions generally carry risks such as:
- Unstable API changes
- Untested edge cases
- Potential compatibility breaks with Cesium.js updates
For production use, stick to stable releases unless you need experimental features from betas[10].
Citations:
- 1: https://github.com/reearth/resium/blob/main/CHANGELOG.md
- 2: Moved value gets dropped if reassigned rust-lang/rust#42903
- 3: https://community.native-instruments.com/discussion/4823/official-update-status-native-access-current-version-3-17-0
- 4: https://resium.reearth.io
- 5: https://doc.batch.com/android-v1/sdk-changelog/
- 6: [Bug]: Update 24.0.0 beta2 to beta3, RC3 to final nextcloud/server#31891
- 7: https://www.npmjs.com/package/resium/v/1.0.0-beta.2
- 8: https://www.vdocipher.com/docs/mobile/android/changelog/
- 9: https://docs.mender.io/release-information/release-notes-changelog/mender-server
- 10: https://www.npmjs.com/package/resium
Resium Beta Version Caution: Consider Using the Stable Release
- The latest stable version of Resium is v1.18.x; the beta version v1.19.0-beta.1 is unreleased and carries inherent risks (e.g., unstable API changes or compatibility issues).
- There’s no specific documentation flagging breaking changes with Cesium v1.126.0 for this beta, but beta releases are generally less tested.
- For production usage, it is advisable to either perform thorough compatibility testing with Cesium v1.126.0 or revert to the stable release.
| { key: "2d-map", value: t("2D") }, | ||
| { key: "3d-map", value: t("3D") }, | ||
| ]; | ||
| const Map: React.FC<Props> = ({ className }) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename the component to avoid overshadowing the global "Map" property.
JavaScript provides a built-in global Map. To prevent confusion or potential conflicts, consider renaming your component. For instance:
-const Map: React.FC<Props> = ({ className }) => {
+const FlowMap: React.FC<Props> = ({ className }) => {📝 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.
| const Map: React.FC<Props> = ({ className }) => { | |
| const FlowMap: React.FC<Props> = ({ className }) => { |
🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
| const [visualizationMode, _setVisualizationMode] = | ||
| useState<SupportedVisualizations>("3d-map"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused state setter.
The _setVisualizationMode is never used, suggesting it's no longer needed after the transition to Cesium.
- const [visualizationMode, _setVisualizationMode] =
- useState<SupportedVisualizations>("3d-map");
+ const visualizationMode: SupportedVisualizations = "3d-map";📝 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.
| const [visualizationMode, _setVisualizationMode] = | |
| useState<SupportedVisualizations>("3d-map"); | |
| const visualizationMode: SupportedVisualizations = "3d-map"; |
| @@ -14,7 +8,7 @@ import { useShortcuts } from "@flow/hooks"; | |||
| import { useT } from "@flow/lib/i18n"; | |||
| import type { Node } from "@flow/types"; | |||
|
|
|||
| import { ActionsList, Resources } from "./components"; | |||
| import { ActionsList } from "./components"; | |||
|
|
|||
| type Tab = "navigator" | "actions-list" | "resources"; | |||
There was a problem hiding this comment.
Update type definition to remove 'resources'.
The Tab type still includes 'resources' even though the tab has been removed.
-type Tab = "navigator" | "actions-list" | "resources";
+type Tab = "navigator" | "actions-list";📝 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.
| type Tab = "navigator" | "actions-list" | "resources"; | |
| type Tab = "navigator" | "actions-list"; |
| // { | ||
| // id: "resources", | ||
| // title: "Resources", | ||
| // icon: <HardDrive className="size-5" weight="thin" />, | ||
| // component: <Resources />, | ||
| // }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out code instead of leaving it in the codebase.
The Resources tab implementation should be removed entirely rather than commented out, as commented code can lead to confusion and maintenance issues.
- // {
- // id: "resources",
- // title: "Resources",
- // icon: <HardDrive className="size-5" weight="thin" />,
- // component: <Resources />,
- // },📝 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.
| // { | |
| // id: "resources", | |
| // title: "Resources", | |
| // icon: <HardDrive className="size-5" weight="thin" />, | |
| // component: <Resources />, | |
| // }, |
Overview
What I've done
What I haven't done
How I tested
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit
New Features
Refactor
Chores