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.

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

Can't shrink item when grid is full (when using max row size) #2449

Closed
ashawthing opened this issue Sep 7, 2023 · 7 comments · Fixed by #2451
Closed

Can't shrink item when grid is full (when using max row size) #2449

ashawthing opened this issue Sep 7, 2023 · 7 comments · Fixed by #2451
Labels

Comments

@ashawthing
Copy link

Subject of the issue

I have set row = 34 and added enough items to fill the screen. The next one I add is off the screen. I then try to shrink the previous items to make room for the new one to move onto the screen.

However, resizing seems to be disabled.

Your environment

9.0.2

Steps to reproduce

You MUST provide a working demo - keep it simple and avoid frameworks as that could have issues - you can use
https://jsfiddle.net/adumesny/jqhkry7g

Expected behavior

I would expect reducing the size of an item to still work. Seems reasonable that you can't make anything bigger.

@ashawthing
Copy link
Author

ashawthing commented Sep 7, 2023

I've added a very crude fix which gets me going for now. Updated moveNodeCheck() to do the simple case moveNode if the size is smaller - because we know it will be no worse making it smaller.

  public moveNodeCheck(node: GridStackNode, o: GridStackMoveOpts): boolean {
    // if (node.locked) return false;
    if (!this.changedPosConstrain(node, o)) return false;
    o.pack = true;

    // simpler case: move item directly...
    if (!this.maxRow) {
      return this.moveNode(node, o);
    }

    // if shrinking can do simpler case
    if (o.x === node.x && o.y === node.y && o.w <= node.w && o.h <= node.h) return this.moveNode(node, o);

@adumesny
Copy link
Member

adumesny commented Sep 7, 2023

| The next one I add is off the screen
that is the real problem - after that all bets are off and not surprised things fail... your workaround is just a bandaid.

Please provide a running example as bug requires to validate and debug the underlying issue.

@ashawthing
Copy link
Author

I found a better fix, I changed this

    // check if we're covering 50% collision and could move
    let canMove = clone.moveNode(clonedNode, o) && clone.getRow() <= this.maxRow;

to this

    // check if we're covering 50% collision and could move
    let canMove = clone.moveNode(clonedNode, o) && clone.getRow() <= Math.max(this.getRow(), this.maxRow);

It says you can move and resize as long as it doesn't make the situation worse (i.e. taller than current max or actual max).

I'll see if I can share an example in a sec.

@ashawthing
Copy link
Author

This will do it. Use the button to add a widget and it will break the max row restriction. I'd then like to shrink the width of panel 1 so I can move the new panel into a valid position.

https://jsfiddle.net/bxk2g9zr/

@adumesny
Copy link
Member

adumesny commented Sep 7, 2023

again a bandaid. the out of bound widget should never have been added in the first place..., but this is resonable workaround to fix a bad situation...

Can you provide a code changelist, with better comments saying not making it worse and refering to this bug. thanks.

ashawthing added a commit to ashawthing/gridstack.js that referenced this issue Sep 8, 2023
@ashawthing
Copy link
Author

I've created a pull request for my change. I hope that is what you wanted. I'm not an expert when it comes to GIT my team uses TFS.

adumesny added a commit to adumesny/gridstack.js that referenced this issue Sep 8, 2023
* fix gridstack#2449
* move, while still being under maxRow or at least not making it worse (case where widget was somehow added past our max)
@adumesny adumesny mentioned this issue Sep 8, 2023
3 tasks
@adumesny
Copy link
Member

adumesny commented Sep 8, 2023

it's ok thanks. I pushed in the fix as that was a commit and not a review for me to approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants