#2667 Fixed sidebar items not honouring gs-w#2988
Merged
adumesny merged 1 commit intogridstack:masterfrom Mar 16, 2025
Merged
Conversation
…ragging between multiple grids. Fixed gs-w and gs-h attributes being written back to original sidebar item
adumesny
reviewed
Mar 16, 2025
| this._writePosAttr(target, node); | ||
| // Do not write sidebar item attributes back to the original sidebar el | ||
| if (!node._sidebarOrig) { | ||
| this._writePosAttr(target, node); |
Member
There was a problem hiding this comment.
good fix to not write sidebar back, but still seems ,like we need to write to the dropped item... will have to debug.
adumesny
reviewed
Mar 16, 2025
| /** @internal true if we should remove DOM element on _notify() rather than clearing _id (old way) */ | ||
| _removeDOM?: boolean; | ||
| /** @internal original position/size of item if dragged from sidebar */ | ||
| _sidebarOrig?: GridStackPosition; |
Member
There was a problem hiding this comment.
reluntant to add yet another data storage, but maybe this makes it clearer...
adumesny
approved these changes
Mar 16, 2025
Member
adumesny
left a comment
There was a problem hiding this comment.
thanks for the fix. dragging between 2 different restricted column count wasn't handled so this is good.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
fix #2667
These changes fix sidebar items not honoring gs-w and gs-h when dragging between multiple grids. When dragging between multiple grids, the size of the widget is limited by nodeBoundFix() based on number of columns. Eg. if you enter a grid which limits the gs-w attribute then enter another grid, I would expect the gs-w to honor the original size defined in the sidebar item, but instead it keeps the value which was limited in the previous grid.
These changes add another property for tracking the sidebar item's original position, so any number of grids can be entered and it still respects the original size, only limiting gs-w when appropriate. The existing _gridstackNodeOrig logic was not sufficient to handle the different behavior of the sidebar item.
These changes also fix another bug I discovered, which can be seen in the original example sandbox provided in #2667 . writePosAttr was writing the updated values to the original sidebar element, permanently altering gs-w, so any subsequent drags of the item would use the updated value. Sidebar items should not be mutable in this way.
Checklist
yarn test)