Resizing issue #1530

hbcarlos · 2020-12-09T13:38:59Z

Description

As discussed in #1001 the resizing with scrolling is not working.
I'm trying to solve it here but I'm having some troubles. Now is working fine but I'm resizing the layout adding some rows on resizestart because when while resizing an item there is a back and force with the number of cells creating a strange behaviour.

Checklist

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

…nto develop

adumesny

thank you for doing this! but minRow + 100 is a big no. Ping me if you need more help on fixing right way.

adumesny · 2020-12-12T17:14:23Z

src/utils.ts

+  /** @internal */
+  static updateScrollResize(event: MouseEvent, el: HTMLElement, distance: number): void {
+    let scrollEl = this.getScrollParent(el);
+    if (scrollEl === null) return;


if (!scrollEl ) return;

adumesny · 2020-12-12T17:14:49Z

src/utils.ts

+      scrollEl.scrollBy({ behavior: 'smooth', top: event.clientY - distance});
+    }
+
+    if (bottom) {


else if (bottom)

adumesny · 2020-12-12T17:17:25Z

src/gridstack-dd.ts


+    // Saving `minRow` and adding new ones for resizing
+    minRow = this.opts.minRow;
+    this.opts.minRow = this.getRow() + 100;


big no no - don't arbitrary add 100 rows!
we should only add 1 extra row (when computing the needed rows) IFF the item you are resizing/moving is at the bottom (node.y+node.h). don't need to mod minRow for this...

adumesny · 2020-12-12T17:18:51Z

src/utils.ts

    }
  }
+
+  /** @internal */


put a real comment on your code. seems reasonable given we do something similar (though quite different code. I will have to diff into the scroll as I didn't write that)

src/gridstack-dd.ts

adumesny · 2020-12-12T17:22:17Z

src/h5/dd-resizable.ts

  /** @internal */
  private temporalRect: Rect;
  /** @internal */
+  private scrollY: number;


do we need that in JQ version as well ?

adumesny · 2020-12-12T17:23:34Z

src/h5/dd-resizable.ts

  private _resizeStart(event: MouseEvent): DDResizable {
    this.originalRect = this.el.getBoundingClientRect();
+    const scrollEl = Utils.getScrollParent(this.el);
+    this.scrollY = scrollEl === null ? 0 : scrollEl.scrollTop;


this.scrollY = scrollEl ? scrollEl.scrollTop : 0;

adumesny · 2020-12-12T17:24:35Z

src/h5/dd-resizable.ts

  private _getChange(event: MouseEvent, dir: string): Rect {
    const oEvent = this.startEvent;
+    const scrollEl = Utils.getScrollParent(this.el);
+    const scrolled = scrollEl === null ? 0 : (scrollEl.scrollTop - this.scrollY);


similar change

adumesny · 2020-12-12T17:26:45Z

src/h5/dd-resizable.ts

  /** @internal */
  private _resizeStart(event: MouseEvent): DDResizable {
    this.originalRect = this.el.getBoundingClientRect();
+    const scrollEl = Utils.getScrollParent(this.el);


cache this since you need it in all 3 methods and continuously, or does it change over time ?

This changes over time. When there is no scroll because the layout is small scrollEl is null and eventually when the layout grows returns the scrollable element.

right, so you could scrollEl = scrollEl || Utils.getScrollParent(this.el) and cache it once it appears, unless it changes values ?
quick looking it appears to go up recursively so maybe it could be different elements over time... so maybe not.

hbcarlos · 2020-12-14T11:14:33Z

Thanks for the tips @adumesny. I need some help with the placeholder and the view of the item while resizing.

There is two case that I'm not sure how to solve.
As you can see in the next gif, when the scrolling starts the item position changes.

When the item is at the bottom and the mouse pointer goes out of the grid-stack element while resizing, the Utils.getScrollParent(this.el); returns the grid-stack element and not his parent.

adumesny · 2020-12-14T17:04:54Z

I'll take a look in the next couple days... I just spend entire weekend re-writing collision so need to take a break from gridstack

hbcarlos · 2020-12-28T18:13:52Z

The resize is working. The problem was the scrollable parent, sometimes got lost.

…nto develop

adumesny · 2021-01-03T01:12:48Z

sorry, taking a look now. I had to fix some Safari mac issues and released I also tweaked on we do resizing (event handling really).

adumesny

that looks reasonable (minor doc comments). Can you update to the latest dev code so I can push it in ?
I need to cherry pick as well to verify it works. Have you tried how tat affects the JQ version as well ?

adumesny · 2021-01-03T01:55:15Z

src/utils.ts

-    } else if (el.scrollHeight > el.clientHeight) {
-      returnEl = el;
+    if (el === null) return document.documentElement;
+    const style = getComputedStyle(el);


is getComputedStyle() better than checking the simple el.scrollHeight > el.clientHeight because you want to remember what parent MIGHT scroll later ? (in case you don't already scroll ?) - please add comment.

Yes, also with el.scrollHeight > el.clientHeight sometimes is selecting a non scrollable parent.

interesting. didn't think that would be possible... but I realize the call you have return the first that could scroll which is safer anyway... but always return the doc as least, which might not scroll so it should return NULL and have that be handled ideally...

Exactly, the best option would be returning NULL when there isn't a scrollable parent but in this case, we need to prevent gridstack to continue growing.

I was using the example float.html to check my changes. I think in this example the ideal would be that gridstack stops growing when there is no more page because the element that is scrolling is the document not the container that wraps gridstack.

And we need to manually prevent gridstack to grow because:
In my case, I have two screens one on top of another and I have the browser with the webpage with gridstack on the screen above.

When I try to resize or drag and drop an item to the bottom my mouse continue on the screen bellow so gridstack still growing even when the page can't scroll. Once I stop there is no scrollable parent so the documentElement is resized to wrap everything.

well we have a maxRow to prevent grid from growing past that size, but you are saying that if none of the parents can scroll (by default divs will auto scroll unless you have overflow: hidden or something) then we should stop growing so we don't clip, which makes sense... Looks like you are checking for overflow | overflow-y for and auto or scroll
you don't need 2 screens to test this. browser smaller than full screen will do that too...

if you want to take a stab at it that would be great. _prepareDragDropByNode() should cache if we can not v-scroll and prevent move/size past the max somehow.

I'll take a look at this over the week (probably weekend, I'm quite busy right now) :)

src/utils.ts

adumesny · 2021-01-03T02:14:05Z

src/h5/dd-resizable.ts

  /** @internal */
  private _resizeStart(event: MouseEvent): DDResizable {
    this.originalRect = this.el.getBoundingClientRect();
+    this.scrollEl = Utils.getScrollParent(this.el);


reset this in destroy()

hbcarlos · 2021-01-03T06:45:58Z

sorry, taking a look now. I had to fix some Safari mac issues and released I also tweaked on we do resizing (event handling really).

No problem at all.

Thanks for merging and sorry for the comments, I can open a PR to add some comments if you want.

adumesny · 2021-01-03T07:00:29Z

no need, I took care of it I believe. thanks for fixing this issue!

hbcarlos added 2 commits December 9, 2020 14:20

Resizing issue

565b31a

Merge branch 'develop' of https://github.com/gridstack/gridstack.js i…

1d3b40c

…nto develop

hbcarlos marked this pull request as draft December 9, 2020 13:39

hbcarlos marked this pull request as ready for review December 9, 2020 23:39

adumesny reviewed Dec 12, 2020

View reviewed changes

adumesny marked this pull request as draft December 12, 2020 17:43

review changes

5ba15dc

Select scrollable parent

84cec8d

Merge branch 'develop' of https://github.com/gridstack/gridstack.js i…

2ec824c

…nto develop

hbcarlos marked this pull request as ready for review December 28, 2020 18:26

adumesny reviewed Jan 3, 2021

View reviewed changes

adumesny marked this pull request as draft January 3, 2021 02:20

adumesny marked this pull request as ready for review January 3, 2021 02:20

adumesny merged commit 3bcf007 into gridstack:develop Jan 3, 2021

Uh oh!

Resizing issue #1530

Resizing issue #1530

Uh oh!

Conversation

hbcarlos commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

adumesny left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adumesny Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hbcarlos commented Dec 14, 2020

Uh oh!

adumesny commented Dec 14, 2020

Uh oh!

hbcarlos commented Dec 28, 2020

Uh oh!

adumesny commented Jan 3, 2021

Uh oh!

adumesny left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hbcarlos Jan 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hbcarlos commented Jan 3, 2021

Uh oh!

adumesny commented Jan 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hbcarlos commented Dec 9, 2020 •

edited

Loading

adumesny left a comment •

edited

Loading

adumesny Dec 14, 2020 •

edited

Loading

hbcarlos Jan 3, 2021 •

edited

Loading