Skip to content

Remove added node from layout cache on drop event #2461

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

Merged
merged 1 commit into from
Sep 27, 2023
Merged

Remove added node from layout cache on drop event #2461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions src/gridstack-engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,17 @@ export class GridStackEngine {
return this._layouts?.[column]?.findIndex(l => l._id === n._id) ?? -1;
}

public removeNodeFromLayoutCache(n: GridStackNode) {
if (!this._layouts) {
return;
}
for (let i = 0; i < this._layouts.length; i++) {
let index = this.findCacheLayout(n, i);
if (index !== -1) {
this._layouts[i].splice(index, 1);
}
}
}

/** called to remove all internal values but the _id */
public cleanupNode(node: GridStackNode): GridStackEngine {
Expand Down
1 change: 1 addition & 0 deletions src/gridstack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2135,6 +2135,7 @@ export class GridStack {
this._updateContainerHeight();
this.engine.addedNodes.push(node);// @ts-ignore
this._triggerAddEvent();// @ts-ignore
this.engine.removeNodeFromLayoutCache(node);
Copy link
Member

@adumesny adumesny Sep 27, 2023

Choose a reason for hiding this comment

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

this doesn't make sense to me as new grid target should not have that node to start with in it's cache.
instead line 2092 oGrid.engine.removeNodeFromLayoutCache(node); should be called to remove when LEAVING that grid.
I'll pull your changes and make the correct fix.

Copy link
Member

@adumesny adumesny Sep 27, 2023

Choose a reason for hiding this comment

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

nevermind I stand corrected. the issue is dropping an item that is larger into a grid - should it remember the width it wants to be and reloading should handle out of bound sizes (that is a bug) ? visually you are mathcing the wanted size but it w=2 outer grid vs w=3 inner grid. sicne it's a drag we visually match them (closest possible) so I'm ok with dropping the original width.

Copy link
Member

@adumesny adumesny Sep 27, 2023

Choose a reason for hiding this comment

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

the problem with your fix is that if an item wants to be w=3 (from sidebar with attributes values) but dropped into a 1 column, then you will forever loose the fact it wants to get widget if grid ever goes to 2,3,4 columns

the real fix is to make sure loading w=3 into a subgrip does the right thing - which IT DOES in v9.2.1 I tested with my example (based on your bug). So I will remove it.

this._triggerChangeEvent();

this.engine.endUpdate();
Expand Down