Skip to content

fix: incorrect node._id check #3077

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

Merged

Conversation

FilipKujavec
Copy link
Contributor

Description

Fixed a mistake I made in this PR #3072
(node._id ?? false) still is falsy when node_id = 0

Checklist

  • [-] Created tests which fail without the change (if possible)
  • All tests passing (yarn test)
  • [-] Extended the README / documentation, if necessary

Also @adumesny should I create a new issue for the incorrect order mentioned in #3071?

@@ -417,7 +417,7 @@ export class GridStackEngine {
// remember it's position & width so we can restore back (1 -> 12 column) #1655 #1985
// IFF we're not in the middle of column resizing!
const saveOrig = (node.x || 0) + (node.w || 1) > this.column;
if (saveOrig && this.column < this.defaultColumn && !this._inColumnResize && !this.skipCacheUpdate && (node._id ?? false) && this.findCacheLayout(node, this.defaultColumn) === -1) {
if (saveOrig && this.column < this.defaultColumn && !this._inColumnResize && !this.skipCacheUpdate && node._id != null && this.findCacheLayout(node, this.defaultColumn) === -1) {
Copy link
Member

@adumesny adumesny Jun 23, 2025

Choose a reason for hiding this comment

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

_id is internal to me, so it should always be set, or undefined. weird that you are checking (and should be !== null).
I would have to check why it isn't set (and why null works in your case ?
maybe I can start _id at 1 instead...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is set, its just that a _id of 0 leads to it being falsy again as my earlier code was wrong. != null just checks that its not null or undefined. If its never going to be null !== undefined would work fine as well
image

@adumesny adumesny merged commit 9875352 into gridstack:master Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants