Skip to content

Commit b5a9742

Browse files
committed
Even more bugs, snap now for the first colstart works
1 parent c6833f4 commit b5a9742

4 files changed

Lines changed: 55 additions & 111 deletions

File tree

modules/@apostrophecms/layout-widget/ui/apos/components/AposGridManager.vue

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@
261261
class="apos-layout__item-ghost"
262262
/>
263263
<div
264-
v-if="hasMotion && ghostData.snapLeft"
264+
v-if="hasMotion && typeof ghostData.snapLeft === 'number'"
265265
:style="{
266266
left: ghostData.snapLeft + 'px',
267267
top: ghostData.snapTop + 'px',
@@ -361,9 +361,7 @@ export default {
361361
colstart: null,
362362
rowstart: null,
363363
rowspan: null,
364-
order: null,
365-
snapColstart: null,
366-
snapRowstart: null
364+
order: null
367365
},
368366
moveDataIndex: null,
369367
// Recalculate the grid overlay styles
@@ -612,7 +610,7 @@ export default {
612610
event.preventDefault();
613611
event.stopPropagation();
614612
const {
615-
left, top, snapLeft, snapTop, colstart, rowstart, snapColstart, snapRowstart
613+
left, top, snapLeft, snapTop, colstart, rowstart
616614
} = this.manager.onGhostMove(
617615
{
618616
state: this.gridState,
@@ -632,10 +630,6 @@ export default {
632630
this.ghostDataWrite.colstart = colstart;
633631
this.ghostDataWrite.rowstart = rowstart;
634632
}
635-
if (snapColstart && snapRowstart) {
636-
this.ghostDataWrite.snapColstart = snapColstart;
637-
this.ghostDataWrite.snapRowstart = snapRowstart;
638-
}
639633
},
640634
onMouseMove(event) {
641635
if (this.isResizing) {
@@ -703,7 +697,6 @@ export default {
703697
index: this.moveDataIndex
704698
}
705699
);
706-
console.log('Patches from move:', patches);
707700
this.$emit('move-end', patches);
708701
this.resetGhostData();
709702
},

modules/@apostrophecms/layout-widget/ui/apos/lib/grid-manager.js

Lines changed: 2 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,7 @@ export class GridManager {
342342
* snapLeft?: number,
343343
* snapTop?: number,
344344
* colstart?: number,
345-
* rowstart?: number,
346-
* snapColstart?: number,
347-
* snapRowstart?: number
345+
* rowstart?: number
348346
* }} - The new position of the ghost item and optional snap info.
349347
*/
350348
onGhostMove({
@@ -392,12 +390,6 @@ export class GridManager {
392390
}
393391

394392
// Compute optional snapping with minimal overhead.
395-
// Uses cached grid metrics and the positions index to find the nearest
396-
// valid cell start that can fit the moving item (ignoring the item itself).
397-
// let snapLeft;
398-
// let snapTop;
399-
// let snapColstart;
400-
// let snapRowstart;
401393
const style = this.getGridComputedStyle();
402394
const colGap = parseFloat(style.columnGap || style.gap) || 0;
403395
const rowGap = parseFloat(style.rowGap || style.gap) || 0;
@@ -423,78 +415,16 @@ export class GridManager {
423415
const colstart = c;
424416
const rowstart = r;
425417

426-
// // Fast validator: region must be empty or occupied by the moving item
427-
// // in every spanned row.
428-
// const positions = state.positions;
429-
// const canPlaceAt = (rr, cc) => {
430-
// // bounds check
431-
// if (rr < 1 || rr > maxStartY || cc < 1 || cc > maxStartX) {
432-
// return false;
433-
// }
434-
// for (let ry = 0; ry < rowspan; ry++) {
435-
// const rowIndex = positions.get(rr + ry);
436-
// if (!rowIndex) {
437-
// // No occupancy recorded: free row segment
438-
// continue;
439-
// }
440-
// for (let cx = 0; cx < colspan; cx++) {
441-
// const occupant = rowIndex.get(cc + cx);
442-
// if (occupant && occupant !== item._id) {
443-
// return false;
444-
// }
445-
// }
446-
// }
447-
// return true;
448-
// };
449-
450-
// // Search in expanding Manhattan rings around (r,c) for a valid spot.
451-
// // With typical columns <= 12 and small row counts, this is cheap.
452-
// let found = false;
453-
// const maxCDelta = Math.max(0, maxStartX - 1);
454-
// const maxRDelta = Math.min(3, Math.max(0, maxStartY - 1));
455-
// for (let rd = 0; rd <= maxRDelta && !found; rd++) {
456-
// const rCandidates = rd === 0 ? [ r ] : [ r - rd, r + rd ];
457-
// for (let i = 0; i < rCandidates.length && !found; i++) {
458-
// const rr = rCandidates[i];
459-
// if (rr < 1 || rr > maxStartY) {
460-
// continue;
461-
// }
462-
// // Column scan from nearest outward
463-
// for (let cd = 0; cd <= maxCDelta; cd++) {
464-
// const cc1 = c - cd;
465-
// if (cc1 >= 1 && cc1 <= maxStartX && canPlaceAt(rr, cc1)) {
466-
// snapColstart = cc1; snapRowstart = rr; found = true; break;
467-
// }
468-
// if (cd === 0) {
469-
// continue; // avoid duplicate center
470-
// }
471-
// const cc2 = c + cd;
472-
// if (cc2 >= 1 && cc2 <= maxStartX && canPlaceAt(rr, cc2)) {
473-
// snapColstart = cc2; snapRowstart = rr; found = true; break;
474-
// }
475-
// }
476-
// }
477-
// }
478-
479-
// if (found && snapColstart && snapRowstart) {
480-
// snapLeft = Math.round((snapColstart - 1) * stepX);
481-
// snapTop = Math.round((snapRowstart - 1) * stepY);
482-
// }
483-
484418
const snapLeft = Math.round((colstart - 1) * stepX);
485419
const snapTop = Math.round((rowstart - 1) * stepY);
486-
const snapColstart = colstart;
487-
const snapRowstart = rowstart;
488420

489421
return {
490422
left,
491423
top,
492424
snapLeft,
493425
snapTop,
494426
colstart,
495-
rowstart,
496-
snapColstart,
497-
snapRowstart
427+
rowstart
498428
};
499429
}
500430

modules/@apostrophecms/layout-widget/ui/apos/lib/grid-state.mjs

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@
99
* colstart: number,
1010
* rowstart: number,
1111
* rowspan: number,
12-
* order: number,
13-
* snapColstart?: number,
14-
* snapRowstart?: number,
12+
* order: number
1513
* }} GhostDataWrite
1614
*
1715
*
@@ -120,10 +118,18 @@ export function createPositionIndex(items, rows) {
120118
}
121119
for (const item of sorted) {
122120
const {
123-
colstart, colspan, rowstart
121+
colstart, colspan, rowstart, rowspan
124122
} = item;
125-
for (let i = 0; i < colspan; i++) {
126-
positionsIndex.get(rowstart)?.set(colstart + i, item._id);
123+
const height = Math.max(1, rowspan || 1);
124+
for (let r = 0; r < height; r++) {
125+
const row = rowstart + r;
126+
const xIndex = positionsIndex.get(row);
127+
if (!xIndex) {
128+
continue;
129+
}
130+
for (let i = 0; i < colspan; i++) {
131+
xIndex.set(colstart + i, item._id);
132+
}
127133
}
128134
}
129135

@@ -402,6 +408,10 @@ export function getMoveChanges({
402408
item,
403409
precomp
404410
}) {
411+
// Guard against mismatched item/data identifiers when the target id exists in lookup
412+
if (data.id && state?.lookup?.has?.(data.id) && item?._id && data.id !== item._id) {
413+
return [];
414+
}
405415
if (!data.colstart || !data.rowstart ||
406416
(data.colstart === item.colstart && data.rowstart === item.rowstart)
407417
) {
@@ -451,26 +461,17 @@ export function getMoveChanges({
451461
}
452462

453463
// Strategy 2: horizontal nudge of neighbours only
454-
const overlapExists = true; // we only reach here when placeIfFree is false
464+
// We only reach here when placeIfFree is false
455465
// Decide preferred nudge directions based on movement intent and edge overlaps
456466
const oldStartCol = item.colstart;
457467
const oldEndCol = oldStartCol + (item.colspan || 1) - 1;
458468
// Check if the target start column is empty across all spanned rows AND
459469
// lies outside the item's original horizontal footprint. This avoids
460470
// misclassifying small same-direction shifts (where newStartCol is still
461471
// within the old footprint) as the "empty start" special case.
462-
const startOutsideOld = (newStartCol < oldStartCol) || (newStartCol > oldEndCol);
463-
const targetStartEmpty = startOutsideOld && (() => {
464-
for (let r = newStartRow; r <= newEndRow; r++) {
465-
const occRow = precomp?.occByRow?.get(r);
466-
const rowIndex = state.positions.get(r);
467-
const id = (occRow ? occRow[newStartCol] : rowIndex?.get(newStartCol));
468-
if (id && id !== item._id) {
469-
return false;
470-
}
471-
}
472-
return true;
473-
})();
472+
const _startOutsideOld = (newStartCol < oldStartCol) || (newStartCol > oldEndCol);
473+
// Note: no special opposite-direction bias solely because the target start
474+
// cell is empty. We rely on boundary overlap/equal-edge heuristics below.
474475
// Note: vertical-only moves are handled specially below
475476
// Note: vertical-only moves are handled specially below
476477

@@ -500,11 +501,6 @@ export function getMoveChanges({
500501
if (!primaryDir) {
501502
return [ 'east', 'west' ];
502503
}
503-
// Special rule: targeting an empty start cell but overall footprint overlaps
504-
// -> attempt only the opposite direction.
505-
if (overlapExists && targetStartEmpty) {
506-
return primaryDir === 'east' ? [ 'west' ] : [ 'east' ];
507-
}
508504
if (!target) {
509505
// No concrete target: only try primary to avoid unintended cascades.
510506
return primaryDir === 'east' ? [ 'east' ] : [ 'west' ];
@@ -841,6 +837,8 @@ function attemptHorizontalNudge({
841837
// No shift for this item; it defines the next available start.
842838
neededStart = Math.max(neededStart, end0 + 1);
843839
}
840+
// No need to fail if neededStart exceeds grid after processing;
841+
// we already enforce bounds on each neighbor shift.
844842
}
845843
} else {
846844
// dir === 'west'
@@ -862,9 +860,8 @@ function attemptHorizontalNudge({
862860
// No shift; it defines the next available end.
863861
neededEnd = Math.min(neededEnd, start0 - 1);
864862
}
865-
if (neededEnd < 0) {
866-
return null;
867-
}
863+
// Similarly, don't fail solely due to neededEnd becoming < 0
864+
// after processing; individual shifts are already bounded.
868865
}
869866
}
870867

test/layout-widget-ui.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe('Layout Widget', function () {
7878
assert.equal(pm.get('a')?.colstart, undefined);
7979
});
8080

81-
it('[getMoveChanges] Test 1: reject until equal-edge swap allowed', async function () {
81+
it('[getMoveChanges] v1: reject until equal-edge swap allowed', async function () {
8282
// 12 cols, 1 row
8383
// 1) a: 1..5, 2) b: 9..12
8484
const lib = await getLib();
@@ -147,7 +147,7 @@ describe('Layout Widget', function () {
147147
assert.equal(pm.get('a')?.colstart, 5); // a nudged east to start at 5
148148
});
149149

150-
it('[getMoveChanges] Test 2: reject overlaps for b; allow free move for a', async function () {
150+
it('[getMoveChanges] v2: reject overlaps for b; allow free move for a', async function () {
151151
// 1) a: 1..4, 2) b: 8..12
152152
const lib = await getLib();
153153
const items = [
@@ -184,6 +184,30 @@ describe('Layout Widget', function () {
184184
assert.equal(pm.get('b')?.colstart, undefined);
185185
});
186186

187+
it('[getMoveChanges] v3: reject overlaps for a and b (east)', async function () {
188+
// 1) a: 1..4, 2) b: 9..12
189+
const lib = await getLib();
190+
const items = [
191+
buildItem('a', 1, 5, 0),
192+
buildItem('b', 9, 4, 1)
193+
];
194+
const state = makeState(lib, items, 12, 1);
195+
196+
// Moving a into overlaps should be rejected
197+
for (const start of [ 5, 6, 7 ]) {
198+
const patches = lib.getMoveChanges({
199+
data: {
200+
id: 'a',
201+
colstart: start,
202+
rowstart: 1
203+
},
204+
state,
205+
item: items[1]
206+
});
207+
assert.deepEqual(patches, [], `Failed to reject move with ${start - 4} step(s) east`);
208+
}
209+
});
210+
187211
it('[getMoveChanges] equal-edge triggers opposite-direction nudging first, then fallback', async function () {
188212
// a: 1..4, b: 7..8. Move a -> colstart 5 (a new end 8 equals b end 8).
189213
const lib = await getLib();

0 commit comments

Comments
 (0)