Skip to content

Commit 8b24961

Browse files
myabcmyabc
committed
Model a scroll container in the DnD fixtures
Pragmatic warns when auto-scroll attaches to an element whose computed overflow is visible. Production always resolves a real scroll ancestor, so the warning was accurate: the fixtures had none. A wrapper carrying overflow but no height satisfies the check without shifting the geometry these specs measure drop edges from. Where the overflow belongs differs. The directive walks up to the closest scrollable ancestor, so its hosts and the card-view root get a wrapper; the drag service and the preview spec register on the element they are handed, so that element takes it directly. Nested-root fixtures need a second, inner wrapper. Two independent root engines resolving one shared host collide on Pragmatic's element-keyed registry, which dedupes per root and not across them. The opSortableListsScrollContainer suite keeps its unscrollable hosts. It asserts that warning on purpose.
1 parent f22e7c6 commit 8b24961

4 files changed

Lines changed: 143 additions & 113 deletions

File tree

‎frontend/src/app/features/work-packages/components/wp-card-view/services/wp-card-drag-and-drop.service.spec.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,9 +1002,11 @@ describe('WorkPackageCardDragAndDropService — real two-list directive fixture'
10021002
standalone: true,
10031003
imports: [OpSortableListsDirective, TestCardListHostComponent],
10041004
template: `
1005-
<div class="root" opSortableLists>
1006-
<op-test-sortable-card-list listId="list-a" [mobile]="mobileA()" />
1007-
<op-test-sortable-card-list listId="list-b" [mobile]="mobileB()" />
1005+
<div class="scroll-host" style="overflow: auto;">
1006+
<div class="root" opSortableLists>
1007+
<op-test-sortable-card-list listId="list-a" [mobile]="mobileA()" />
1008+
<op-test-sortable-card-list listId="list-b" [mobile]="mobileB()" />
1009+
</div>
10081010
</div>
10091011
`,
10101012
})

‎frontend/src/app/shared/directives/sortable-lists/sortable-lists.directive.behavior.spec.ts‎

Lines changed: 129 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,9 @@ import {
4141
type SortableListsRemovedEvent,
4242
} from './sortable-lists.directive';
4343

44-
// Deliberately does NOT `vi.mock` Pragmatic's autoscroll module: under the
45-
// repo's `isolate:false` vitest config, spec files share a module registry
46-
// within a worker, and this specifier is also `vi.mock`-ed (independently)
47-
// by `sortable-lists-engine.spec.ts` for its own, unrelated engine-level
48-
// tests. Two spec files independently mocking the same specifier crashes the
49-
// shared worker (an unhandled "error when mocking a module" rejection,
50-
// reproducible regardless of factory shape); the mock is also effectively
51-
// global for the whole worker once registered, so even a "compatible" mock
52-
// here would fight with that file's synthetic (non-real) replacement in
53-
// combined runs. Instead this suite observes the REAL implementation's own
54-
// documented side effects directly:
44+
// Deliberately does NOT `vi.mock` Pragmatic's autoscroll module: this suite
45+
// observes the REAL implementation's own documented side effects, which is
46+
// what makes the scroll-container assertions below meaningful.
5547
// - the scroll-fallback tests below assert on `data-auto-scrollable`, the
5648
// attribute Pragmatic's real `autoScrollForElements` sets on registration
5749
// and removes on cleanup (see `@atlaskit/pragmatic-drag-and-drop-auto-scroll`'s
@@ -71,20 +63,22 @@ describe('sortable-lists directive group behavior', () => {
7163
@Component({
7264
imports: [OpSortableListsDirective, OpSortableListsItemDirective],
7365
template: `
74-
<div
75-
class="root"
76-
opSortableLists
77-
(opSortableListsDrop)="drops.push($event)"
78-
style="padding-bottom: 40px;"
79-
>
80-
@for (entry of items(); track entry.id) {
81-
<div
82-
class="item"
83-
[opSortableListsItem]="entry.id"
84-
[opSortableListsItemCanDrag]="entry.canDrag"
85-
style="height: 40px; width: 200px;"
86-
>{{ entry.id }}</div>
87-
}
66+
<div class="scroll-host" style="overflow: auto;">
67+
<div
68+
class="root"
69+
opSortableLists
70+
(opSortableListsDrop)="drops.push($event)"
71+
style="padding-bottom: 40px;"
72+
>
73+
@for (entry of items(); track entry.id) {
74+
<div
75+
class="item"
76+
[opSortableListsItem]="entry.id"
77+
[opSortableListsItemCanDrag]="entry.canDrag"
78+
style="height: 40px; width: 200px;"
79+
>{{ entry.id }}</div>
80+
}
81+
</div>
8882
</div>
8983
`,
9084
})
@@ -225,10 +219,12 @@ describe('sortable-lists directive group behavior', () => {
225219
@Component({
226220
imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective],
227221
template: `
228-
<div class="root" opSortableLists opSortableListsList>
229-
@for (id of ids(); track id) {
230-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
231-
}
222+
<div class="scroll-host" style="overflow: auto;">
223+
<div class="root" opSortableLists opSortableListsList>
224+
@for (id of ids(); track id) {
225+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
226+
}
227+
</div>
232228
</div>
233229
`,
234230
})
@@ -291,29 +287,31 @@ describe('sortable-lists directive group behavior', () => {
291287
@Component({
292288
imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective],
293289
template: `
294-
<div class="root" opSortableLists>
295-
<div
296-
class="list-a"
297-
opSortableListsList
298-
opSortableListsListId="list-a"
299-
(opSortableListsDrop)="events.push({ type: 'drop', listId: 'a', event: $event })"
300-
(opSortableListsRemoved)="events.push({ type: 'removed', listId: 'a', event: $event })"
301-
>
302-
@for (id of idsA(); track id) {
303-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
304-
}
305-
</div>
290+
<div class="scroll-host" style="overflow: auto;">
291+
<div class="root" opSortableLists>
292+
<div
293+
class="list-a"
294+
opSortableListsList
295+
opSortableListsListId="list-a"
296+
(opSortableListsDrop)="events.push({ type: 'drop', listId: 'a', event: $event })"
297+
(opSortableListsRemoved)="events.push({ type: 'removed', listId: 'a', event: $event })"
298+
>
299+
@for (id of idsA(); track id) {
300+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
301+
}
302+
</div>
306303
307-
<div
308-
class="list-b"
309-
opSortableListsList
310-
opSortableListsListId="list-b"
311-
(opSortableListsDrop)="events.push({ type: 'drop', listId: 'b', event: $event })"
312-
(opSortableListsRemoved)="events.push({ type: 'removed', listId: 'b', event: $event })"
313-
>
314-
@for (id of idsB(); track id) {
315-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
316-
}
304+
<div
305+
class="list-b"
306+
opSortableListsList
307+
opSortableListsListId="list-b"
308+
(opSortableListsDrop)="events.push({ type: 'drop', listId: 'b', event: $event })"
309+
(opSortableListsRemoved)="events.push({ type: 'removed', listId: 'b', event: $event })"
310+
>
311+
@for (id of idsB(); track id) {
312+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
313+
}
314+
</div>
317315
</div>
318316
</div>
319317
`,
@@ -409,22 +407,24 @@ describe('sortable-lists directive group behavior', () => {
409407
@Component({
410408
imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective],
411409
template: `
412-
<div class="root" opSortableLists (opSortableListsDrop)="rootDrops.push($event)">
413-
@if (explicit()) {
414-
<div
415-
class="list"
416-
opSortableListsList
417-
(opSortableListsDrop)="listDrops.push($event)"
418-
>
410+
<div class="scroll-host" style="overflow: auto;">
411+
<div class="root" opSortableLists (opSortableListsDrop)="rootDrops.push($event)">
412+
@if (explicit()) {
413+
<div
414+
class="list"
415+
opSortableListsList
416+
(opSortableListsDrop)="listDrops.push($event)"
417+
>
418+
@for (id of ids(); track id) {
419+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
420+
}
421+
</div>
422+
} @else {
419423
@for (id of ids(); track id) {
420424
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
421425
}
422-
</div>
423-
} @else {
424-
@for (id of ids(); track id) {
425-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
426426
}
427-
}
427+
</div>
428428
</div>
429429
`,
430430
})
@@ -495,16 +495,23 @@ describe('sortable-lists directive group behavior', () => {
495495
});
496496

497497
describe('nested roots', () => {
498+
// Two independent roots each need their own closest-scrollable-ancestor,
499+
// or both walks terminate on the same element and Pragmatic ends up with
500+
// two registrations on one node — see sortable-lists-engine.ts:142-149.
498501
@Component({
499502
imports: [OpSortableListsDirective, OpSortableListsItemDirective],
500503
template: `
501-
<div class="outer-root" opSortableLists (opSortableListsDrop)="outerDrops.push($event)">
502-
<div class="outer-item" opSortableListsItem="outer-a" style="height: 40px; width: 200px;">outer-a</div>
503-
504-
<div class="inner-root" opSortableLists (opSortableListsDrop)="innerDrops.push($event)">
505-
@for (id of innerIds(); track id) {
506-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
507-
}
504+
<div class="scroll-host" style="overflow: auto;">
505+
<div class="outer-root" opSortableLists (opSortableListsDrop)="outerDrops.push($event)">
506+
<div class="outer-item" opSortableListsItem="outer-a" style="height: 40px; width: 200px;">outer-a</div>
507+
508+
<div class="inner-scroll-host" style="overflow: auto;">
509+
<div class="inner-root" opSortableLists (opSortableListsDrop)="innerDrops.push($event)">
510+
@for (id of innerIds(); track id) {
511+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
512+
}
513+
</div>
514+
</div>
508515
</div>
509516
</div>
510517
`,
@@ -548,27 +555,35 @@ describe('sortable-lists directive group behavior', () => {
548555
// list's id — DI's nearest-list lookup does not stop at the inner root,
549556
// so without the ownership check it would register against the inner
550557
// engine under an id that engine never registered a list for.
558+
//
559+
// The inner wrapper also gives the inner root its own scrollable
560+
// ancestor, distinct from the outer root's, so the two don't collide on
561+
// one Pragmatic registration — see sortable-lists-engine.ts:142-149.
551562
@Component({
552563
imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective],
553564
template: `
554-
<div class="outer-root" opSortableLists>
555-
<div
556-
class="outer-list"
557-
opSortableListsList
558-
opSortableListsListId="outer-list"
559-
(opSortableListsDrop)="outerListDrops.push($event)"
560-
(opSortableListsRemoved)="outerListRemoved.push($event)"
561-
>
565+
<div class="scroll-host" style="overflow: auto;">
566+
<div class="outer-root" opSortableLists>
562567
<div
563-
class="inner-root"
564-
opSortableLists
565-
(opSortableListsDrop)="innerDrops.push($event)"
566-
(opSortableListsRemoved)="innerRemoved.push($event)"
567-
style="padding-bottom: 40px;"
568+
class="outer-list"
569+
opSortableListsList
570+
opSortableListsListId="outer-list"
571+
(opSortableListsDrop)="outerListDrops.push($event)"
572+
(opSortableListsRemoved)="outerListRemoved.push($event)"
568573
>
569-
@for (id of innerIds(); track id) {
570-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
571-
}
574+
<div class="inner-scroll-host" style="overflow: auto;">
575+
<div
576+
class="inner-root"
577+
opSortableLists
578+
(opSortableListsDrop)="innerDrops.push($event)"
579+
(opSortableListsRemoved)="innerRemoved.push($event)"
580+
style="padding-bottom: 40px;"
581+
>
582+
@for (id of innerIds(); track id) {
583+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
584+
}
585+
</div>
586+
</div>
572587
</div>
573588
</div>
574589
</div>
@@ -637,10 +652,12 @@ describe('sortable-lists directive group behavior', () => {
637652
@Component({
638653
imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective],
639654
template: `
640-
<div class="root" opSortableLists opSortableListsList>
641-
@for (id of ids(); track id) {
642-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
643-
}
655+
<div class="scroll-host" style="overflow: auto;">
656+
<div class="root" opSortableLists opSortableListsList>
657+
@for (id of ids(); track id) {
658+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
659+
}
660+
</div>
644661
</div>
645662
`,
646663
})
@@ -778,20 +795,22 @@ describe('sortable-lists directive group behavior', () => {
778795
@Component({
779796
imports: [OpSortableListsDirective, OpSortableListsItemDirective],
780797
template: `
781-
<div
782-
class="root"
783-
opSortableLists
784-
[opSortableListsAxis]="'horizontal'"
785-
(opSortableListsDrop)="drops.push($event)"
786-
style="white-space: nowrap;"
787-
>
788-
@for (id of ids(); track id) {
789-
<div
790-
class="item"
791-
[opSortableListsItem]="id"
792-
style="display: inline-block; width: 40px; height: 20px;"
793-
>{{ id }}</div>
794-
}
798+
<div class="scroll-host" style="overflow: auto;">
799+
<div
800+
class="root"
801+
opSortableLists
802+
[opSortableListsAxis]="'horizontal'"
803+
(opSortableListsDrop)="drops.push($event)"
804+
style="white-space: nowrap;"
805+
>
806+
@for (id of ids(); track id) {
807+
<div
808+
class="item"
809+
[opSortableListsItem]="id"
810+
style="display: inline-block; width: 40px; height: 20px;"
811+
>{{ id }}</div>
812+
}
813+
</div>
795814
</div>
796815
`,
797816
})
@@ -804,10 +823,12 @@ describe('sortable-lists directive group behavior', () => {
804823
@Component({
805824
imports: [OpSortableListsDirective, OpSortableListsItemDirective],
806825
template: `
807-
<div class="root" opSortableLists (opSortableListsDrop)="drops.push($event)">
808-
@for (id of ids(); track id) {
809-
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
810-
}
826+
<div class="scroll-host" style="overflow: auto;">
827+
<div class="root" opSortableLists (opSortableListsDrop)="drops.push($event)">
828+
@for (id of ids(); track id) {
829+
<div class="item" [opSortableListsItem]="id" style="height: 40px; width: 200px;">{{ id }}</div>
830+
}
831+
</div>
811832
</div>
812833
`,
813834
})

‎frontend/src/app/shared/helpers/drag-and-drop/drag-and-drop.service.spec.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ function buildRow(id:string, opts:{ handle?:boolean } = {}):HTMLElement {
6060

6161
function buildContainer(ids:string[], opts:{ handle?:boolean } = {}):{ container:HTMLElement; rows:HTMLElement[] } {
6262
const container = document.createElement('div');
63-
container.style.cssText = 'width:200px;';
63+
// `overflow-y` with no height keeps the container a scroll container for
64+
// Pragmatic's computed-overflow check. This also computes overflow-x to
65+
// `auto`, but rows are exactly 200px wide in a 200px container.
66+
container.style.cssText = 'width:200px; overflow-y:auto;';
6467
const rows = ids.map((id) => {
6568
const row = buildRow(id, opts);
6669
container.appendChild(row);
@@ -159,7 +162,7 @@ describe('DragAndDropService', () => {
159162
describe('container append', () => {
160163
it('reports a drop below the rows as a null-target append intent', async () => {
161164
const { container, rows } = buildContainer(['a0', 'a1']);
162-
container.style.cssText = 'width:200px; padding-bottom:40px;';
165+
container.style.cssText = 'width:200px; padding-bottom:40px; overflow-y:auto;';
163166
const onMoved = vi.fn();
164167
service.register(buildMember(container, { onMoved }));
165168

‎frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ describe('createSortableRoot drag preview offset', () => {
7070

7171
function setup():{ rows:HTMLElement[] } {
7272
const root = document.createElement('div');
73+
// The engine attaches auto-scroll to this element directly, with no
74+
// ancestor walk, so the overflow has to sit here. This also computes
75+
// overflow-x to auto; 600px rows just happen to fit the viewport.
76+
root.style.cssText = 'overflow-y:auto;';
7377
const rows = ['a', 'b'].map((id) => {
7478
const row = document.createElement('div');
7579
row.style.cssText = 'height:40px; width:600px;';

0 commit comments

Comments
 (0)