Quality/naming consistency - #313
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #313 +/- ##
==========================================
+ Coverage 91.54% 91.86% +0.32%
==========================================
Files 128 128
Lines 9350 9350
Branches 1966 1966
==========================================
+ Hits 8559 8589 +30
+ Misses 713 685 -28
+ Partials 78 76 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardises naming across the CAD core and its test suite by replacing legacy “item”/“Base*” terminology with clearer “entity”/“*Base” naming, aligning APIs and internal state with the rest of the codebase.
Changes:
- Rename scene/selection APIs from
addItem/selectedItemstoaddEntity/selectedEntitiesacross tools and tests. - Rename core base classes (
BasePolyline,BaseDimension, etc.) toPolylineBase,DimensionBase,LinearDimensionBase,CircularDimensionBaseand update imports/docs. - Update
Blockto useentitiesinstead ofitems, including DXF load/write and container-entity rendering paths.
Reviewed changes
Copilot reviewed 72 out of 72 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/tools/zoom.test.js | Updates scene creation helper from addItem to addEntity. |
| test/tools/trim.test.js | Updates test setup entity creation from addItem to addEntity. |
| test/tools/scale.test.js | Migrates to addEntity and selectedEntities naming in assertions. |
| test/tools/rotate.test.js | Migrates to addEntity in setup. |
| test/tools/purge.test.js | Migrates DesignCore.Scene.addItem to addEntity. |
| test/tools/offset.test.js | Migrates test entity creation to addEntity. |
| test/tools/move.test.js | Migrates to addEntity in setup. |
| test/tools/mirror.test.js | Migrates to addEntity and selectedEntities in preview assertions. |
| test/tools/matchProp.test.js | Migrates DesignCore.Scene.addItem to addEntity. |
| test/tools/fillet.test.js | Migrates test entity creation to addEntity. |
| test/tools/extend.test.js | Migrates test entity creation to addEntity. |
| test/tools/explode.test.js | Updates Insert block population API to addEntity. |
| test/tools/erase.test.js | Migrates test entity creation to addEntity. |
| test/tools/cutclip.test.js | Updates selection assertions from selectedItems to selectedEntities. |
| test/tools/cornerEntity.test.js | Migrates test entity creation to addEntity. |
| test/tools/copyclip.test.js | Updates selection assertions from selectedItems to selectedEntities. |
| test/tools/copybase.test.js | Updates selection assertions from selectedItems to selectedEntities. |
| test/tools/copy.test.js | Migrates test entity creation to addEntity. |
| test/tools/chamferFilletBase.test.js | Migrates test entity creation to addEntity. |
| test/tools/chamfer.test.js | Migrates test entity creation to addEntity. |
| test/test-helpers/hatchGridRenderer.js | Switches hatch boundary construction to PolylineBase. |
| test/tables/layerManager.test.js | Migrates test entity creation to addEntity. |
| test/tables/block.test.js | Updates block API to entities and adds an execution-offset behaviour test. |
| test/properties/propertyManager.test.js | Renames PropertyManager test targets to new *Entity* method names. |
| test/lib/snapping.test.js | Migrates test entity creation to addEntity. |
| test/lib/selectionManager.test.js | Migrates test entity creation to addEntity. |
| test/lib/scene.test.js | Updates Scene API test from addItem to addEntity. |
| test/lib/inputManager.test.js | Migrates test entity creation to addEntity. |
| test/lib/canvas.test.js | Migrates test entity creation to addEntity. |
| test/integration/userInputIntegration.test.js | Updates integration selection injection to selectedEntities. |
| test/integration/exportIntegration.test.js | Switches to PolylineBase and block record entities. |
| test/entities/polylineBase.test.js | Renames tests and constructors to PolylineBase. |
| test/entities/insert.test.js | Updates Insert block API to entities/addEntity. |
| test/entities/hatch.test.js | Switches hatch boundary entities to PolylineBase and selection to selectedEntities. |
| test/dimensions/linearDimensionBase.test.js | Renames BaseLinearDimension to LinearDimensionBase. |
| test/dimensions/dimensionBase.test.js | Renames BaseDimension to DimensionBase and updates block child storage. |
| core/tools/trim.js | Updates docs to reference PolylineBase. |
| core/tools/scale.js | Uses selectionManager.selectedEntities for preview updates. |
| core/tools/rotate.js | Uses selectionManager.selectedEntities for preview updates. |
| core/tools/move.js | Uses selectionManager.selectedEntities for preview updates. |
| core/tools/mirror.js | Uses selectionManager.selectedEntities for preview updates. |
| core/tools/fillet.js | Switches polyline base type checks/imports to PolylineBase. |
| core/tools/extend.js | Updates docs to reference PolylineBase. |
| core/tools/explode.js | Migrates insert block storage from items to entities. |
| core/tools/cutclip.js | Uses selectionManager.selectedEntities for clipboard population. |
| core/tools/cornerEntity.js | Switches polyline base type checks/imports to PolylineBase. |
| core/tools/copyclip.js | Uses selectionManager.selectedEntities for clipboard population. |
| core/tools/copybase.js | Uses selectionManager.selectedEntities for clipboard population. |
| core/tools/copy.js | Uses selectionManager.selectedEntities for preview updates. |
| core/tools/chamferFilletBase.js | Switches polyline base type checks/imports to PolylineBase. |
| core/tools/chamfer.js | Switches polyline base type checks/imports to PolylineBase. |
| core/tables/block.js | Renames internal child storage to entities and updates DXF + manipulation APIs. |
| core/properties/propertyManager.js | Renames public API to *Entity* methods and reloads selectedEntities. |
| core/properties/property.js | Updates property-group comment to PolylineBase. |
| core/lib/selectionManager.js | Renames internal selection clones to selectedEntities. |
| core/lib/scene.js | Renames Scene factory method to addEntity. |
| core/lib/renderers/rendererBase.js | Updates container-entity docs to DimensionBase. |
| core/lib/dxf/dxf.js | Updates DXF entity ingestion API and block child storage to entities. |
| core/lib/canvas.js | Paints selection from selectedEntities and updates container-entity docs. |
| core/entities/polylineBase.js | Renames base polyline class to PolylineBase. |
| core/entities/polyline.js | Updates inheritance/imports to extend PolylineBase. |
| core/entities/insert.js | Returns block.entities for recursive rendering. |
| core/entities/hatch.js | Uses selectedEntities for boundary selection and hover/selection checks. |
| core/dimensions/rotatedDimension.js | Updates base class import to LinearDimensionBase. |
| core/dimensions/radialDimension.js | Updates base class import to CircularDimensionBase. |
| core/dimensions/linearDimensionBase.js | Renames base class to LinearDimensionBase and imports DimensionBase. |
| core/dimensions/dimensionBase.js | Renames base dimension class to DimensionBase and block child storage to entities. |
| core/dimensions/dimension.js | Renames internal workflow state from selectedItems to selectedEntities and switches polyline checks to PolylineBase. |
| core/dimensions/diametricDimension.js | Updates base class import to CircularDimensionBase. |
| core/dimensions/circularDimensionBase.js | Renames base class to CircularDimensionBase and switches polyline checks to PolylineBase. |
| core/dimensions/angularDimension.js | Switches polyline checks/imports to PolylineBase and base class to DimensionBase. |
| core/dimensions/alignedDimension.js | Updates base class import to LinearDimensionBase. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
54
| Object.defineProperty(this, 'selectedEntities', { | ||
| value: [], | ||
| writable: true, | ||
| }); |
Comment on lines
+126
to
+130
| if (copyOfItem.points?.length) { | ||
| const points = copyOfItem.points.map((p) => new Point(p.x, p.y, p.bulge, p.sequence).add(delta)); | ||
| copyOfItem.setProperty('points', points); | ||
| } | ||
| block.items.push(copyOfItem); | ||
| block.entities.push(copyOfItem); |
| // check the insert has a block and | ||
| // check the block has items | ||
| if (insert.block === undefined || insert.block.items.length == 0) { | ||
| if (insert.block === undefined || insert.block.entities.length == 0) { |
Comment on lines
44
to
46
| this.selectionSet = new SelectionSet(); // store a list of selected items indices | ||
| this.selectedItems = []; // store a copy of selected items | ||
| this.selectedEntities = []; // store a copy of selected items | ||
| } |
Comment on lines
30
to
37
| /** | ||
| * Set item properties | ||
| * @param {string} property | ||
| * @param {any} newPropertyValue | ||
| * @param {string} itemType | ||
| */ | ||
| setItemProperties(property, newPropertyValue, itemType='All') { | ||
| setEntityProperties(property, newPropertyValue, itemType='All') { | ||
| const stateChanges = []; |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 85 out of 85 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const segment = selectedEntity.getClosestSegment(selection2.selectedPoint); | ||
|
|
||
| if (segment instanceof Line) { | ||
| // update the selected item to be the polyline arc segment |
Comment on lines
519
to
520
| // distance/referenceLength rather than raw distance: reload the selectedEntitys | ||
| // before and after with a known referenceLength to confirm scaling occurred. |
Comment on lines
+431
to
+432
| // The selectedEntitys copy should have its points mirrored (y negated) | ||
| const previewItem = core.scene.selectionManager.selectedEntities[0]; |
Comment on lines
13
to
16
| const line1 = new Line({ points: [new Point(-102, -102), new Point(201, 202)] }); | ||
| // add to scene (bypass scene.addItem()) | ||
| scene.addItem('Line', line1); | ||
| // add to scene (bypass scene.addEntity()) | ||
| scene.addEntity('Line', line1); | ||
| expect(scene.boundingBox().xMin).toBeCloseTo(-102); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 86 out of 86 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
14
to
16
| // add to scene (bypass scene.addEntity()) | ||
| scene.addEntity('Line', line1); | ||
| expect(scene.boundingBox().xMin).toBeCloseTo(-102); |
Comment on lines
21
to
+23
| const line2 = new Line({ points: [new Point(1001, -2002), new Point(-2001, 2002)] }); | ||
| // add to scene (bypass scene.addItem()) | ||
| scene.addItem('Line', line2); | ||
| // add to scene (bypass scene.addEntity()) | ||
| scene.addEntity('Line', line2); |
dubstar-04
marked this pull request as ready for review
May 3, 2026 11:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.