Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- Fixed test compilation #2850

Merged
merged 4 commits into from
Nov 16, 2024
Merged

- Fixed test compilation #2850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e05aa87
- Fixed test compilation
lmartorella Nov 13, 2024
cd1f6e8
- Fixed ID being now forced to be a string
lmartorella Nov 13, 2024
9c1a295
- Fixed two cases
lmartorella Nov 13, 2024
47221a8
- Added infinite loop guard, tests was blocked due to this
lmartorella Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ module.exports = function(config) {
// }
// }
// },
exclude: ["demo", "dist/ng"] // ignore dummy demo .ts files
exclude: ["demo", "dist/ng"], // ignore dummy demo .ts files
include: [
"./spec/**/*-spec.ts"
]
},

// base path that will be used to resolve all patterns (eg. files, exclude)
Expand Down
86 changes: 43 additions & 43 deletions spec/gridstack-engine-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ describe('gridstack engine', function() {
let e: any = GridStackEngine;
let w: any = window;

let findNode = function(engine, id) {
return engine.nodes.find((i) => i.id === id);
let findNode = function(engine: GridStackEngine, id: string) {
return engine.nodes.find(i => i.id === id);
};

it('should exist setup function.', function() {
Expand All @@ -23,9 +23,9 @@ describe('gridstack engine', function() {
engine = new GridStackEngine();
expect(engine.column).toEqual(12);
expect(engine.float).toEqual(false);
expect(engine.maxRow).toEqual(undefined);
expect(engine.maxRow).toEqual(undefined!);
expect(engine.nodes).toEqual([]);
expect(engine.batchMode).toEqual(undefined);
expect(engine.batchMode).toEqual(undefined!);
expect((engine as any).onChange).toEqual(undefined);
});

Expand Down Expand Up @@ -146,21 +146,21 @@ describe('gridstack engine', function() {
beforeAll(function() {
engine = new GridStackEngine({float:true});
engine.nodes = [
engine.prepareNode({x: 0, y: 0, id: 1, _dirty: true}),
engine.prepareNode({x: 3, y: 2, w: 3, h: 2, id: 2, _dirty: true}),
engine.prepareNode({x: 3, y: 7, w: 3, h: 2, id: 3})
engine.prepareNode({x: 0, y: 0, id: "1", _dirty: true}),
engine.prepareNode({x: 3, y: 2, w: 3, h: 2, id: "2", _dirty: true}),
engine.prepareNode({x: 3, y: 7, w: 3, h: 2, id: "3"})
];
});

beforeEach(function() {
delete engine.batchMode;
delete (engine as any).batchMode;
});

it('should return all dirty nodes', function() {
let nodes = engine.getDirtyNodes();
expect(nodes.length).toEqual(2);
expect(nodes[0].id).toEqual(1);
expect(nodes[1].id).toEqual(2);
expect(nodes[0].id).toEqual("1");
expect(nodes[1].id).toEqual("2");
});

it('should\'n clean nodes if batchMode true', function() {
Expand Down Expand Up @@ -230,9 +230,9 @@ describe('gridstack engine', function() {
spyOn(spy, 'callback');
engine = new GridStackEngine({float:true, onChange: spy.callback});
engine.nodes = [
engine.prepareNode({x: 0, y: 0, id: 1, _dirty: true}),
engine.prepareNode({x: 3, y: 2, w: 3, h: 2, id: 2, _dirty: true}),
engine.prepareNode({x: 3, y: 7, w: 3, h: 2, id: 3})
engine.prepareNode({x: 0, y: 0, id: "1", _dirty: true}),
engine.prepareNode({x: 3, y: 2, w: 3, h: 2, id: "2", _dirty: true}),
engine.prepareNode({x: 3, y: 7, w: 3, h: 2, id: "3"})
];
});

Expand Down Expand Up @@ -262,50 +262,50 @@ describe('gridstack engine', function() {

it('shouldn\'t pack one node with y coord eq 0', function() {
engine.nodes = [
{x: 0, y: 0, w:1, h:1, id: 1},
{x: 0, y: 0, w:1, h:1, id: "1"},
];
(engine as any)._packNodes();
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, h: 1}));
expect(findNode(engine, 1)._dirty).toBeFalsy();
expect(findNode(engine, "1")).toEqual(jasmine.objectContaining({x: 0, y: 0, h: 1}));
expect(findNode(engine, "1")!._dirty).toBeFalsy();
});

it('should pack one node correctly', function() {
engine.nodes = [
{x: 0, y: 1, w:1, h:1, id: 1},
{x: 0, y: 1, w:1, h:1, id: "1"},
];
(engine as any)._packNodes();
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, _dirty: true}));
expect(findNode(engine, "1")).toEqual(jasmine.objectContaining({x: 0, y: 0, _dirty: true}));
});

it('should pack nodes correctly', function() {
engine.nodes = [
{x: 0, y: 1, w:1, h:1, id: 1},
{x: 0, y: 5, w:1, h:1, id: 2},
{x: 0, y: 1, w:1, h:1, id: "1"},
{x: 0, y: 5, w:1, h:1, id: "2"},
];
(engine as any)._packNodes();
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 0, _dirty: true}));
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 1, _dirty: true}));
expect(findNode(engine, "1")).toEqual(jasmine.objectContaining({x: 0, y: 0, _dirty: true}));
expect(findNode(engine, "2")).toEqual(jasmine.objectContaining({x: 0, y: 1, _dirty: true}));
});

it('should pack nodes correctly', function() {
engine.nodes = [
{x: 0, y: 5, w:1, h:1, id: 1},
{x: 0, y: 1, w:1, h:1, id: 2},
{x: 0, y: 5, w:1, h:1, id: "1"},
{x: 0, y: 1, w:1, h:1, id: "2"},
];
(engine as any)._packNodes();
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 0, _dirty: true}));
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, _dirty: true}));
expect(findNode(engine, "2")).toEqual(jasmine.objectContaining({x: 0, y: 0, _dirty: true}));
expect(findNode(engine, "1")).toEqual(jasmine.objectContaining({x: 0, y: 1, _dirty: true}));
});

it('should respect locked nodes', function() {
engine.nodes = [
{x: 0, y: 1, w:1, h:1, id: 1, locked: true},
{x: 0, y: 5, w:1, h:1, id: 2},
{x: 0, y: 1, w:1, h:1, id: "1", locked: true},
{x: 0, y: 5, w:1, h:1, id: "2"},
];
(engine as any)._packNodes();
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 0, y: 1, h: 1}));
expect(findNode(engine, 1)._dirty).toBeFalsy();
expect(findNode(engine, 2)).toEqual(jasmine.objectContaining({x: 0, y: 2, _dirty: true}));
expect(findNode(engine, "1")).toEqual(jasmine.objectContaining({x: 0, y: 1, h: 1}));
expect(findNode(engine, "1")!._dirty).toBeFalsy();
expect(findNode(engine, "2")).toEqual(jasmine.objectContaining({x: 0, y: 2, _dirty: true}));
});
});
});
Expand Down Expand Up @@ -342,31 +342,31 @@ describe('gridstack engine', function() {
});
it('should add widgets around locked one', function() {
let nodes: GridStackNode[] = [
{x: 0, y: 1, w: 12, h: 1, locked: true, noMove: true, noResize: true, id: 0},
{x: 1, y: 0, w: 2, h: 3, id: 1}
{x: 0, y: 1, w: 12, h: 1, locked: true, noMove: true, noResize: true, id: "0"},
{x: 1, y: 0, w: 2, h: 3, id: "1"}
];
// add locked item
engine.addNode(nodes[0])
expect(findNode(engine, 0)).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 12, h: 1, locked: true}));
expect(findNode(engine, "0")).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 12, h: 1, locked: true}));
// add item that moves past locked one
engine.addNode(nodes[1])
expect(findNode(engine, 0)).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 12, h: 1, locked: true}));
expect(findNode(engine, 1)).toEqual(jasmine.objectContaining({x: 1, y: 2, h: 3}));
expect(findNode(engine, "0")).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 12, h: 1, locked: true}));
expect(findNode(engine, "1")).toEqual(jasmine.objectContaining({x: 1, y: 2, h: 3}));
// locked item can still be moved directly (what user does)
let node0 = findNode(engine, 0);
expect(engine.moveNode(node0, {y:6})).toEqual(true);
expect(findNode(engine, 0)).toEqual(jasmine.objectContaining({x: 0, y: 6, h: 1, locked: true}));
let node0 = findNode(engine, "0");
expect(engine.moveNode(node0!, {y:6})).toEqual(true);
expect(findNode(engine, "0")).toEqual(jasmine.objectContaining({x: 0, y: 6, h: 1, locked: true}));
// but moves regular one past it
let node1 = findNode(engine, 1);
expect(engine.moveNode(node1, {x:6, y:6})).toEqual(true);
let node1 = findNode(engine, "1");
expect(engine.moveNode(node1!, {x:6, y:6})).toEqual(true);
expect(node1).toEqual(jasmine.objectContaining({x: 6, y: 7, w: 2, h: 3}));
// but moves regular one before (gravity ON)
engine.float = false;
expect(engine.moveNode(node1, {x:7, y:3})).toEqual(true);
expect(engine.moveNode(node1!, {x:7, y:3})).toEqual(true);
expect(node1).toEqual(jasmine.objectContaining({x: 7, y: 0, w: 2, h: 3}));
// but moves regular one before (gravity OFF)
engine.float = true;
expect(engine.moveNode(node1, {x:7, y:3})).toEqual(true);
expect(engine.moveNode(node1!, {x:7, y:3})).toEqual(true);
expect(node1).toEqual(jasmine.objectContaining({x: 7, y: 3, w: 2, h: 3}));
});
});
Expand Down
10 changes: 5 additions & 5 deletions spec/gridstack-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ describe('gridstack', function() {
expect(parseInt(el2.getAttribute('gs-h'))).toBe(4);

// add default 1x1 item to the end (1 column)
let el3 = grid.addWidget();
let el3 = grid.addWidget({ });
expect(el3).not.toBe(null);
expect(parseInt(el3.getAttribute('gs-x'))).toBe(0);
expect(parseInt(el3.getAttribute('gs-y'))).toBe(6);
Expand Down Expand Up @@ -747,7 +747,7 @@ describe('gridstack', function() {
expect(grid.engine.nodes.length).toEqual(0);
expect(document.getElementById('item2')).toBe(null);

let el3 = grid.addWidget(widgetHTML);
let el3 = grid.makeWidget(widgetHTML);
expect(el3).not.toBe(null);
grid.removeWidget(el3, false);
expect(grid.engine.nodes.length).toEqual(0);
Expand Down Expand Up @@ -903,7 +903,7 @@ describe('gridstack', function() {
});
it('should autoPosition (empty options)', function() {
let grid = GridStack.init();
let widget = grid.addWidget();
let widget = grid.addWidget({ });

expect(parseInt(widget.getAttribute('gs-x'), 10)).toBe(8);
expect(parseInt(widget.getAttribute('gs-y'), 10)).toBe(0);
Expand Down Expand Up @@ -933,15 +933,15 @@ describe('gridstack', function() {
it('null options should clear x position', function() {
let grid = GridStack.init({float: true});
let HTML = '<div class="grid-stack-item" gs-x="9"><div class="grid-stack-item-content"></div></div>';
let widget = grid.addWidget(HTML, {x:null, y:null, w:undefined});
let widget = grid.makeWidget(HTML, {x:null, y:null, w:undefined});

expect(parseInt(widget.getAttribute('gs-x'), 10)).toBe(8);
expect(parseInt(widget.getAttribute('gs-y'), 10)).toBe(0);
});
it('width attr should be retained', function() { // #1276
let grid = GridStack.init({float: true});
let HTML = '<div class="grid-stack-item" gs-w="3" gs-max-w="4" gs-id="foo"><div class="grid-stack-item-content"></div></div>';
let widget = grid.addWidget(HTML, {x: 1, y: 5});
let widget = grid.makeWidget(HTML, {x: 1, y: 5});
expect(parseInt(widget.getAttribute('gs-x'), 10)).toBe(1);
expect(parseInt(widget.getAttribute('gs-y'), 10)).toBe(5);
expect(parseInt(widget.getAttribute('gs-w'), 10)).toBe(3);
Expand Down
4 changes: 2 additions & 2 deletions spec/utils-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe('gridstack utils', function() {
expect(Utils.parseHeight('12.5cm')).toEqual(jasmine.objectContaining({h: 12.5, unit: 'cm'}));
expect(Utils.parseHeight('12.5mm')).toEqual(jasmine.objectContaining({h: 12.5, unit: 'mm'}));
expect(Utils.parseHeight('12.5')).toEqual(jasmine.objectContaining({h: 12.5, unit: 'px'}));
expect(function() { Utils.parseHeight('12.5 df'); }).toThrowError('Invalid height');
expect(function() { Utils.parseHeight('12.5 df'); }).toThrowError('Invalid height val = 12.5 df');
});

it('should parse negative height value', function() {
Expand All @@ -94,7 +94,7 @@ describe('gridstack utils', function() {
expect(Utils.parseHeight('-12.3cm')).toEqual(jasmine.objectContaining({h: -12.3, unit: 'cm'}));
expect(Utils.parseHeight('-12.3mm')).toEqual(jasmine.objectContaining({h: -12.3, unit: 'mm'}));
expect(Utils.parseHeight('-12.5')).toEqual(jasmine.objectContaining({h: -12.5, unit: 'px'}));
expect(function() { Utils.parseHeight('-12.5 df'); }).toThrowError('Invalid height');
expect(function() { Utils.parseHeight('-12.5 df'); }).toThrowError('Invalid height val = -12.5 df');
});
});

Expand Down
4 changes: 4 additions & 0 deletions src/gridstack-engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@ export class GridStackEngine {

let didMove = false;
const newOpt: GridStackMoveOpts = {nested: true, pack: false};
let counter = 0;
while (collide = collide || this.collide(node, area, opt.skip)) { // could collide with more than 1 item... so repeat for each
if (counter++ > this.nodes.length * 2) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to see case where this happens before taking this workaround... will revert it locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually fixing a broken test, so it is easy to replicate:

Chrome Headless 131.0.0.0: gridstack grid.min/max width/height should set gs-min-w to 2. FAILED
Error: Infinite collide check

Without this safe check, the test suite is simply going in infinite loop causing "DISCONNECTED" on headless or a stuck browser when testing on real browser. In both cases, no results are produced:

Disconnected reconnect failed before timeout of 2000ms (ping timeout)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I fixed the infinite loop (had NaN for maxW) but left the check in there in case...

throw new Error("Infinite collide check");
}
let moved: boolean;
// if colliding with a locked item OR loading (move after) OR moving down with top gravity (and collide could move up) -> skip past the collide,
// but remember that skip down so we only do this once (and push others otherwise).
Expand Down
3 changes: 0 additions & 3 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
"strict": false,
"target": "ES2020"
},
"exclude": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that may force .spec.ts files to be in the npm build package which we don't want...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no .spec files in ./src, so this is safe to delete.

"./src/**/*.spec.ts",
],
"include": [
"./src/**/*.ts"
],
Expand Down