Skip to content

Commit 0bcef1d

Browse files
authored
Merge pull request #3420 from adumesny/master
don't recompute w/h during resize if not changed (rounding errors)
2 parents af32ccd + 875848e commit 0bcef1d

5 files changed

Lines changed: 107 additions & 12 deletions

File tree

‎doc/CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ Change log
152152
* fix: [#2666](https://github.com/gridstack/gridstack.js/issues/2666) iOS auto-scroll uses visualViewport, and un-bind touch handlers
153153
* fix: [#3418](https://github.com/gridstack/gridstack.js/pull/3418) preserve saved layout after cancelling a cross-grid drag- #3418 - thank you [sameerdeolalikar](https://github.com/sameerdeolalikar)
154154
* fix: [#2819](https://github.com/gridstack/gridstack.js/issues/2819) item not moved (to alt position) when colliding fails with an object
155+
* fix: [#3230](https://github.com/gridstack/gridstack.js/issues/3230) don't recompute w/h unless side can change (rounding errors)
155156

156157
## 14.0.0 (2026-09-20)
157158
* feat: [#754](https://github.com/gridstack/gridstack.js/issues/754), [#2866](https://github.com/gridstack/gridstack.js/issues/2866) new `mode?: 'top' | 'float' | 'list' | 'compact'` - items are continuously re-flowed in sequential (row-major) order, like a re-orderable list: dragging, resizing, adding or removing an item re-flows everyone else instead of pushing them down, and dropping an item on another takes its place. See new
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { GridStack } from '../../src/gridstack';
2+
import type { GridStackMouseEvent, DDUIData } from '../../src/types';
3+
4+
// Regression test for https://github.com/gridstack/gridstack.js/issues/3230
5+
// "Horizontal resize unexpectedly changes height when using very high column count and small cellHeight"
6+
//
7+
// _dragOrResize() used to re-derive BOTH w and h from the resize helper's pixel size on every
8+
// 'resize' move event, regardless of which handle was actually being dragged. The dimension the
9+
// user isn't touching is supposed to stay constant in pixels, but re-deriving it via
10+
// Math.round(px / cellSize) is lossy: any subpixel/rounding noise in the measured pixel value can
11+
// flip it by a whole row/column once cellHeight (or cellWidth) is only a few pixels, which is
12+
// exactly the "column: 1000, cellHeight: 1" setup from the issue.
13+
describe('GridStack resize direction isolation (#3230)', () => {
14+
it('does not change height when only the horizontal (w) handle is dragged', () => {
15+
document.body.innerHTML = `
16+
<div class="grid-stack">
17+
<div class="grid-stack-item" gs-x="0" gs-y="0" gs-w="10" gs-h="10">
18+
<div class="grid-stack-item-content">item</div>
19+
</div>
20+
</div>
21+
`;
22+
const grid = GridStack.init({ cellHeight: 1, margin: 3 });
23+
const node = grid.engine.nodes[0];
24+
const el = node.el!;
25+
26+
const cellWidth = 1;
27+
const cellHeight = 1;
28+
29+
// simulate dragging only the 'w' (west/left) resize handle
30+
const event = {
31+
type: 'resize',
32+
target: el,
33+
resizeDir: 'w',
34+
hasMovedX: true,
35+
hasMovedY: false,
36+
} as unknown as GridStackMouseEvent;
37+
38+
// height carries subpixel rounding noise (10.5 instead of an exact 10) even though the 'w'
39+
// handle never touches height - DDResizable leaves it as the untouched original pixel height
40+
const ui = {
41+
position: { top: 0, left: 2 },
42+
size: { width: 8, height: 10.5 },
43+
} as DDUIData;
44+
45+
(grid as unknown as { _dragOrResize: (...args: unknown[]) => void })
46+
._dragOrResize(el, event, ui, node, cellWidth, cellHeight);
47+
48+
expect(node.w).toBe(8); // the dragged dimension updates correctly
49+
expect(node.h).toBe(10); // the untouched dimension must not drift because of pixel rounding noise
50+
});
51+
});

‎spec/utils-spec.ts‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -818,15 +818,47 @@ describe('gridstack utils', () => {
818818
it('should get transform values from parent', () => {
819819
const parent = document.createElement('div');
820820
document.body.appendChild(parent);
821-
821+
822822
const result = Utils.getValuesFromTransformedElement(parent);
823-
823+
824824
expect(result.xScale).toBeDefined();
825825
expect(result.yScale).toBeDefined();
826826
expect(result.xOffset).toBeDefined();
827827
expect(result.yOffset).toBeDefined();
828-
828+
829+
document.body.removeChild(parent);
830+
});
831+
832+
// Regression test for https://github.com/gridstack/gridstack.js/issues/3230
833+
it('keeps scale close to 1 despite device-pixel snapping error, thanks to a large probe', () => {
834+
const parent = document.createElement('div');
835+
document.body.appendChild(parent);
836+
const originalGetBoundingClientRect = HTMLElement.prototype.getBoundingClientRect;
837+
838+
// Simulate what real browsers do at fractional OS/browser zoom (125%, 150%,...): any
839+
// measured element's getBoundingClientRect() is off from its authored CSS size by a fixed
840+
// absolute snapping error, regardless of how big that element is.
841+
const snapError = 0.5;
842+
HTMLElement.prototype.getBoundingClientRect = function (this: HTMLElement) {
843+
const width = parseFloat(this.style.width) || 0;
844+
const height = parseFloat(this.style.height) || 0;
845+
return {
846+
width: width + snapError,
847+
height: height + snapError,
848+
top: 0, left: 0, right: width + snapError, bottom: height + snapError, x: 0, y: 0,
849+
toJSON: () => ({}),
850+
} as DOMRect;
851+
};
852+
853+
const { xScale, yScale } = Utils.getValuesFromTransformedElement(parent);
854+
855+
HTMLElement.prototype.getBoundingClientRect = originalGetBoundingClientRect;
829856
document.body.removeChild(parent);
857+
858+
// a 0.5px absolute snapping error is only a 0.05% relative deviation on a 1000px probe -
859+
// it would have been a 33% deviation (scale 0.667) on the old 1x1px probe
860+
expect(xScale).toBeCloseTo(1, 3);
861+
expect(yScale).toBeCloseTo(1, 3);
830862
});
831863
});
832864

@@ -936,11 +968,11 @@ describe('gridstack utils', () => {
936968
it('should handle scroll resize events', () => {
937969
// Test that the function exists and can be called
938970
expect(typeof Utils.updateScrollResize).toBe('function');
939-
971+
940972
// Simple test to avoid jsdom scrollBy issues
941973
const el = document.createElement('div');
942974
const mockEvent = { clientY: 50 } as MouseEvent;
943-
975+
944976
// The function implementation involves DOM scrolling which is complex in jsdom
945977
// We just verify it's callable without extensive mocking
946978
try {
@@ -951,4 +983,5 @@ describe('gridstack utils', () => {
951983
}
952984
});
953985
});
986+
954987
});

‎src/gridstack.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3129,9 +3129,13 @@ export class GridStack {
31293129
// Scrolling page if needed
31303130
Utils.updateScrollResize(event, el, cellHeight);
31313131

3132-
// get new size
3133-
p.w = Math.round((ui.size!.width - mLeft) / cellWidth);
3134-
p.h = Math.round((ui.size!.height - mTop) / cellHeight);
3132+
// get new size - only re-derive the dimension(s) the active handle actually changes.
3133+
// re-deriving an untouched dimension from measured pixels is lossy (subpixel/rounding noise)
3134+
// and can drift it by a row/column on fine grids where cellWidth/cellHeight are only a few
3135+
// pixels (e.g. high column count + cellHeight:1). #3230
3136+
const dir = event.resizeDir || '';
3137+
p.w = (dir.indexOf('e') > -1 || dir.indexOf('w') > -1) ? Math.round((ui.size!.width - mLeft) / cellWidth) : node.w!;
3138+
p.h = (dir.indexOf('n') > -1 || dir.indexOf('s') > -1) ? Math.round((ui.size!.height - mTop) / cellHeight) : node.h!;
31353139
if (node.w === p.w && node.h === p.h) return;
31363140
if (node._lastTried && node._lastTried.w === p.w && node._lastTried.h === p.h) return; // skip one we tried (but failed)
31373141

‎src/utils.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -693,23 +693,29 @@ export class Utils {
693693
* returns the scale and offsets from said element
694694
*/
695695
public static getValuesFromTransformedElement(parent: HTMLElement): DragTransform {
696+
// use a large probe rather than 1x1px: getBoundingClientRect() reflects the browser's
697+
// device-pixel-snapped render, and that snapping error (up to ~1 device pixel) is a huge
698+
// relative error on a 1px probe (can be several %) but negligible on a large one, at
699+
// fractional OS/browser zoom (125%, 150%,...). A wrong scale here directly corrupts
700+
// drag/resize pixel->grid-unit math downstream. #3230
701+
const probeSize = 1000;
696702
const transformReference = document.createElement('div');
697703
Utils.addElStyles(transformReference, {
698704
opacity: '0',
699705
position: 'fixed',
700706
top: 0 + 'px',
701707
left: 0 + 'px',
702-
width: '1px',
703-
height: '1px',
708+
width: probeSize + 'px',
709+
height: probeSize + 'px',
704710
zIndex: '-999999',
705711
});
706712
parent.appendChild(transformReference);
707713
const transformValues = transformReference.getBoundingClientRect();
708714
parent.removeChild(transformReference);
709715
transformReference.remove();
710716
return {
711-
xScale: 1 / transformValues.width,
712-
yScale: 1 / transformValues.height,
717+
xScale: probeSize / transformValues.width,
718+
yScale: probeSize / transformValues.height,
713719
xOffset: transformValues.left,
714720
yOffset: transformValues.top,
715721
}

0 commit comments

Comments
 (0)