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

fitToContent fixes #2412

Merged
merged 1 commit into from
Aug 23, 2023
Merged

fitToContent fixes #2412

merged 1 commit into from
Aug 23, 2023

Conversation

adumesny
Copy link
Member

@adumesny adumesny commented Aug 23, 2023

Description

more fix #404

  • changing column size and manually resizing a widget will now call resizeToContent()
  • added resizeToContentCB for app to override how to resize.
  • Added resizeToContentParent to make generic code more robust.
  • fixed some 1 column loading Nan issues

Checklist

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

* changing column size and manually resizing a widget will now call resizeToContent()
* added resizeToContentCB for app to override how to resize.
* Added `resizeToContentParent` to make generic code more robust.
* fixed some 1 column loading Nan issues
@adumesny adumesny changed the title more fix #404 fitToContent fixes Aug 23, 2023
@adumesny adumesny merged commit 3131015 into gridstack:master Aug 23, 2023
@JonSohn
Copy link

JonSohn commented Aug 23, 2023

Hi, I really needed this feature so I already tested it. However, I ran into a few issues that I wanted to share with you, as it might help:

  1. right now the fitToContent function only works if you have an additional container in the grid-stack-item-content element. This is because of the firstChild call in the resizeToContent function which can be a Textnode or Comment due to spacing inside the dom. It is also addressed here in a sidenote. A possible solution would be to use firstElementChild instead? This will still require a container, but it allows spacing between the elements.
  2. I ran into an issue where a single element in the grid is not resized, even when configured to fitToContent. I traced it back to the doContentResize function. The problem is caused by the iteration that the nodes go through in the engine. This is because within the loop each node is resized and moved, so during the iteration sometimes the nodes change their place in the array and are not processed when they are moved down in the array. A possible solution would be to call resizeToContent with an array instead of individual nodes, or you could copy the array for iteration.

I hope I could help with this. If you have any questions, feel free to come back to me.

P.S: I don't usually work with GitHub. If this is the wrong place or I didn't follow something within the guidelines, please let me know.

@adumesny
Copy link
Member Author

adumesny commented Aug 23, 2023

@JonSohn great and thank you for finding the issues. I just released v9.0.0 but will incorporate those fixes in a review soon...

adumesny added a commit to adumesny/gridstack.js that referenced this pull request Aug 23, 2023
* based on review gridstack#2412
* now have clone list, and using firstElementChild checking for NULL for correct sizing
@adumesny adumesny mentioned this pull request Aug 23, 2023
3 tasks
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.

make widget auto-size height to fit content (no vertical scrollbar)
2 participants