-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
Default columns arg for non-12 columns scenarios #2851
Conversation
…or non-12 scenarios
@@ -31,6 +31,7 @@ export class GridStackEngine { | |||
public addedNodes: GridStackNode[] = []; | |||
public removedNodes: GridStackNode[] = []; | |||
public batchMode: boolean; | |||
public defaultColumn = 12; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think instead of having a 'hard coded' max layout of 12, we can just use the largest size in the layout as the current max. will have to check code again...
| With this fix it is then possible to subclass GridStackEngine and change the default "max" column, rather having the constants hard-coded in the codebase. we shouldn't need to subclass to set the 'max column' layout saving. will add an option of this instead...or infer it from max layout size info. |
expect(findNode(engine, "left")).toEqual(jasmine.objectContaining({x: 0, y: 0, w: 1, h: 1})); | ||
expect(findNode(engine, "right")).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 1, h: 1})); | ||
// Resize back to 24 column | ||
engine.column = 24; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that this would work BEFORE your changes too as you are calling engine.columnChanged() directly with 24 and defaultColumn is not updated to match
@lmartorella this works so not sure we needed this code after all... we save the 24 layout info and reuse it anyway... maybe some corner case adding a single larger item with scaling would be affected, not sure... this code handled large layouts already... load() {
....
if (maxColumn > column) {
this._ignoreLayoutsNodeChange = true; // skip layout update
this.engine.cacheLayout(items, maxColumn, true);
}
} test that run ok it('24 layout in 12 column to 1 back 12 then 24', function() {
const children: GridStackWidget[] = [{ x: 0, y: 0, w: 12, h: 1, id: 'left' }, { x: 12, y: 0, w: 12, h: 1, id: 'right' }];
children.forEach((c, i) => c.content = c.id);
grid = GridStack.init({children});
const left = find('left');
const right = find('right');
// side-by-side components 12+12 = 24 columns but only have 12!
expect(left).toEqual(jasmine.objectContaining({x: 0, y: 0, w: 12}));
expect(right).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 12}));
// Resize to 1 column
grid.column(1);
expect(left).toEqual(jasmine.objectContaining({x: 0, y: 0, w: 1}));
expect(right).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 1}));
// Resize back to 12 column
grid.column(12);
expect(left).toEqual(jasmine.objectContaining({x: 0, y: 0, w: 12}));
expect(right).toEqual(jasmine.objectContaining({x: 0, y: 1, w: 12}));
// Resize to 24 column
grid.column(24);
expect(left).toEqual(jasmine.objectContaining({x: 0, y: 0, w: 12}));
expect(right).toEqual(jasmine.objectContaining({x: 12, y: 0, w: 12}));
}); |
* for gridstack#2851 make sure to update the value and added some better testing (grid, not engine that behaves differently).
Description
Some applications can use default column number (for the largest resolutions) higher than 12.
In that case, during column count dynamic switch due to responsive-based resizes, it can happen that the
nodeBoundFix
function would try to cap the coordinates to 12 rather than the default max column number.With this fix it is then possible to subclass
GridStackEngine
and change the default "max" column, rather having the constants hard-coded in the codebase.Thx, L.
Checklist
yarn test
) (unfortunately the main trunk is not stable at the moment)