Add resize event listener#1616
Conversation
adumesny
left a comment
There was a problem hiding this comment.
thank you for making this suggestion! Please make the corrections listed.
| console.log(g + 'resizestart ' + el.textContent + ' size: (' + w + ' x ' + h + ')'); | ||
| }); | ||
|
|
||
| grid.on('resize', function(event, newWidget) { |
There was a problem hiding this comment.
make it el to be consistent with others and pass the DOM element, not the node...
| called while the user is resizing an item, with current node attributes | ||
|
|
||
| ```js | ||
| grid.on('resize', function(event: Event, item: GridStackNode) { |
There was a problem hiding this comment.
don't pass the node, pass the element (from which node can be accessed) to be consistent... then they can use the actual px width/height or node x,y (column based)
| if (resizing && node.subGrid) { (node.subGrid as GridStack).onParentResize(); } | ||
| this._updateContainerHeight(); | ||
|
|
||
| if (this._gsEventHandler[event.type]) { |
There was a problem hiding this comment.
looks like we should also add 'drag' event... also we need to consider if we want to call it for pixel changes, or just when column/row changes happen (what you have)
Right now you have it if only column/row changes and passing node copy (why clone the values ? just pass the node as it has already moved to new location...
I would pass node.el instead to be consistent with start/top events AND add drag event as well.
There was a problem hiding this comment.
I had a problem getting the Element with the updated values. I've overlooked the node.el, this does the trick.
I added it to column/row changes, because I think it makes the most sense regarding snapping. The moment it snaps to another grid you want an update and do something with that
There was a problem hiding this comment.
I've changed it to use the target, but I had to use _writePosAttr to get the current attributes while resizing
| } | ||
| export type GridStackEvent = 'added' | 'change' | 'disable' | 'dragstart' | 'dragstop' | 'dropped' | | ||
| 'enable' | 'removed' | 'resizestart' | 'resizestop'; | ||
| 'enable' | 'removed' | 'resizestart' | 'resizestop' | 'resize'; |
adumesny
left a comment
There was a problem hiding this comment.
thanks for the change.
trying to understand what you need "live" resizing events because if you are updating the content inside, the live picture is supposed to follow the mouse cursor NOT the grid snapping dimensions (which is what the placeholder does but is empty). I'm wondering if we asking for problem doing that....
|
|
||
| grid.on('drag', function(event, el) { | ||
| let node = el.gridstackNode; | ||
| let x = el.getAttribute('gs-x'); |
There was a problem hiding this comment.
actually I don't know why I have el.getAttribute('gs-x') vs node.x as they should be the same....maybe for debugging. I should make that clearer...
| }); | ||
|
|
||
| grid.on('resize', function(event, el) { | ||
| let w = el.getAttribute('gs-w'); |
There was a problem hiding this comment.
add let node = el.gridstackNode; (which is simpler to use really)
|
|
||
| ### resize(event, el) | ||
|
|
||
| called while the user is resizing an item, with current node attributes |
There was a problem hiding this comment.
remove "with current node attributes"
| this._updateContainerHeight(); | ||
|
|
||
| if (this._gsEventHandler[event.type]) { | ||
| let target: GridItemHTMLElement = event.target as GridItemHTMLElement; |
There was a problem hiding this comment.
is target != node.el ?
I need to think about are we calling about placeholder (which is grid constrained) or the actual content is that is freely being resized by the mouse (pixel following)... I would have to debug it and see what makes sense. if you are doing the live item when it won't match the mouse...
Also I would not conditionally call _writePosAttr() if you have an event as it's changes the behavior and hard to debug/test. Always do this (assuming we need that - which I don't believe we do on the placeholder as it need to reflect the new column/row sizing.
There was a problem hiding this comment.
Target == node.el, but from node.el you can't go back to el.gridstackNode. Probably because it's a recursive reference.
There was a problem hiding this comment.
node.el.gridstackNode should work - I should always have that circular link, but placeholder vs real widget being dragged/resized might be different. Again I'll have to look.
|
The problem I'm trying to solve with the live resizing events is that I want to verify the new widget size and do something accordingly. Let's say there is a widget with a chart that is 4x4. When the tile gets smaller than 4x2, the chart doesn't fit anymore. So I would like to display a message/give a red border from a certain size, to warn the user that the size is too small and the chart will be removed (or something like that) I know I can use min width/height to force minimal sizes, but I want the user to be able to make it smaller and deal with the consequences. |
| if (resizing && node.subGrid) { (node.subGrid as GridStack).onParentResize(); } | ||
| this._updateContainerHeight(); | ||
| } | ||
| this._writePosAttr(target, node.x, node.y, node.w, node.h); |
There was a problem hiding this comment.
this all needs to move inside the if (this.engine.moveNodeCheck(node, x, y, w, h)) {}
otherwise it didn't change (constrain).
Also I'm not sure we should call _writePosAttr() as wouldn't the gs-x and x attributes conflict (with x,y,width,height absolute position maybe winning as they are inline but could be an issue) as user can easily look at node.x values (easier) or actual pixel size of the sizing element that is not grid constrain.
thanks for going through those edits... as you can see maintaining a lib is no easy task :)
|
thanks, I will tweak it further. |
fixes to gridstack#1616 for new drag|resize event - only call when they change. Updated docs
Description
Added an on resize listener to get the size of the node while resizing
Checklist
yarn test)