Fix move/resize/undo geometry flattening (#1090, #1086) + dependency updates - #1091
Merged
Conversation
Undoing a resize that followed a move reverted the shape all the way back to its creation-time position instead of just the resize, for ellipse/star/path (and, per code inspection, any shape driven through this path). Two bugs compounded: the drag-start dummy transform was an identity matrix instead of a translate, so the type check in updateTransformList never recognized it and left a stray extra transform behind after every move; and the mouseup handler's "old transform" for undo was read from dragStartTransforms, which is only populated for move drags and is null during resize/rotate, so undoing a resize always recorded '' as the prior state instead of the move's actual transform. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resizing a shape never called recalculateDimensions(): the mouseup handler treated any transform list with more than one item as a "pre-existing multi-transform" case to consolidate into a single matrix, but a resize always builds a translate/scale/translate list of exactly that shape. As a result every resize left the change as a transform matrix instead of updating the shape's x/y/width/height/ rx/ry/points/d attributes - so the coordinate and size panel inputs never reflected a resize, and the shape appeared scaled (distorting stroke width) rather than recalculated. recalculateDimensions() already has a dedicated branch for exactly this translate/scale/translate shape (and for plain moves), so now non-group elements always try it first; the raw matrix consolidation is only used as a fallback when recalculateDimensions can't handle the transform shape (or for groups, which must keep their transform attribute). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps devDependencies and production deps to their latest available versions, including majors: vite 7->8, jsdom 27->29, i18next 25->26, react/react-dom 19.1->19.2, nyc 17->18, open-cli 8->9, vite-plugin-istanbul 7->9, @babel/preset-react 7->8, plus minor/patch bumps across the rest. Verified lint, the full unit test suite, and the production build all still pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reviewer's GuideFixes geometry flattening for move/resize interactions and undo behavior by changing how drag dummy transforms are created and consolidated, preferring geometry recalculation for elements and matrix consolidation for groups/fallbacks, plus updating dependencies and tests to match new behavior and tooling versions. Sequence diagram for move→resize→undo geometry handlingsequenceDiagram
actor User
participant SvgCanvas as SvgCanvas
participant Element as SVGElement
participant UndoMgr as UndoManager
User->>SvgCanvas: mousedown
SvgCanvas->>SvgCanvas: setStartTransform(Element)
User->>SvgCanvas: drag move (select mode)
SvgCanvas->>Element: updateTransformList(svgRoot, Element, dx, dy)
SvgCanvas->>SvgCanvas: dragStartTransforms.set(Element, transform)
User->>SvgCanvas: mouseup (after move)
SvgCanvas->>SvgCanvas: mouseUpEvent(evt)
SvgCanvas->>SvgCanvas: recalculateDimensions(Element)
SvgCanvas-->>UndoMgr: ChangeElementCommand(...)
User->>SvgCanvas: mousedown
SvgCanvas->>SvgCanvas: setStartTransform(Element)
User->>SvgCanvas: drag resize
SvgCanvas->>Element: updateTransformList(svgRoot, Element, dx, dy)
User->>SvgCanvas: mouseup (after resize)
SvgCanvas->>SvgCanvas: mouseUpEvent(evt)
SvgCanvas->>SvgCanvas: getStartTransform()
SvgCanvas-->>SvgCanvas: oldTransform = dragStartTransforms.has(Element)
SvgCanvas-->>SvgCanvas: else oldTransform = getStartTransform()
SvgCanvas->>SvgCanvas: recalculateDimensions(Element)
SvgCanvas-->>UndoMgr: ChangeElementCommand(...)
User->>UndoMgr: undo
UndoMgr->>Element: apply oldTransform / geometry
Flow diagram for mouseUpEvent transform consolidation and geometry flatteningflowchart TD
A[mouseUpEvent for dragged element] --> B[Get transform list tlist]
B --> C[Determine isGroup and hasDragTranslate]
C --> D{isGroup and hasDragTranslate?}
D -->|yes| E[consolidateTransform svgRoot elem tlist oldTransform]
E --> Z[Add ChangeElementCommand to batchCmd]
D -->|no| F[cmd = recalculateDimensions elem]
F --> G{cmd exists?}
G -->|yes| H[Add cmd to batchCmd]
H --> Y[End]
G -->|no| I{tlist.numberOfItems > 1 and hasDragTranslate?}
I -->|yes| J[consolidateTransform svgRoot elem tlist oldTransform]
J --> Z
I -->|no| K[Read newTransform from elem]
K --> L{newTransform !== oldTransform?}
L -->|yes| M[Add ChangeElementCommand elem transform oldTransform]
M --> Y
L -->|no| Y[End]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 security issues, 2 other issues, and left some high level feedback:
Security issues:
- BlueOak-1.0.0: Open-source license could not be identified (link)
- BlueOak-1.0.0: Open-source license could not be identified (link)
Fixed security issues:
- jspdf (link)
General comments:
- In the jsdom SVGTransformList.replaceItem stub, relying on the non-standard _items array makes the test brittle; consider using existing list methods (e.g., removeItem/insertItem) or a lightweight mock implementation instead of mutating internal state.
- This PR mixes a fairly intricate interaction/undo fix with multiple dependency upgrades; splitting behavioral changes and tooling/dependency bumps into separate PRs would make future bisects and reviews easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the jsdom SVGTransformList.replaceItem stub, relying on the non-standard _items array makes the test brittle; consider using existing list methods (e.g., removeItem/insertItem) or a lightweight mock implementation instead of mutating internal state.
- This PR mixes a fairly intricate interaction/undo fix with multiple dependency upgrades; splitting behavioral changes and tooling/dependency bumps into separate PRs would make future bisects and reviews easier.
## Individual Comments
### Comment 1
<location path="tests/unit/move-resize-undo.test.js" line_range="134-117" />
<code_context>
+ function testResizeOnly (tag, attrs) {
</code_context>
<issue_to_address>
**suggestion (testing):** Add an undo assertion for the resize-only scenario to ensure undo restores the original geometry and transform state.
Because move, resize, and rotate share code paths, it’s important that this helper also cover undo behavior. After computing `afterResize`, call `svgCanvas.undoMgr.undo()`, capture `afterUndo`, and assert `deepEqual(afterUndo, attrs)` plus any expectations on the `transform` attribute (likely empty). This will help catch regressions where resize-only undo behaves differently from move+resize undo.
Suggested implementation:
```javascript
function testResizeOnly (tag, attrs) {
it(`resize-only (no prior move) updates geometry and supports undo for <${tag}>`, function () {
const attrStr = Object.entries(attrs).map(([k, v]) => `${k}="${v}"`).join(' ')
svgCanvas.setSvgString(
`<svg xmlns="http://www.w3.org/2000/svg" width="640" height="480"><${tag} id="shape1" ${attrStr}/></svg>`
)
const elem = document.getElementById('shape1')
svgCanvas.clearSelection()
svgCanvas.addToSelection([elem])
const selector = svgCanvas.selectorManager.requestSelector(elem)
```
To fully implement the undo assertion, you will also need to:
1. Locate where `testResizeOnly` currently computes the geometry after performing the resize, something like:
```js
const afterResize = getGeometry(elem)
```
or equivalent logic that reads `x`/`y`/`width`/`height` (or `cx`/`cy`/`rx`/`ry`) and the `transform` attribute.
2. Immediately after computing `afterResize`, insert:
```js
svgCanvas.undoMgr.undo()
const afterUndo = getGeometry(elem)
assert.deepEqual(
afterUndo,
attrs,
`${tag}: undo(resize-only) should restore original geometry`
)
assert.isFalse(
elem.hasAttribute('transform') && elem.getAttribute('transform'),
`${tag}: undo(resize-only) should leave transform empty`
)
```
- If your helper `getGeometry` already returns a combined object with attributes and transform, adapt the `deepEqual` to compare only the geometry portion and assert separately on the transform.
- If `transform` is represented differently (e.g. `null`, `''`, or omitted), adjust the transform assertion accordingly, e.g.:
```js
assert.isFalse(elem.hasAttribute('transform'))
```
or
```js
assert.strictEqual(elem.getAttribute('transform'), '')
```
3. Ensure that any shared helpers used by the move+resize tests (for reading geometry and transform) are reused here so that resize-only undo is tested along the same code path as move+resize undo.
</issue_to_address>
### Comment 2
<location path="tests/unit/move-resize-undo.test.js" line_range="131-140" />
<code_context>
+ testTag('rect', { x: 10, y: 10, width: 40, height: 30 })
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding coverage for other element types and interactions (e.g., paths/groups and rotate) that share this transform-flattening path.
These tests cover rects and ellipses for move/resize/undo and resize-only, but the updated mouseup logic now applies to all non-group elements and has a separate branch for groups. To better protect against regressions, please add at least one case for another shape type (e.g., `path` or `polygon`) to confirm `recalculateDimensions`/consolidate fallback behavior, and one for a `g` element to verify group transform consolidation. A simple rotate-then-undo test would also exercise the same consolidation vs. `recalculateDimensions` logic from a different interaction mode.
Suggested implementation:
```javascript
testTag('rect', { x: 10, y: 10, width: 40, height: 30 })
testTag('ellipse', { cx: 30, cy: 30, rx: 20, ry: 15 })
// Non-rect/ellipse shape to cover recalculateDimensions/consolidate fallback
testTag('polygon', { points: '10,10 50,10 50,40 10,40' })
function testResizeOnly (tag, attrs) {
```
Based on the test comment and the broader change to the mouseup logic, you should also:
1. Add a `testTag('path', ...)` or a separate test case using a `<path>` element if your current `testTag` helper supports paths, to ensure transform flattening is exercised for another non-rect/ellipse type.
2. Introduce a new test (or tests) for a `<g>` element that:
- Creates a group (`<g id="group1">`) with at least one child (e.g., a `<rect>`).
- Applies a rotate interaction through the same utilities used in this file (e.g., whatever helper currently performs rotate via the selector/mouse interaction).
- Triggers undo and asserts that group transforms are properly consolidated (e.g., transform attribute restored/flattened as expected).
3. If this file already contains helpers (e.g., `performRotate`, `simulateRotate`, or similar), use them in the new `g` test to keep things consistent; otherwise, follow the existing pattern used in the move/resize tests to simulate mouse events for rotate + undo.
You’ll need to integrate these new tests where appropriate in the file’s existing `describe`/`context` structure, mirroring how the current move/resize/undo tests are organized.
</issue_to_address>
### Comment 3
<location path="package-lock.json" line_range="7419-7427" />
<code_context>
</code_context>
<issue_to_address>
**security (license/minipass):** BlueOak-1.0.0: Open-source license could not be identified
The obligations of the `BlueOak-1.0.0` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 4
<location path="package-lock.json" line_range="9545-9559" />
<code_context>
</code_context>
<issue_to_address>
**security (license/spawn-wrap):** BlueOak-1.0.0: Open-source license could not be identified
The obligations of the `BlueOak-1.0.0` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // The resize must flatten into the shape's real geometry attributes | ||
| // (issue #1086), not just leave a scale matrix on the element. | ||
| assert.notDeepEqual(afterResize, afterMove, `${tag}: resize should update geometry attrs, not just leave a transform`) | ||
| assert.ok(!elem.getAttribute('transform'), `${tag}: resize should not leave a leftover transform attribute`) |
There was a problem hiding this comment.
suggestion (testing): Add an undo assertion for the resize-only scenario to ensure undo restores the original geometry and transform state.
Because move, resize, and rotate share code paths, it’s important that this helper also cover undo behavior. After computing afterResize, call svgCanvas.undoMgr.undo(), capture afterUndo, and assert deepEqual(afterUndo, attrs) plus any expectations on the transform attribute (likely empty). This will help catch regressions where resize-only undo behaves differently from move+resize undo.
Suggested implementation:
function testResizeOnly (tag, attrs) {
it(`resize-only (no prior move) updates geometry and supports undo for <${tag}>`, function () {
const attrStr = Object.entries(attrs).map(([k, v]) => `${k}="${v}"`).join(' ')
svgCanvas.setSvgString(
`<svg xmlns="http://www.w3.org/2000/svg" width="640" height="480"><${tag} id="shape1" ${attrStr}/></svg>`
)
const elem = document.getElementById('shape1')
svgCanvas.clearSelection()
svgCanvas.addToSelection([elem])
const selector = svgCanvas.selectorManager.requestSelector(elem)To fully implement the undo assertion, you will also need to:
-
Locate where
testResizeOnlycurrently computes the geometry after performing the resize, something like:const afterResize = getGeometry(elem)
or equivalent logic that reads
x/y/width/height(orcx/cy/rx/ry) and thetransformattribute. -
Immediately after computing
afterResize, insert:svgCanvas.undoMgr.undo() const afterUndo = getGeometry(elem) assert.deepEqual( afterUndo, attrs, `${tag}: undo(resize-only) should restore original geometry` ) assert.isFalse( elem.hasAttribute('transform') && elem.getAttribute('transform'), `${tag}: undo(resize-only) should leave transform empty` )
- If your helper
getGeometryalready returns a combined object with attributes and transform, adapt thedeepEqualto compare only the geometry portion and assert separately on the transform. - If
transformis represented differently (e.g.null,'', or omitted), adjust the transform assertion accordingly, e.g.:orassert.isFalse(elem.hasAttribute('transform'))
assert.strictEqual(elem.getAttribute('transform'), '')
- If your helper
-
Ensure that any shared helpers used by the move+resize tests (for reading geometry and transform) are reused here so that resize-only undo is tested along the same code path as move+resize undo.
Comment on lines
+131
to
+140
| testTag('rect', { x: 10, y: 10, width: 40, height: 30 }) | ||
| testTag('ellipse', { cx: 30, cy: 30, rx: 20, ry: 15 }) | ||
|
|
||
| function testResizeOnly (tag, attrs) { | ||
| it(`resize-only (no prior move) updates geometry for <${tag}>`, function () { | ||
| const attrStr = Object.entries(attrs).map(([k, v]) => `${k}="${v}"`).join(' ') | ||
| svgCanvas.setSvgString( | ||
| `<svg xmlns="http://www.w3.org/2000/svg" width="640" height="480"><${tag} id="shape1" ${attrStr}/></svg>` | ||
| ) | ||
| const elem = document.getElementById('shape1') |
There was a problem hiding this comment.
suggestion (testing): Consider adding coverage for other element types and interactions (e.g., paths/groups and rotate) that share this transform-flattening path.
These tests cover rects and ellipses for move/resize/undo and resize-only, but the updated mouseup logic now applies to all non-group elements and has a separate branch for groups. To better protect against regressions, please add at least one case for another shape type (e.g., path or polygon) to confirm recalculateDimensions/consolidate fallback behavior, and one for a g element to verify group transform consolidation. A simple rotate-then-undo test would also exercise the same consolidation vs. recalculateDimensions logic from a different interaction mode.
Suggested implementation:
testTag('rect', { x: 10, y: 10, width: 40, height: 30 })
testTag('ellipse', { cx: 30, cy: 30, rx: 20, ry: 15 })
// Non-rect/ellipse shape to cover recalculateDimensions/consolidate fallback
testTag('polygon', { points: '10,10 50,10 50,40 10,40' })
function testResizeOnly (tag, attrs) {Based on the test comment and the broader change to the mouseup logic, you should also:
- Add a
testTag('path', ...)or a separate test case using a<path>element if your currenttestTaghelper supports paths, to ensure transform flattening is exercised for another non-rect/ellipse type. - Introduce a new test (or tests) for a
<g>element that:- Creates a group (
<g id="group1">) with at least one child (e.g., a<rect>). - Applies a rotate interaction through the same utilities used in this file (e.g., whatever helper currently performs rotate via the selector/mouse interaction).
- Triggers undo and asserts that group transforms are properly consolidated (e.g., transform attribute restored/flattened as expected).
- Creates a group (
- If this file already contains helpers (e.g.,
performRotate,simulateRotate, or similar), use them in the newgtest to keep things consistent; otherwise, follow the existing pattern used in the move/resize tests to simulate mouse events for rotate + undo.
You’ll need to integrate these new tests where appropriate in the file’s existing describe/context structure, mirroring how the current move/resize/undo tests are organized.
Comment on lines
7419
to
7427
| "node_modules/minipass": { | ||
| "version": "7.1.2", | ||
| "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.2.tgz", | ||
| "integrity": "sha512-qOOzS1cBTWYF4BH8fVePDBOO9iptMnGUEZwNc/cMWnTV2nVLZ7VoNWEPHkYczZA0pdoA7dl6e7FL659nX9S2aw==", | ||
| "version": "7.1.3", | ||
| "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.3.tgz", | ||
| "integrity": "sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==", | ||
| "dev": true, | ||
| "license": "ISC", | ||
| "license": "BlueOak-1.0.0", | ||
| "engines": { | ||
| "node": ">=16 || 14 >=14.17" | ||
| } |
There was a problem hiding this comment.
security (license/minipass): BlueOak-1.0.0: Open-source license could not be identified
The obligations of the BlueOak-1.0.0 license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
Comment on lines
9545
to
9559
| "node_modules/spawn-wrap": { | ||
| "version": "2.0.0", | ||
| "resolved": "https://registry.npmjs.org/spawn-wrap/-/spawn-wrap-2.0.0.tgz", | ||
| "integrity": "sha512-EeajNjfN9zMnULLwhZZQU3GWBoFNkbngTUPfaawT4RkMiviTxcX0qfhVbGey39mfctfDHkWtuecgQ8NJcyQWHg==", | ||
| "version": "3.0.0", | ||
| "resolved": "https://registry.npmjs.org/spawn-wrap/-/spawn-wrap-3.0.0.tgz", | ||
| "integrity": "sha512-z+s5vv4KzFPJVddGab0xX2n7kQPGMdNUX5l9T8EJqsXdKTWpcxmAqWHpsgHEXoC1taGBCc7b79bi62M5kdbrxQ==", | ||
| "dev": true, | ||
| "license": "ISC", | ||
| "license": "BlueOak-1.0.0", | ||
| "dependencies": { | ||
| "cross-spawn": "^7.0.6", | ||
| "foreground-child": "^2.0.0", | ||
| "is-windows": "^1.0.2", | ||
| "make-dir": "^3.0.0", | ||
| "rimraf": "^3.0.0", | ||
| "rimraf": "^6.1.3", | ||
| "signal-exit": "^3.0.2", | ||
| "which": "^2.0.1" | ||
| }, |
There was a problem hiding this comment.
security (license/spawn-wrap): BlueOak-1.0.0: Open-source license could not be identified
The obligations of the BlueOak-1.0.0 license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
expect.poll() no longer supports chaining a .resolves/.rejects matcher in the updated @playwright/test version - its callback result is already awaited directly. Drop the redundant .resolves calls so the e2e suite passes again after the dependency bump. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Reimplement the jsdom SVGTransformList#replaceItem stub in terms of the list's public removeItem/insertItemBefore API instead of mutating its internal _items array directly. - Add an undo assertion to the resize-only case so it also verifies undo restores the original geometry and leaves no transform. - Add path element coverage alongside rect/ellipse for the move-then-resize-then-undo and resize-only flows. - Add a group move test covering the isGroup consolidation branch, which recalculateDimensions() intentionally skips. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves all 14 flagged vulnerabilities (1 low, 5 moderate, 7 high, 1 critical) in transitive dev-tooling dependencies - eslint plugin chains, remark/markdown-it, nyc/istanbul, npm-run-all - by updating package-lock.json resolutions. No direct dependency versions changed; npm audit reports 0 vulnerabilities afterward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
Addressed the Sourcery review feedback:
All changes verified with |
Collaborator
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 security issues, and left some high level feedback:
Security issues:
- BlueOak-1.0.0: Open-source license could not be identified (link)
- BlueOak-1.0.0: Open-source license could not be identified (link)
Fixed security issues:
- jspdf (link)
General comments:
- The mouseup transform logic still relies on magic numeric SVGTransform
typevalues (e.g.,2for translate,1for matrix); consider centralizing these as named constants or using the corresponding DOM constants to improve readability and reduce the risk of incorrect values. - The new
consolidatehelper insidemouseUpEventcloses over local variables; extracting it to a small standalone function that takeselem,tlist, andoldTransformas arguments would make the control flow clearer and avoid subtle capture issues if this block evolves.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The mouseup transform logic still relies on magic numeric SVGTransform `type` values (e.g., `2` for translate, `1` for matrix); consider centralizing these as named constants or using the corresponding DOM constants to improve readability and reduce the risk of incorrect values.
- The new `consolidate` helper inside `mouseUpEvent` closes over local variables; extracting it to a small standalone function that takes `elem`, `tlist`, and `oldTransform` as arguments would make the control flow clearer and avoid subtle capture issues if this block evolves.
## Individual Comments
### Comment 1
<location path="package-lock.json" line_range="7370-7378" />
<code_context>
</code_context>
<issue_to_address>
**security (license/minipass):** BlueOak-1.0.0: Open-source license could not be identified
The obligations of the `BlueOak-1.0.0` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 2
<location path="package-lock.json" line_range="9485-9499" />
<code_context>
</code_context>
<issue_to_address>
**security (license/spawn-wrap):** BlueOak-1.0.0: Open-source license could not be identified
The obligations of the `BlueOak-1.0.0` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
7370
to
7378
| "node_modules/minipass": { | ||
| "version": "7.1.2", | ||
| "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.2.tgz", | ||
| "integrity": "sha512-qOOzS1cBTWYF4BH8fVePDBOO9iptMnGUEZwNc/cMWnTV2nVLZ7VoNWEPHkYczZA0pdoA7dl6e7FL659nX9S2aw==", | ||
| "version": "7.1.3", | ||
| "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.3.tgz", | ||
| "integrity": "sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==", | ||
| "dev": true, | ||
| "license": "ISC", | ||
| "license": "BlueOak-1.0.0", | ||
| "engines": { | ||
| "node": ">=16 || 14 >=14.17" | ||
| } |
There was a problem hiding this comment.
security (license/minipass): BlueOak-1.0.0: Open-source license could not be identified
The obligations of the BlueOak-1.0.0 license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
Comment on lines
9485
to
9499
| "node_modules/spawn-wrap": { | ||
| "version": "2.0.0", | ||
| "resolved": "https://registry.npmjs.org/spawn-wrap/-/spawn-wrap-2.0.0.tgz", | ||
| "integrity": "sha512-EeajNjfN9zMnULLwhZZQU3GWBoFNkbngTUPfaawT4RkMiviTxcX0qfhVbGey39mfctfDHkWtuecgQ8NJcyQWHg==", | ||
| "version": "3.0.0", | ||
| "resolved": "https://registry.npmjs.org/spawn-wrap/-/spawn-wrap-3.0.0.tgz", | ||
| "integrity": "sha512-z+s5vv4KzFPJVddGab0xX2n7kQPGMdNUX5l9T8EJqsXdKTWpcxmAqWHpsgHEXoC1taGBCc7b79bi62M5kdbrxQ==", | ||
| "dev": true, | ||
| "license": "ISC", | ||
| "license": "BlueOak-1.0.0", | ||
| "dependencies": { | ||
| "cross-spawn": "^7.0.6", | ||
| "foreground-child": "^2.0.0", | ||
| "is-windows": "^1.0.2", | ||
| "make-dir": "^3.0.0", | ||
| "rimraf": "^3.0.0", | ||
| "rimraf": "^6.1.3", | ||
| "signal-exit": "^3.0.2", | ||
| "which": "^2.0.1" | ||
| }, |
There was a problem hiding this comment.
security (license/spawn-wrap): BlueOak-1.0.0: Open-source license could not be identified
The obligations of the BlueOak-1.0.0 license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
- Replace magic SVGTransform.type numbers (2, 1) with the named SVG_TRANSFORM_TRANSLATE/SVG_TRANSFORM_MATRIX constants, matching the convention already used in math.js and recalculate.js. - Extract the inline consolidate() closure into a standalone consolidateTransform(svgRoot, elem, tlist, oldTransform) function that takes its dependencies as explicit parameters instead of capturing them from the enclosing mouseUpEvent scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 security issues
Security issues:
- BlueOak-1.0.0: Open-source license could not be identified (link)
- BlueOak-1.0.0: Open-source license could not be identified (link)
Fixed security issues:
- jspdf (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="package-lock.json" line_range="7370-7378" />
<code_context>
</code_context>
<issue_to_address>
**security (license/minipass):** BlueOak-1.0.0: Open-source license could not be identified
The obligations of the `BlueOak-1.0.0` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>
### Comment 2
<location path="package-lock.json" line_range="9485-9499" />
<code_context>
</code_context>
<issue_to_address>
**security (license/spawn-wrap):** BlueOak-1.0.0: Open-source license could not be identified
The obligations of the `BlueOak-1.0.0` license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
*Source: trivy*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
7370
to
7378
| "node_modules/minipass": { | ||
| "version": "7.1.2", | ||
| "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.2.tgz", | ||
| "integrity": "sha512-qOOzS1cBTWYF4BH8fVePDBOO9iptMnGUEZwNc/cMWnTV2nVLZ7VoNWEPHkYczZA0pdoA7dl6e7FL659nX9S2aw==", | ||
| "version": "7.1.3", | ||
| "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.3.tgz", | ||
| "integrity": "sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==", | ||
| "dev": true, | ||
| "license": "ISC", | ||
| "license": "BlueOak-1.0.0", | ||
| "engines": { | ||
| "node": ">=16 || 14 >=14.17" | ||
| } |
There was a problem hiding this comment.
security (license/minipass): BlueOak-1.0.0: Open-source license could not be identified
The obligations of the BlueOak-1.0.0 license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
Comment on lines
9485
to
9499
| "node_modules/spawn-wrap": { | ||
| "version": "2.0.0", | ||
| "resolved": "https://registry.npmjs.org/spawn-wrap/-/spawn-wrap-2.0.0.tgz", | ||
| "integrity": "sha512-EeajNjfN9zMnULLwhZZQU3GWBoFNkbngTUPfaawT4RkMiviTxcX0qfhVbGey39mfctfDHkWtuecgQ8NJcyQWHg==", | ||
| "version": "3.0.0", | ||
| "resolved": "https://registry.npmjs.org/spawn-wrap/-/spawn-wrap-3.0.0.tgz", | ||
| "integrity": "sha512-z+s5vv4KzFPJVddGab0xX2n7kQPGMdNUX5l9T8EJqsXdKTWpcxmAqWHpsgHEXoC1taGBCc7b79bi62M5kdbrxQ==", | ||
| "dev": true, | ||
| "license": "ISC", | ||
| "license": "BlueOak-1.0.0", | ||
| "dependencies": { | ||
| "cross-spawn": "^7.0.6", | ||
| "foreground-child": "^2.0.0", | ||
| "is-windows": "^1.0.2", | ||
| "make-dir": "^3.0.0", | ||
| "rimraf": "^3.0.0", | ||
| "rimraf": "^6.1.3", | ||
| "signal-exit": "^3.0.2", | ||
| "which": "^2.0.1" | ||
| }, |
There was a problem hiding this comment.
security (license/spawn-wrap): BlueOak-1.0.0: Open-source license could not be identified
The obligations of the BlueOak-1.0.0 license for this code could not be determined automatically. Unknown licenses may carry obligations or restrictions and should be reviewed manually to ensure compliance
Source: trivy
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
1 participant
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.
Summary
x/y/width/height/rx/ry/etc.), so the coordinate/size panel inputs never reflected a resize and shapes appeared scaled (distorting stroke width) instead of recalculated.@babel/preset-react7→8).Root cause (#1090 / #1086)
Both issues stem from the same defect in
packages/svgcanvas/core/event.js's mouseup handler:recalculateDimensions()(which already has a dedicated branch for this exact shape) was never actually called for a resize, and the "old transform" recorded for undo was read from a map that's only populated during move drags, so undoing a resize always recorded''as the prior state instead of the move's real transform.Fix: the drag-start dummy is now a real
translate(0,0)so it's correctly recognized and replaced; and non-group elements now always tryrecalculateDimensions()first (falling back to raw matrix consolidation only for groups or shapes it doesn't recognize), using the transform captured at mousedown as the true "before" state for undo regardless of interaction mode.Test plan
npm run lintnpx vitest run tests/unit— 562 tests pass, including new regression testtests/unit/move-resize-undo.test.jscovering move→resize→undo and resize-only flattening forrectandellipsenpm run build— production build succeeds🤖 Generated with Claude Code
Summary by Sourcery
Fix move/resize/undo handling so drag transforms are correctly flattened into element geometry, ensuring accurate undo behavior and geometry attributes, and update key workspace dependencies.
Bug Fixes:
Enhancements:
Build:
Tests: