refactor: reorganize builtin style elements #775
Conversation
Introduce the "builtin-style-elements.ts" that export all builtin styles to simplify the root "index.ts" file. This simplifies the maintenance and will reduce the impact of the future refactoring on `EdgeStyle` and `Perimeter`. Simplify default builtins registration using a loop as this is already done to register Shapes.
WalkthroughThis change reorganizes the export and import structure for edge marker styles, edge styles, and perimeters within the codebase. It introduces a new module, Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Graph
participant StyleRegistry
participant MarkerShape
App->>Graph: Create instance
Graph->>StyleRegistry: Register default edge styles (via loop)
Graph->>StyleRegistry: Register default perimeters (via loop)
Graph->>MarkerShape: Register default edge markers (via loop)
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
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:
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: 0
🧹 Nitpick comments (3)
packages/core/src/view/style/edge/index.ts (1)
84-84: Appropriately changed to named export.Converting the EdgeStyle class from an implicit non-exported class to an explicitly named export aligns with the PR objective of reorganizing and simplifying style elements.
However, consider refactoring this class in the future:
JavaScript best practices for static utility classes🧰 Tools
🪛 Biome (1.9.4)
[error] 84-155: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/core/src/view/style/builtin-style-elements.ts (1)
2-2: Update the copyright year.The copyright year is set to 2025, which appears to be incorrect. It should reflect the actual year the file was created.
-Copyright 2025-present The maxGraph project Contributors +Copyright 2024-present The maxGraph project Contributorspackages/core/src/view/style/register.ts (1)
69-78: Variable naming inconsistencyThe variable name in the loop should match what it represents for better clarity.
- for (const [name, edgeStyle] of perimetersToRegister) { - StyleRegistry.putValue(name, edgeStyle); + for (const [name, perimeter] of perimetersToRegister) { + StyleRegistry.putValue(name, perimeter);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/core/src/index.ts(1 hunks)packages/core/src/view/Graph.ts(1 hunks)packages/core/src/view/geometry/edge/ConnectorShape.ts(1 hunks)packages/core/src/view/handler/EdgeHandler.ts(1 hunks)packages/core/src/view/style/builtin-style-elements.ts(1 hunks)packages/core/src/view/style/edge/index.ts(2 hunks)packages/core/src/view/style/marker/EdgeMarkerRegistry.ts(1 hunks)packages/core/src/view/style/marker/edge-markers.ts(1 hunks)packages/core/src/view/style/perimeter/index.ts(1 hunks)packages/core/src/view/style/register.ts(4 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/core/src/view/style/edge/index.ts
[error] 84-155: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
🔇 Additional comments (14)
packages/core/src/view/style/marker/EdgeMarkerRegistry.ts (1)
19-22: Import paths updated correctly.The import paths have been updated to reflect the reorganization of style-related modules in the codebase. This ensures proper references after the structural changes.
packages/core/src/view/style/edge/index.ts (1)
19-26: Import paths simplified properly.The import paths have been simplified by removing the
/edge/subdirectory prefix, which is appropriate since the imports are now relative to the current directory.packages/core/src/view/geometry/edge/ConnectorShape.ts (1)
21-21: Import path correctly updated.The import path for MarkerShape has been properly updated to reflect the reorganized directory structure, maintaining the connection between ConnectorShape and the MarkerShape functionality.
packages/core/src/view/Graph.ts (1)
43-43: Import statement properly updated.The import for EdgeStyle has been correctly changed from a default import to a named import, reflecting the changes made in the edge/index.ts file and aligning with the broader refactoring of style-related modules.
packages/core/src/view/handler/EdgeHandler.ts (2)
48-48: Import path updated for EdgeStyle.The import has been changed from a default import to a named import from the new path structure, aligning with the broader style elements reorganization.
601-604: Code functionality preserved with the new import structure.The usage of
EdgeStyle.EntityRelationreference continues to work properly with the updated import, ensuring backward compatibility while benefiting from the reorganized structure.packages/core/src/view/style/marker/edge-markers.ts (1)
19-23: Import paths updated to reflect new directory structure.Import paths have been adjusted to use the correct relative paths after the restructuring of style-related modules.
packages/core/src/view/style/perimeter/index.ts (2)
19-23: Import paths simplified for perimeter functions.The imports now reference the sibling files directly without the redundant
./perimeter/subdirectory prefix, making the imports cleaner and more maintainable.
30-30: Changed Perimeter from default export to named export.Converted
Perimeterto a named export, which aligns with the broader refactoring goal of standardizing how style elements are exported and then consolidated throughbuiltin-style-elements.ts.packages/core/src/view/style/builtin-style-elements.ts (1)
1-29: Well-structured new module for consolidating style elements.This new file successfully serves as a central point for exporting all built-in style elements:
- Exports EdgeStyle from './edge'
- Exports Perimeter from './perimeter'
- Exports all exports from './marker/edge-markers' as the EdgeMarker namespace
The JSDoc comments provide clear documentation about these being built-in edge markers that can be registered in MarkerShape, making the code more discoverable and easier to use.
packages/core/src/index.ts (1)
160-163: LGTM - Clean reorganization of style elementsThe new export statement referring to the consolidated 'builtin-style-elements' module and the updated import path for MarkerShape align well with the PR objectives of simplifying the root index.ts file and making it easier to maintain.
packages/core/src/view/style/register.ts (3)
18-27: LGTM - Well-structured import organizationThe imports have been updated to reflect the new module structure, with a clearer separation of concerns. Using named imports instead of default imports is also a good practice for better clarity and consistency.
40-52: LGTM - Improved registration patternUsing an array of tuples and a loop for registration is a great improvement that makes the code more maintainable and consistent with how Shapes are registered, as mentioned in the PR objectives.
111-124: LGTM - Consistent registration patternThe marker registration follows the same pattern as the other registrations, making the codebase more consistent and maintainable.
|
Introduce the "builtin-style-elements.ts" that export all builtin styles to simplify the root "index.ts" file.
This simplifies the maintenance and will reduce the impact of the future refactoring on
EdgeStyleandPerimeter.Simplify default builtins registration using a loop as this is already done to register Shapes.
Notes
Covers #759
Summary by CodeRabbit
Refactor
Chores