Skip to content

Commit 38c089d

Browse files
committed
* fix #3355 auto-scroll hands off to the outer container when the inner is pinned
1 parent f100643 commit 38c089d

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

‎doc/CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ Change log
149149
<!-- END doctoc generated TOC please keep comment here to allow auto update -->
150150

151151
## 14.0.1 (TBD)
152+
* fix: [#3355](https://github.com/gridstack/gridstack.js/issues/3355) auto-scroll hands off to the outer container when a nested scroll container is pinned
152153
* fix: [#2729](https://github.com/gridstack/gridstack.js/issues/2729) `cancel` selector now matches elements inside a shadow root
153154
* fix: [#2728](https://github.com/gridstack/gridstack.js/issues/2728) drag helper drifts off the cursor when the page scrolls under a CSS-transformed containing block
154155
* fix: [#1959](https://github.com/gridstack/gridstack.js/issues/1959) `update()` refuses a move that would land on a locked item

‎spec/regression-spec.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,4 +1008,76 @@ describe('regression >', () => {
10081008
expect(dragStarted(host.querySelector('.grid-stack-item-content') as HTMLElement)).toBe(true);
10091009
});
10101010
});
1011+
1012+
describe('3355 nested scroll containers dead-end the drag >', () => {
1013+
let inner: HTMLElement, outer: HTMLElement, item: HTMLElement;
1014+
afterEach(() => {
1015+
vi.restoreAllMocks();
1016+
document.getElementById('gs-cont')?.remove();
1017+
});
1018+
1019+
/** an item inside a scroller inside another scroller, each with its own limits */
1020+
const nest = (innerAtLimit: boolean) => {
1021+
document.body.insertAdjacentHTML('afterbegin',
1022+
'<div id="gs-cont"><div class="outer"><div class="inner">' +
1023+
'<div class="grid-stack-item"><div class="grid-stack-item-content">x</div></div>' +
1024+
'</div></div></div>');
1025+
outer = document.querySelector('.outer') as HTMLElement;
1026+
inner = document.querySelector('.inner') as HTMLElement;
1027+
item = document.querySelector('.grid-stack-item') as HTMLElement;
1028+
1029+
const mk = (el: HTMLElement, top: number, max: number) => {
1030+
const box = {top};
1031+
Object.defineProperty(el, 'scrollTop', {
1032+
get: () => box.top,
1033+
set: (v: number) => { box.top = Math.max(0, Math.min(v, max)); },
1034+
configurable: true,
1035+
});
1036+
return box;
1037+
};
1038+
const innerBox = mk(inner, innerAtLimit ? 500 : 0, 500);
1039+
const outerBox = mk(outer, 0, 500);
1040+
1041+
const dd = DDElement.init(item as GridItemHTMLElement).setupDraggable({}).ddDraggable!;
1042+
const self = dd as unknown as {
1043+
helper?: HTMLElement; _autoScrollContainer?: HTMLElement; _autoScrollMaxSpeed?: number;
1044+
_getClipping(el: HTMLElement, s: HTMLElement): number;
1045+
_autoScrollTick(): void; dragging?: boolean;
1046+
};
1047+
self.helper = item;
1048+
self._autoScrollContainer = inner;
1049+
self._autoScrollMaxSpeed = 10;
1050+
// pretend the helper is hanging below the visible area so we want to scroll DOWN
1051+
vi.spyOn(self as unknown as Record<string, () => number>, '_getClipping').mockReturnValue(40);
1052+
// getScrollElement walks up and should find `outer` above `inner`
1053+
vi.spyOn(Utils, 'getScrollElement').mockImplementation(() => outer);
1054+
return { dd, self, innerBox, outerBox };
1055+
};
1056+
1057+
it('hands off to the outer container when the inner one is pinned', () => {
1058+
const { self, innerBox, outerBox } = nest(true);
1059+
expect(innerBox.top).toBe(500); // already at its limit
1060+
1061+
self._autoScrollTick();
1062+
1063+
expect(outerBox.top).toBeGreaterThan(0); // the page behind finally moves
1064+
expect(self._autoScrollContainer).toBe(outer); // and we keep going on that one
1065+
});
1066+
1067+
it('stays on the inner container while it can still scroll', () => {
1068+
const { self, innerBox, outerBox } = nest(false);
1069+
self._autoScrollTick();
1070+
expect(innerBox.top).toBeGreaterThan(0);
1071+
expect(outerBox.top).toBe(0); // outer untouched
1072+
expect(self._autoScrollContainer).toBe(inner);
1073+
});
1074+
1075+
it('gives up once everything is pinned', () => {
1076+
const { self, outerBox } = nest(true);
1077+
outerBox.top = 500; // both at their limits
1078+
const stop = vi.spyOn(self as unknown as Record<string, () => void>, '_stopScrolling');
1079+
self._autoScrollTick();
1080+
expect(stop).toHaveBeenCalled();
1081+
});
1082+
});
10111083
});

‎src/dd-draggable.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,17 @@ export class DDDraggable extends DDBaseImplement implements HTMLElementExtendOpt
586586

587587
const prevScroll = scrollCont.scrollTop;
588588
scrollCont.scrollTop += scrollAmount;
589-
if (scrollCont.scrollTop === prevScroll) { this._stopScrolling(); return; }
589+
if (scrollCont.scrollTop === prevScroll) {
590+
// This container is pinned at its limit. We used to just give up, which with nested scroll
591+
// containers meant the drag dead-ended the moment the INNER one bottomed out and the page
592+
// behind it never moved (#3355). Hand off to the next scrollable ancestor instead.
593+
const next = scrollCont.parentElement ? Utils.getScrollElement(scrollCont.parentElement) : undefined;
594+
if (!next || next === scrollCont) { this._stopScrolling(); return; }
595+
const nextPrev = next.scrollTop;
596+
next.scrollTop += scrollAmount;
597+
if (next.scrollTop === nextPrev) { this._stopScrolling(); return; } // that one's pinned too
598+
this._autoScrollContainer = next; // keep going on the one that can actually move
599+
}
590600

591601
if (this.dragging && this.lastDrag) {
592602
this._updateCBDrift(); // we just scrolled; the 'scroll' event lands async so re-sync now

0 commit comments

Comments
 (0)