Skip to content

Commit c02bb6d

Browse files
author
DavidQ
committed
Fix Object Vector V2 center markers and multi-shape ordering actions - PR_26133_100-object-vector-shape-selection-fixes
1 parent 62ea0fe commit c02bb6d

4 files changed

Lines changed: 195 additions & 42 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# PR_26133_100 Object Vector Shape Selection Fixes
2+
3+
## Scope
4+
- Read docs/dev/PROJECT_INSTRUCTIONS.md before changes.
5+
- Preserved the existing Workspace manifest/schema contract.
6+
- Limited implementation to Object Vector Studio V2 marker rendering and selected-shape z-order behavior, plus matching workspace-v2 coverage.
7+
- PR_26133_099 delta reference was requested, but no matching tmp/PR_26133_099-storage-preview-workspace-followup_delta.zip was present; current integrated repo state was used as the prior baseline.
8+
9+
## Changes
10+
- Shape origin/pivot marker now renders as an X and its visual span is doubled from 8px to 16px.
11+
- Object origin/pivot marker now renders as a + and uses a doubled 16px visual span.
12+
- Shape z-order actions in Objects > Shapes now apply to all currently selected shapes while preserving relative selection order.
13+
- Locked shape protection, shape override remapping, and selected/direct-selected index remapping are preserved after multi-shape order changes.
14+
- Workspace V2 Playwright coverage was updated for the new X/+ marker contract and doubled marker sizing.
15+
16+
## Targeted Object Vector V2 Validation
17+
- PASS: object rotation path preserved; object origin marker is rendered separately from selected-shape pivot markers.
18+
- PASS: shape rotation path preserved; selected shape origin marker remains tied to transformed bounds/origin and now renders as X.
19+
- PASS: multi-selected shape z-order command moved selected rows together and preserved selection after reorder.
20+
- PASS: marker structure and size verified in Playwright selectors: shape X span 16px, object + span 16px.
21+
22+
## Commands
23+
- PASS: node --check tools/object-vector-studio-v2/js/ToolStarterApp.js
24+
- PASS: node --check tests/playwright/tools/WorkspaceManagerV2.spec.mjs
25+
- PASS: git diff --check (CRLF advisory warnings only)
26+
- PASS: npx playwright test tests/playwright/tools/WorkspaceManagerV2.spec.mjs --project=playwright --workers=1 --reporter=list -g "Object Vector Studio V2 layout shell|creates Object Vector Studio V2 shapes with canvas drawing"
27+
- PASS: npm run test:workspace-v2 (56 passed)
28+
29+
## Notes
30+
- Full samples smoke test was skipped per PR_26133_100 instructions.
31+
- No workspace schema or manifest contract changes were made.

tests/playwright/tools/WorkspaceManagerV2.spec.mjs

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2861,16 +2861,31 @@ test.describe("Workspace Manager V2 bootstrap", () => {
28612861
await expect(page.locator("#objectVectorStudioV2RenderSurface [data-selection-bounds='0']")).toHaveCount(1);
28622862
await expect(page.locator("#objectVectorStudioV2RenderSurface [data-resize-handle]")).toHaveCount(4);
28632863
await expect(page.locator("#objectVectorStudioV2RenderSurface [data-pivot-origin='0']")).toHaveCount(1);
2864+
await expect(page.locator("#objectVectorStudioV2RenderSurface [data-object-origin='object.asteroids.object-1']")).toHaveCount(1);
28642865
const selectionChrome = await page.locator("#objectVectorStudioV2RenderSurface").evaluate((surface) => {
28652866
const selectionBox = surface.querySelector("[data-selection-bounds='0']");
28662867
const handle = surface.querySelector("[data-resize-handle]");
2868+
const [shapeDiagonalA, shapeDiagonalB] = Array.from(surface.querySelector("[data-pivot-origin='0']").querySelectorAll("line"));
2869+
const [objectHorizontal, objectVertical] = Array.from(surface.querySelector("[data-object-origin='object.asteroids.object-1']").querySelectorAll("line"));
28672870
return {
28682871
handleHeight: Number(handle.getAttribute("height")),
28692872
handleWidth: Number(handle.getAttribute("width")),
2870-
selectionStrokeWidth: getComputedStyle(selectionBox).strokeWidth
2873+
objectHorizontalSpan: Math.abs(Number(objectHorizontal.getAttribute("x2")) - Number(objectHorizontal.getAttribute("x1"))),
2874+
objectVerticalSpan: Math.abs(Number(objectVertical.getAttribute("y2")) - Number(objectVertical.getAttribute("y1"))),
2875+
selectionStrokeWidth: getComputedStyle(selectionBox).strokeWidth,
2876+
shapeDiagonalASpan: Math.abs(Number(shapeDiagonalA.getAttribute("x2")) - Number(shapeDiagonalA.getAttribute("x1"))),
2877+
shapeDiagonalBSpan: Math.abs(Number(shapeDiagonalB.getAttribute("y2")) - Number(shapeDiagonalB.getAttribute("y1")))
28712878
};
28722879
});
2873-
expect(selectionChrome).toEqual({ handleHeight: 3, handleWidth: 3, selectionStrokeWidth: "0.75px" });
2880+
expect(selectionChrome).toEqual({
2881+
handleHeight: 3,
2882+
handleWidth: 3,
2883+
objectHorizontalSpan: 16,
2884+
objectVerticalSpan: 16,
2885+
selectionStrokeWidth: "0.75px",
2886+
shapeDiagonalASpan: 16,
2887+
shapeDiagonalBSpan: 16
2888+
});
28742889

28752890
await clickPreviewShape(1, { shiftKey: true });
28762891
await expect(page.locator("#objectVectorStudioV2RenderSurface [data-shape-index='0']")).toHaveClass(/is-selected/);
@@ -3007,10 +3022,10 @@ test.describe("Workspace Manager V2 bootstrap", () => {
30073022
expect(originAfterApply).toEqual({ x: 2, y: -3 });
30083023
await expect(page.locator("#statusLog")).toHaveValue(/OK Updated shape row 0 origin\/pivot to 2, -3\./);
30093024
const pivotMarkerAfterOrigin = await page.locator("#objectVectorStudioV2RenderSurface [data-pivot-origin='0']").evaluate((pivot) => {
3010-
const [horizontal, vertical] = Array.from(pivot.querySelectorAll("line"));
3025+
const [diagonal] = Array.from(pivot.querySelectorAll("line"));
30113026
return {
3012-
x: Number(vertical.getAttribute("x1")),
3013-
y: Number(horizontal.getAttribute("y1"))
3027+
x: Number(((Number(diagonal.getAttribute("x1")) + Number(diagonal.getAttribute("x2"))) / 2).toFixed(3)),
3028+
y: Number(((Number(diagonal.getAttribute("y1")) + Number(diagonal.getAttribute("y2"))) / 2).toFixed(3))
30143029
};
30153030
});
30163031
expect(pivotMarkerAfterOrigin).toEqual({ x: 150, y: 40 });
@@ -4374,19 +4389,24 @@ test.describe("Workspace Manager V2 bootstrap", () => {
43744389
strokeWidth: 20
43754390
});
43764391
const pivotMarkerState = await page.locator("#objectVectorStudioV2RenderSurface .object-vector-studio-v2__pivot-origin").evaluate((pivot) => {
4377-
const [horizontal, vertical] = Array.from(pivot.querySelectorAll("line"));
4392+
const [diagonalForward, diagonalBack] = Array.from(pivot.querySelectorAll("line"));
43784393
return {
43794394
ariaLabel: pivot.getAttribute("aria-label"),
4380-
horizontalSpan: Math.abs(Number(horizontal.getAttribute("x2")) - Number(horizontal.getAttribute("x1"))),
4395+
diagonalBackSpan: Math.abs(Number(diagonalBack.getAttribute("y2")) - Number(diagonalBack.getAttribute("y1"))),
4396+
diagonalForwardSpan: Math.abs(Number(diagonalForward.getAttribute("x2")) - Number(diagonalForward.getAttribute("x1"))),
43814397
title: pivot.querySelector("title")?.textContent || "",
4382-
verticalSpan: Math.abs(Number(vertical.getAttribute("y2")) - Number(vertical.getAttribute("y1")))
4398+
xMarker: diagonalForward.getAttribute("x1") === diagonalBack.getAttribute("x1")
4399+
&& diagonalForward.getAttribute("x2") === diagonalBack.getAttribute("x2")
4400+
&& diagonalForward.getAttribute("y1") === diagonalBack.getAttribute("y2")
4401+
&& diagonalForward.getAttribute("y2") === diagonalBack.getAttribute("y1")
43834402
};
43844403
});
43854404
expect(pivotMarkerState).toEqual({
43864405
ariaLabel: "Origin/Pivot marker for selected shape rotation and scale",
4387-
horizontalSpan: 8,
4406+
diagonalBackSpan: 16,
4407+
diagonalForwardSpan: 16,
43884408
title: "Origin/Pivot: rotate and scale pivot for the selected shape.",
4389-
verticalSpan: 8
4409+
xMarker: true
43904410
});
43914411
const previewToolbarAfterText = await page.evaluate(() => Array.from(document.querySelectorAll(".object-vector-studio-v2__preview-edit-toolbar button")).map((button) => ({
43924412
disabled: button.disabled,

tools/object-vector-studio-v2/js/ToolStarterApp.js

Lines changed: 128 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3238,6 +3238,7 @@ export class ToolStarterApp {
32383238
hitLayer.remove();
32393239
}
32403240
this.renderObjectBounds(object);
3241+
this.renderObjectOriginMarker(object);
32413242
this.renderSnapPointTargets(object);
32423243
this.renderDrawingPreview();
32433244
this.renderDrawingHint();
@@ -3416,6 +3417,34 @@ export class ToolStarterApp {
34163417
this.elements.renderSurface.append(box);
34173418
}
34183419

3420+
renderObjectOriginMarker(object) {
3421+
if (!object) {
3422+
return;
3423+
}
3424+
const origin = this.objectTransformOrigin(object);
3425+
const x = this.scaleDrawingValue(origin.x, OBJECT_PREVIEW_DRAWING_SCALE);
3426+
const y = this.scaleDrawingValue(origin.y, OBJECT_PREVIEW_DRAWING_SCALE);
3427+
const marker = document.createElementNS(SVG_NS, "g");
3428+
marker.classList.add("object-vector-studio-v2__object-origin");
3429+
marker.dataset.objectOrigin = object.id;
3430+
marker.setAttribute("role", "img");
3431+
marker.setAttribute("aria-label", "Object origin/pivot marker for object rotation and scale");
3432+
const markerTitle = document.createElementNS(SVG_NS, "title");
3433+
markerTitle.textContent = "Object Origin/Pivot: rotate and scale pivot for the selected object.";
3434+
const horizontal = document.createElementNS(SVG_NS, "line");
3435+
horizontal.setAttribute("x1", x - 8);
3436+
horizontal.setAttribute("x2", x + 8);
3437+
horizontal.setAttribute("y1", y);
3438+
horizontal.setAttribute("y2", y);
3439+
const vertical = document.createElementNS(SVG_NS, "line");
3440+
vertical.setAttribute("x1", x);
3441+
vertical.setAttribute("x2", x);
3442+
vertical.setAttribute("y1", y - 8);
3443+
vertical.setAttribute("y2", y + 8);
3444+
marker.append(markerTitle, horizontal, vertical);
3445+
this.elements.renderSurface.append(marker);
3446+
}
3447+
34193448
renderCenterOriginMarker() {
34203449
if (!this.centerOriginVisible) {
34213450
return;
@@ -4276,17 +4305,17 @@ export class ToolStarterApp {
42764305
pivot.setAttribute("aria-label", "Origin/Pivot marker for selected shape rotation and scale");
42774306
const pivotTitle = document.createElementNS(SVG_NS, "title");
42784307
pivotTitle.textContent = "Origin/Pivot: rotate and scale pivot for the selected shape.";
4279-
const horizontal = document.createElementNS(SVG_NS, "line");
4280-
horizontal.setAttribute("x1", bounds.originX - 4);
4281-
horizontal.setAttribute("x2", bounds.originX + 4);
4282-
horizontal.setAttribute("y1", bounds.originY);
4283-
horizontal.setAttribute("y2", bounds.originY);
4284-
const vertical = document.createElementNS(SVG_NS, "line");
4285-
vertical.setAttribute("x1", bounds.originX);
4286-
vertical.setAttribute("x2", bounds.originX);
4287-
vertical.setAttribute("y1", bounds.originY - 4);
4288-
vertical.setAttribute("y2", bounds.originY + 4);
4289-
pivot.append(pivotTitle, horizontal, vertical);
4308+
const diagonalForward = document.createElementNS(SVG_NS, "line");
4309+
diagonalForward.setAttribute("x1", bounds.originX - 8);
4310+
diagonalForward.setAttribute("x2", bounds.originX + 8);
4311+
diagonalForward.setAttribute("y1", bounds.originY - 8);
4312+
diagonalForward.setAttribute("y2", bounds.originY + 8);
4313+
const diagonalBack = document.createElementNS(SVG_NS, "line");
4314+
diagonalBack.setAttribute("x1", bounds.originX - 8);
4315+
diagonalBack.setAttribute("x2", bounds.originX + 8);
4316+
diagonalBack.setAttribute("y1", bounds.originY + 8);
4317+
diagonalBack.setAttribute("y2", bounds.originY - 8);
4318+
pivot.append(pivotTitle, diagonalForward, diagonalBack);
42904319
this.elements.renderSurface.append(pivot);
42914320
} catch (error) {
42924321
this.statusLog.write(`FAIL Selection overlay render failed for ${object.name}/shape-${this.selectedShapeIndex} (${shapeTool(selectedShape)}): ${error.message}`);
@@ -6631,44 +6660,111 @@ export class ToolStarterApp {
66316660
this.statusLog.write("WARN Shape order skipped: no shape is selected.");
66326661
return;
66336662
}
6634-
if (selected.locked) {
6635-
this.statusLog.write(`WARN Shape order skipped: shape row ${this.selectedShapeIndex} is locked.`);
6636-
return;
6637-
}
66386663
if (this.guardSelectedObjectMutation("Shape order")) {
66396664
return;
66406665
}
66416666

66426667
const nextPayload = this.cloneCurrentPayload();
66436668
const object = nextPayload.objects.find((candidate) => candidate.id === this.selectedObjectId);
66446669
const shapes = sortedShapes(object);
6645-
const index = this.selectedShapeIndex;
6670+
const selectedIndexes = Array.from(new Set(Array.from(this.selectedShapeIndexes || [])
6671+
.map((shapeIndex) => normalizeShapeIndex(shapeIndex))
6672+
.filter((shapeIndex) => shapeIndex >= 0 && shapeIndex < shapes.length)))
6673+
.sort((left, right) => left - right);
6674+
if (!selectedIndexes.length && this.selectedShapeIndex >= 0 && this.selectedShapeIndex < shapes.length) {
6675+
selectedIndexes.push(this.selectedShapeIndex);
6676+
}
6677+
const lockedIndex = selectedIndexes.find((shapeIndex) => shapes[shapeIndex]?.locked);
6678+
if (Number.isInteger(lockedIndex)) {
6679+
this.statusLog.write(`WARN Shape order skipped: shape row ${lockedIndex} is locked.`);
6680+
return;
6681+
}
66466682
if (shapes.length < 2) {
6647-
this.statusLog.write(`WARN Shape order skipped: shape row ${index} is the only shape in ${object.name}.`);
6683+
this.statusLog.write(`WARN Shape order skipped: shape row ${this.selectedShapeIndex} is the only shape in ${object.name}.`);
66486684
return;
66496685
}
66506686

6651-
const nextIndexByAction = {
6652-
back: 0,
6653-
backward: index - 1,
6654-
forward: index + 1,
6655-
front: shapes.length - 1
6656-
};
6657-
const nextIndex = nextIndexByAction[action];
6658-
if (!Number.isInteger(nextIndex) || nextIndex < 0 || nextIndex >= shapes.length || nextIndex === index) {
6659-
this.statusLog.write(`WARN Shape z-order skipped: shape row ${index} cannot move ${action}.`);
6687+
const selectedIndexSet = new Set(selectedIndexes);
6688+
const oldIndexByShape = new Map(shapes.map((shape, shapeIndex) => [shape, shapeIndex]));
6689+
const reorderedShapes = shapes.slice();
6690+
if (action === "back") {
6691+
reorderedShapes.sort((left, right) => {
6692+
const leftIndex = oldIndexByShape.get(left);
6693+
const rightIndex = oldIndexByShape.get(right);
6694+
const leftSelected = selectedIndexSet.has(leftIndex);
6695+
const rightSelected = selectedIndexSet.has(rightIndex);
6696+
if (leftSelected === rightSelected) {
6697+
return leftIndex - rightIndex;
6698+
}
6699+
return leftSelected ? -1 : 1;
6700+
});
6701+
} else if (action === "front") {
6702+
reorderedShapes.sort((left, right) => {
6703+
const leftIndex = oldIndexByShape.get(left);
6704+
const rightIndex = oldIndexByShape.get(right);
6705+
const leftSelected = selectedIndexSet.has(leftIndex);
6706+
const rightSelected = selectedIndexSet.has(rightIndex);
6707+
if (leftSelected === rightSelected) {
6708+
return leftIndex - rightIndex;
6709+
}
6710+
return leftSelected ? 1 : -1;
6711+
});
6712+
} else if (action === "backward") {
6713+
for (let index = 1; index < reorderedShapes.length; index += 1) {
6714+
const currentOldIndex = oldIndexByShape.get(reorderedShapes[index]);
6715+
const previousOldIndex = oldIndexByShape.get(reorderedShapes[index - 1]);
6716+
if (selectedIndexSet.has(currentOldIndex) && !selectedIndexSet.has(previousOldIndex)) {
6717+
[reorderedShapes[index - 1], reorderedShapes[index]] = [reorderedShapes[index], reorderedShapes[index - 1]];
6718+
}
6719+
}
6720+
} else if (action === "forward") {
6721+
for (let index = reorderedShapes.length - 2; index >= 0; index -= 1) {
6722+
const currentOldIndex = oldIndexByShape.get(reorderedShapes[index]);
6723+
const nextOldIndex = oldIndexByShape.get(reorderedShapes[index + 1]);
6724+
if (selectedIndexSet.has(currentOldIndex) && !selectedIndexSet.has(nextOldIndex)) {
6725+
[reorderedShapes[index], reorderedShapes[index + 1]] = [reorderedShapes[index + 1], reorderedShapes[index]];
6726+
}
6727+
}
6728+
} else {
6729+
this.statusLog.write(`WARN Shape z-order skipped: unsupported action ${action}.`);
66606730
return;
66616731
}
66626732

66636733
const oldShapes = shapes.slice();
6664-
const [movedShape] = shapes.splice(index, 1);
6665-
shapes.splice(nextIndex, 0, movedShape);
6666-
shapes.forEach((shape, shapeIndex) => {
6734+
if (oldShapes.every((shape, shapeIndex) => reorderedShapes[shapeIndex] === shape)) {
6735+
const subject = selectedIndexes.length > 1 ? `selected shape rows ${selectedIndexes.join(", ")}` : `shape row ${this.selectedShapeIndex}`;
6736+
this.statusLog.write(`WARN Shape z-order skipped: ${subject} cannot move ${action}.`);
6737+
return;
6738+
}
6739+
const oldIndexToNextIndex = new Map();
6740+
oldShapes.forEach((shape, oldIndex) => {
6741+
oldIndexToNextIndex.set(oldIndex, reorderedShapes.indexOf(shape));
6742+
});
6743+
reorderedShapes.forEach((shape, shapeIndex) => {
66676744
shape.order = shapeIndex + 1;
66686745
});
6669-
object.shapes = shapes;
6670-
this.remapShapeOverrideIndexes(object, oldShapes, shapes);
6671-
this.commitPayloadUpdate(nextPayload, this.selectedObjectId, nextIndex, `OK Shape row ${index} z-order ${action}.`, "Shape z-order failed schema validation");
6746+
object.shapes = reorderedShapes;
6747+
this.remapShapeOverrideIndexes(object, oldShapes, reorderedShapes);
6748+
const selectedShapeIndexes = new Set(selectedIndexes
6749+
.map((shapeIndex) => oldIndexToNextIndex.get(shapeIndex))
6750+
.filter((shapeIndex) => Number.isInteger(shapeIndex) && shapeIndex >= 0));
6751+
const directSelectedShapeIndexes = new Set(Array.from(this.directSelectedShapeIndexes || [])
6752+
.map((shapeIndex) => oldIndexToNextIndex.get(normalizeShapeIndex(shapeIndex)))
6753+
.filter((shapeIndex) => Number.isInteger(shapeIndex) && shapeIndex >= 0));
6754+
if (!directSelectedShapeIndexes.size) {
6755+
selectedShapeIndexes.forEach((shapeIndex) => directSelectedShapeIndexes.add(shapeIndex));
6756+
}
6757+
const nextSelectedShapeIndex = oldIndexToNextIndex.get(this.selectedShapeIndex);
6758+
const primaryShapeIndex = Number.isInteger(nextSelectedShapeIndex) && nextSelectedShapeIndex >= 0
6759+
? nextSelectedShapeIndex
6760+
: Math.min(...selectedShapeIndexes);
6761+
const okMessage = selectedIndexes.length > 1
6762+
? `OK Shape rows ${selectedIndexes.join(", ")} z-order ${action}.`
6763+
: `OK Shape row ${this.selectedShapeIndex} z-order ${action}.`;
6764+
this.commitPayloadUpdate(nextPayload, this.selectedObjectId, primaryShapeIndex, okMessage, "Shape z-order failed schema validation", {
6765+
directSelectedShapeIndexes,
6766+
selectedShapeIndexes
6767+
});
66726768
}
66736769

66746770
objectTransformOrigin(object) {

tools/object-vector-studio-v2/styles/toolStarter.css

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,13 +1585,19 @@ textarea:hover {
15851585
pointer-events: none;
15861586
}
15871587

1588+
.object-vector-studio-v2__object-origin line,
15881589
.object-vector-studio-v2__pivot-origin line {
15891590
stroke: #f8fafc;
15901591
stroke-width: 1.2;
15911592
pointer-events: none;
15921593
vector-effect: non-scaling-stroke;
15931594
}
15941595

1596+
.object-vector-studio-v2__object-origin line {
1597+
stroke: var(--tool-starter-accent-strong, #fbbf24);
1598+
stroke-width: 1.5;
1599+
}
1600+
15951601
.object-vector-studio-v2__object-detail-grid {
15961602
display: grid;
15971603
grid-template-columns: 104px minmax(0, 1fr);

0 commit comments

Comments
 (0)