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

Add resize event listener #1616

Merged
merged 1 commit into from
Feb 20, 2021
Merged

Add resize event listener #1616

merged 1 commit into from
Feb 20, 2021

Conversation

MrCorba
Copy link
Contributor

@MrCorba MrCorba commented Feb 12, 2021

Description

Added an on resize listener to get the size of the node while resizing

Checklist

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

Copy link
Member

@adumesny adumesny left a comment

Choose a reason for hiding this comment

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

thank you for making this suggestion! Please make the corrections listed.

demo/events.js Outdated
@@ -46,6 +46,10 @@ function addEvents(grid, id) {
console.log(g + 'resizestart ' + el.textContent + ' size: (' + w + ' x ' + h + ')');
});

grid.on('resize', function(event, newWidget) {
Copy link
Member

Choose a reason for hiding this comment

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

make it el to be consistent with others and pass the DOM element, not the node...

doc/README.md Outdated
called while the user is resizing an item, with current node attributes

```js
grid.on('resize', function(event: Event, item: GridStackNode) {
Copy link
Member

Choose a reason for hiding this comment

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

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)

@@ -449,6 +449,15 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
this.engine.moveNode(node, x, y, w, h);
if (resizing && node.subGrid) { (node.subGrid as GridStack).onParentResize(); }
this._updateContainerHeight();

if (this._gsEventHandler[event.type]) {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed it to use the target, but I had to use _writePosAttr to get the current attributes while resizing

src/gridstack.ts Outdated
@@ -21,7 +21,7 @@ export interface GridHTMLElement extends HTMLElement {
gridstack?: GridStack; // grid's parent DOM element points back to grid class
}
export type GridStackEvent = 'added' | 'change' | 'disable' | 'dragstart' | 'dragstop' | 'dropped' |
'enable' | 'removed' | 'resizestart' | 'resizestop';
'enable' | 'removed' | 'resizestart' | 'resizestop' | 'resize';
Copy link
Member

Choose a reason for hiding this comment

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

alphabetize please

Copy link
Member

@adumesny adumesny left a comment

Choose a reason for hiding this comment

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

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....

@@ -19,6 +19,13 @@ function addEvents(grid, id) {
console.log(g + 'dragstart ' + el.textContent + ' pos: (' + node.x + ',' + node.y + ') vs (' + x + ',' + y + ')');
});

grid.on('drag', function(event, el) {
let node = el.gridstackNode;
let x = el.getAttribute('gs-x');
Copy link
Member

Choose a reason for hiding this comment

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

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...

demo/events.js Outdated
@@ -46,6 +53,12 @@ function addEvents(grid, id) {
console.log(g + 'resizestart ' + el.textContent + ' size: (' + w + ' x ' + h + ')');
});

grid.on('resize', function(event, el) {
let w = el.getAttribute('gs-w');
Copy link
Member

Choose a reason for hiding this comment

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

add let node = el.gridstackNode; (which is simpler to use really)

doc/README.md Outdated
@@ -238,6 +249,15 @@ grid.on('resizestart', function(event: Event, el: GridItemHTMLElement) {
});
```

### resize(event, el)

called while the user is resizing an item, with current node attributes
Copy link
Member

Choose a reason for hiding this comment

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

remove "with current node attributes"

@@ -449,6 +449,13 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
this.engine.moveNode(node, x, y, w, h);
if (resizing && node.subGrid) { (node.subGrid as GridStack).onParentResize(); }
this._updateContainerHeight();

if (this._gsEventHandler[event.type]) {
let target: GridItemHTMLElement = event.target as GridItemHTMLElement;
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Target == node.el, but from node.el you can't go back to el.gridstackNode. Probably because it's a recursive reference.

Copy link
Member

Choose a reason for hiding this comment

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

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.

@MrCorba
Copy link
Contributor Author

MrCorba commented Feb 16, 2021

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.

@@ -452,6 +453,11 @@ GridStack.prototype._prepareDragDropByNode = function(node: GridStackNode): Grid
if (resizing && node.subGrid) { (node.subGrid as GridStack).onParentResize(); }
this._updateContainerHeight();
}
this._writePosAttr(target, node.x, node.y, node.w, node.h);
Copy link
Member

@adumesny adumesny Feb 16, 2021

Choose a reason for hiding this comment

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

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 :)

@adumesny adumesny merged commit 7d3d4c5 into gridstack:develop Feb 20, 2021
@adumesny
Copy link
Member

thanks, I will tweak it further.

adumesny added a commit to adumesny/gridstack.js that referenced this pull request Feb 20, 2021
fixes to gridstack#1616 for new drag|resize event - only call when they change.
Updated docs
@adumesny adumesny mentioned this pull request Feb 20, 2021
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.

None yet

2 participants