-
Notifications
You must be signed in to change notification settings - Fork 5
Implement item list that supports nested objects #2063
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
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 the SidebarTree component into a new SidebarTreeList component that supports both nested and flat lists while also introducing GEANT4-related features. Key changes include refactoring command button properties to differentiate between flat and nested figures, adding GEANT4 simulator support in request types and example configurations, and removing the old SidebarTree component in favor of the new SidebarTreeList component.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/util/Ui/CommandButtonProps.ts | Updated command button tuples with distinct keys for nested and flat figures. |
| src/types/RequestTypes.ts | Added GEANT4 options to SimulatorType and updated SimulatorNames map. |
| src/examples/exampleMap.json | Included a new GEANT4 example configuration. |
| src/ThreeEditor/js/commands/MoveObjectInTreeCommand.ts | Introduced a new command to handle moving objects within the tree. |
| src/ThreeEditor/js/commands/ChangeObjectOrderCommand.ts | Removed redundant ChangeObjectOrderCommand. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/useGenerateTreeData.ts | Added a hook to generate tree data from simulation scene objects. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/contextActions.tsx | Added various context menu actions for tree list items. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeListItem.tsx | Updated tree item layout and actions; adjusted indentation and component usage. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeList.tsx | Introduced the new tree list component that replaces the old SidebarTree. |
| src/ThreeEditor/components/Sidebar/EditorSidebar.tsx | Updated imports and integration for the new SidebarTreeList component. |
| src/ThreeEditor/Simulation/Scoring/ScoringOutputTypes.ts | Added GEANT4-specific scoring options configuration. |
| src/ThreeEditor/Simulation/Figures/FigureManager.ts | Changed figureLoader export for broader usage in figure deserialization. |
| src/ThreeEditor/Simulation/Base/SimulationMesh.ts | Updated serialization/deserialization to include children via figureLoader. |
| src/App.tsx | Enabled additional CSS variables via the theme configuration. |
| public/examples/ex8.json | Added a new example JSON configuration for GEANT4 projects. |
Comments suppressed due to low confidence (2)
src/util/Ui/CommandButtonProps.ts:45
- [nitpick] The new variable names 'cfgFiguresTuple' and 'nestedFiguresTuple' differ from the previous 'figuresTuple'. Consider using more descriptive names that clearly indicate their purpose (e.g. 'flatFiguresTuple' vs. 'nestedFiguresTuple') to improve clarity.
const cfgFiguresTuple: CommandButtonTuple[] = [
src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeListItem.tsx:119
- [nitpick] The indentation factor has been reduced from 2.5 to 1.5; please verify that this change aligns with the intended UI design and provides a clear visual hierarchy.
marginLeft: ({ spacing }) => spacing(depth * 1.5),
|
@kmichalikk some conflicts appeared |
|
@kmichalikk #2039 was merged just now, so please fix the conflicts |
|
I fixed them but the PR is not ready yet, I'm currently trying to refactor the component to be more universal |
78cdbfd to
30795c3
Compare
|
For now, I removed the special case for ScoringInput which turned out to be easy the rest works okay, some elements could be abstracted further but we'll do it only when it's needed |
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 implements a rewrite of the SidebarTree component to support nested structures and replaces the flat list version with a new, more flexible SidebarTreeList. Key changes include:
- Updating command button properties and manager types to include new categories for nested figures.
- Introducing a new MoveObjectInTreeCommand while removing the deprecated ChangeObjectOrderCommand.
- Refactoring the sidebar tree components (SidebarTreeList, SidebarTreeListItem, etc.) and updating styling and editor sidebar integrations.
- Adjusting figure management to export figureLoader for proper deserialization of nested SimulationMesh children.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/Ui/CommandButtonProps.ts | Updated tuple variables and ManagerName type to support nested and CFG figures. |
| src/ThreeEditor/js/commands/MoveObjectInTreeCommand.ts | Added new command for moving objects within the nested tree. |
| src/ThreeEditor/js/commands/ChangeObjectOrderCommand.ts | Removed obsolete command handling. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/useGenerateTreeData.ts | New hook for generating tree data recursively. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/contextActions.tsx | New context actions for sidebar item interactions. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeListItem.tsx | Refactored tree item component with updated styling and actions. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeList.tsx | Introduced new configurable tree list component. |
| src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeList.style.css | New styles for the sidebar tree list. |
| src/ThreeEditor/components/Sidebar/EditorSidebar.tsx | Updated to integrate the new SidebarTreeList component. |
| src/ThreeEditor/Simulation/Figures/FigureManager.ts | Figure loader now exported for use in SimulationMesh deserialization. |
| src/ThreeEditor/Simulation/Base/SimulationMesh.ts | Enhanced deserialization by recursively processing child meshes. |
| src/App.tsx | Minor styling update by enabling CSS variables. |
Comments suppressed due to low confidence (2)
src/util/Ui/CommandButtonProps.ts:45
- [nitpick] Consider clarifying the naming for 'cfgFiguresTuple' to clearly differentiate it from the nested figures tuple; a more descriptive name would improve readability.
const cfgFiguresTuple: CommandButtonTuple[] = [
src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeListItem.tsx:119
- [nitpick] Ensure that the updated marginLeft calculation (depth * 1.5) consistently matches the overall design system for proper indentation in the tree view.
'marginLeft': ({ spacing }) => spacing(depth * 1.5),
This PR rewrites SidebarTree with support for nested structures. The new component is intended to work with nested and flat lists and fully replace its predecessor.
This pull request introduces several changes to enhance the functionality of the simulation editor, improve the sidebar component, and refactor code for better maintainability. The highlights include adding support for child elements in
SimulationMesh, replacing theSidebarTreecomponent with the newSidebarTreeList, and introducing nesting and sorting capabilities in the sidebar. Additionally, unused imports and components were removed, and some styles were updated.Simulation Editor Enhancements:
SimulationMeshby introducing thechildrenproperty in theSimulationMeshJSONtype and updating thetoSerializedandfromSerializedmethods to handle nested structures. (src/ThreeEditor/Simulation/Base/SimulationMesh.ts, [1] [2]figureLoaderfunction fromFigureManagerto handle deserialization of nested child elements. (src/ThreeEditor/Simulation/Figures/FigureManager.ts, src/ThreeEditor/Simulation/Figures/FigureManager.tsL20-R20)Sidebar Component Refactor:
SidebarTreecomponent withSidebarTreeList, which introduces improved handling of drag-and-drop functionality, nesting, and sorting capabilities. (src/ThreeEditor/components/Sidebar/EditorSidebar.tsx, [1] [2] [3] [4]SidebarTreecomponent and its associated styles, consolidating functionality intoSidebarTreeList. (src/ThreeEditor/components/Sidebar/SidebarTree/SidebarTree.tsx, [1];src/ThreeEditor/components/Sidebar/SidebarTree/SidebarTree.style.css, [2]SidebarTreeListto support drop targets and improved visual hierarchy. (src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeList.style.css, src/ThreeEditor/components/Sidebar/SidebarTreeList/SidebarTreeList.style.cssR1-R19)UI Improvements:
InputLabelandSelectcomponents in the sidebar to use Material-UI'ssxprop for dynamic styling based on the theme palette. (src/ThreeEditor/components/Sidebar/EditorSidebar.tsx, src/ThreeEditor/components/Sidebar/EditorSidebar.tsxL127-R129)Code Cleanup:
MenuandSelectPropertyField, from the sidebar codebase. (src/ThreeEditor/components/Sidebar/EditorSidebar.tsx, [1] [2]