Skip to content

Commit f13b909

Browse files
committed
fix(cdk/drag-drop): remove preview wrapper
Switches back to positioning the preview directly instead of using a wrapper which can be breaking for existing apps. Instead we load a stylesheet dynamically with the necessary resets. (cherry picked from commit 9729a53)
1 parent b86c8c8 commit f13b909

File tree

5 files changed

+112
-138
lines changed

5 files changed

+112
-138
lines changed

src/cdk/drag-drop/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ load(
44
"ng_module",
55
"ng_test_library",
66
"ng_web_test_suite",
7+
"sass_binary",
78
)
89

910
package(default_visibility = ["//visibility:public"])
@@ -14,6 +15,9 @@ ng_module(
1415
["**/*.ts"],
1516
exclude = ["**/*.spec.ts"],
1617
),
18+
assets = [
19+
":resets_scss",
20+
],
1721
deps = [
1822
"//src:dev_mode_types",
1923
"//src/cdk/a11y",
@@ -44,6 +48,11 @@ ng_test_library(
4448
],
4549
)
4650

51+
sass_binary(
52+
name = "resets_scss",
53+
src = "resets.scss",
54+
)
55+
4756
ng_web_test_suite(
4857
name = "unit_tests",
4958
deps = [":unit_test_sources"],

src/cdk/drag-drop/directives/drag.spec.ts

Lines changed: 33 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import {
2323
ViewEncapsulation,
2424
} from '@angular/core';
2525
import {TestBed, ComponentFixture, fakeAsync, flush, tick} from '@angular/core/testing';
26-
import {DOCUMENT} from '@angular/common';
2726
import {ViewportRuler, CdkScrollableModule} from '@angular/cdk/scrolling';
2827
import {_supportsShadowDom} from '@angular/cdk/platform';
2928
import {of as observableOf} from 'rxjs';
@@ -2490,7 +2489,6 @@ describe('CdkDrag', () => {
24902489
startDraggingViaMouse(fixture, item);
24912490

24922491
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
2493-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
24942492
const previewRect = preview.getBoundingClientRect();
24952493
const zeroPxRegex = /^0(px)?$/;
24962494

@@ -2512,23 +2510,18 @@ describe('CdkDrag', () => {
25122510
.withContext('Expected element to be removed from layout')
25132511
.toBe('-999em');
25142512
expect(item.style.opacity).withContext('Expected element to be invisible').toBe('0');
2515-
expect(previewContainer)
2516-
.withContext('Expected preview container to be in the DOM')
2517-
.toBeTruthy();
2518-
expect(previewContainer.style.color)
2519-
.withContext('Expected preview container to reset user agent color')
2520-
.toBe('inherit');
2521-
expect(previewContainer.style.margin)
2522-
.withContext('Expected preview container to reset user agent margin')
2523-
.toMatch(zeroPxRegex);
2524-
expect(previewContainer.style.padding)
2525-
.withContext('Expected preview container to reset user agent padding')
2513+
expect(preview).withContext('Expected preview to be in the DOM').toBeTruthy();
2514+
expect(preview.getAttribute('popover'))
2515+
.withContext('Expected preview to be a popover')
2516+
.toBe('manual');
2517+
expect(preview.style.margin)
2518+
.withContext('Expected preview to reset the margin')
25262519
.toMatch(zeroPxRegex);
25272520
expect(preview.textContent!.trim())
25282521
.withContext('Expected preview content to match element')
25292522
.toContain('One');
2530-
expect(previewContainer.getAttribute('dir'))
2531-
.withContext('Expected preview container element to inherit the directionality.')
2523+
expect(preview.getAttribute('dir'))
2524+
.withContext('Expected preview element to inherit the directionality.')
25322525
.toBe('ltr');
25332526
expect(previewRect.width)
25342527
.withContext('Expected preview width to match element')
@@ -2539,8 +2532,8 @@ describe('CdkDrag', () => {
25392532
expect(preview.style.pointerEvents)
25402533
.withContext('Expected pointer events to be disabled on the preview')
25412534
.toBe('none');
2542-
expect(previewContainer.style.zIndex)
2543-
.withContext('Expected preview container to have a high default zIndex.')
2535+
expect(preview.style.zIndex)
2536+
.withContext('Expected preview to have a high default zIndex.')
25442537
.toBe('1000');
25452538
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
25462539
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
@@ -2561,8 +2554,8 @@ describe('CdkDrag', () => {
25612554
expect(item.style.top).withContext('Expected element to be within the layout').toBeFalsy();
25622555
expect(item.style.left).withContext('Expected element to be within the layout').toBeFalsy();
25632556
expect(item.style.opacity).withContext('Expected element to be visible').toBeFalsy();
2564-
expect(previewContainer.parentNode)
2565-
.withContext('Expected preview container to be removed from the DOM')
2557+
expect(preview.parentNode)
2558+
.withContext('Expected preview to be removed from the DOM')
25662559
.toBeFalsy();
25672560
}));
25682561

@@ -2580,59 +2573,10 @@ describe('CdkDrag', () => {
25802573
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
25812574
startDraggingViaMouse(fixture, item);
25822575

2583-
const preview = document.querySelector('.cdk-drag-preview-container')! as HTMLElement;
2576+
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
25842577
expect(preview.style.zIndex).toBe('3000');
25852578
}));
25862579

2587-
it('should create the preview inside the fullscreen element when in fullscreen mode', fakeAsync(() => {
2588-
// Provide a limited stub of the document since we can't trigger fullscreen
2589-
// mode in unit tests and there are some issues with doing it in e2e tests.
2590-
const fakeDocument = {
2591-
body: document.body,
2592-
documentElement: document.documentElement,
2593-
fullscreenElement: document.createElement('div'),
2594-
ELEMENT_NODE: Node.ELEMENT_NODE,
2595-
querySelectorAll: (...args: [string]) => document.querySelectorAll(...args),
2596-
querySelector: (...args: [string]) => document.querySelector(...args),
2597-
createElement: (...args: [string]) => document.createElement(...args),
2598-
createTextNode: (...args: [string]) => document.createTextNode(...args),
2599-
addEventListener: (
2600-
...args: [
2601-
string,
2602-
EventListenerOrEventListenerObject,
2603-
(boolean | AddEventListenerOptions | undefined)?,
2604-
]
2605-
) => document.addEventListener(...args),
2606-
removeEventListener: (
2607-
...args: [
2608-
string,
2609-
EventListenerOrEventListenerObject,
2610-
(boolean | AddEventListenerOptions | undefined)?,
2611-
]
2612-
) => document.addEventListener(...args),
2613-
createComment: (text: string) => document.createComment(text),
2614-
};
2615-
const fixture = createComponent(DraggableInDropZone, [
2616-
{
2617-
provide: DOCUMENT,
2618-
useFactory: () => fakeDocument,
2619-
},
2620-
]);
2621-
fixture.detectChanges();
2622-
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
2623-
2624-
document.body.appendChild(fakeDocument.fullscreenElement);
2625-
startDraggingViaMouse(fixture, item);
2626-
flush();
2627-
2628-
const previewContainer = document.querySelector(
2629-
'.cdk-drag-preview-container',
2630-
)! as HTMLElement;
2631-
2632-
expect(previewContainer.parentNode).toBe(fakeDocument.fullscreenElement);
2633-
fakeDocument.fullscreenElement.remove();
2634-
}));
2635-
26362580
it('should be able to constrain the preview position', fakeAsync(() => {
26372581
const fixture = createComponent(DraggableInDropZone);
26382582
fixture.componentInstance.boundarySelector = '.cdk-drop-list';
@@ -2928,8 +2872,8 @@ describe('CdkDrag', () => {
29282872
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
29292873
startDraggingViaMouse(fixture, item);
29302874

2931-
expect(document.querySelector('.cdk-drag-preview-container')!.getAttribute('dir'))
2932-
.withContext('Expected preview container to inherit the directionality.')
2875+
expect(document.querySelector('.cdk-drag-preview')!.getAttribute('dir'))
2876+
.withContext('Expected preview to inherit the directionality.')
29332877
.toBe('rtl');
29342878
}));
29352879

@@ -2941,7 +2885,6 @@ describe('CdkDrag', () => {
29412885
startDraggingViaMouse(fixture, item);
29422886

29432887
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
2944-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
29452888

29462889
// Add a duration since the tests won't include one.
29472890
preview.style.transitionDuration = '500ms';
@@ -2954,13 +2897,13 @@ describe('CdkDrag', () => {
29542897
fixture.detectChanges();
29552898
tick(250);
29562899

2957-
expect(previewContainer.parentNode)
2900+
expect(preview.parentNode)
29582901
.withContext('Expected preview to be in the DOM mid-way through the transition')
29592902
.toBeTruthy();
29602903

29612904
tick(500);
29622905

2963-
expect(previewContainer.parentNode)
2906+
expect(preview.parentNode)
29642907
.withContext('Expected preview to be removed from the DOM if the transition timed out')
29652908
.toBeFalsy();
29662909
}));
@@ -3064,7 +3007,6 @@ describe('CdkDrag', () => {
30643007
startDraggingViaMouse(fixture, item);
30653008

30663009
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
3067-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
30683010
preview.style.transition = 'opacity 500ms ease';
30693011

30703012
dispatchMouseEvent(document, 'mousemove', 50, 50);
@@ -3074,8 +3016,8 @@ describe('CdkDrag', () => {
30743016
fixture.detectChanges();
30753017
tick(0);
30763018

3077-
expect(previewContainer.parentNode)
3078-
.withContext('Expected preview container to be removed from the DOM immediately')
3019+
expect(preview.parentNode)
3020+
.withContext('Expected preview to be removed from the DOM immediately')
30793021
.toBeFalsy();
30803022
}));
30813023

@@ -3087,7 +3029,6 @@ describe('CdkDrag', () => {
30873029
startDraggingViaMouse(fixture, item);
30883030

30893031
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
3090-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
30913032
preview.style.transition = 'opacity 500ms ease, transform 1000ms ease';
30923033

30933034
dispatchMouseEvent(document, 'mousemove', 50, 50);
@@ -3097,17 +3038,15 @@ describe('CdkDrag', () => {
30973038
fixture.detectChanges();
30983039
tick(500);
30993040

3100-
expect(previewContainer.parentNode)
3101-
.withContext(
3102-
'Expected preview container to be in the DOM at the end of the opacity transition',
3103-
)
3041+
expect(preview.parentNode)
3042+
.withContext('Expected preview to be in the DOM at the end of the opacity transition')
31043043
.toBeTruthy();
31053044

31063045
tick(1000);
31073046

3108-
expect(previewContainer.parentNode)
3047+
expect(preview.parentNode)
31093048
.withContext(
3110-
'Expected preview container to be removed from the DOM at the end of the transform transition',
3049+
'Expected preview to be removed from the DOM at the end of the transform transition',
31113050
)
31123051
.toBeFalsy();
31133052
}));
@@ -3149,8 +3088,8 @@ describe('CdkDrag', () => {
31493088
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
31503089

31513090
startDraggingViaMouse(fixture, item);
3152-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
3153-
expect(previewContainer.parentNode).toBe(document.body);
3091+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
3092+
expect(preview.parentNode).toBe(document.body);
31543093
}));
31553094

31563095
it('should insert the preview into the parent node if previewContainer is set to `parent`', fakeAsync(() => {
@@ -3161,9 +3100,9 @@ describe('CdkDrag', () => {
31613100
const list = fixture.nativeElement.querySelector('.drop-list');
31623101

31633102
startDraggingViaMouse(fixture, item);
3164-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
3103+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
31653104
expect(list).toBeTruthy();
3166-
expect(previewContainer.parentNode).toBe(list);
3105+
expect(preview.parentNode).toBe(list);
31673106
}));
31683107

31693108
it('should insert the preview into a particular element, if specified', fakeAsync(() => {
@@ -3177,10 +3116,8 @@ describe('CdkDrag', () => {
31773116
fixture.detectChanges();
31783117

31793118
startDraggingViaMouse(fixture, item);
3180-
const previewContainerElement = document.querySelector(
3181-
'.cdk-drag-preview-container',
3182-
) as HTMLElement;
3183-
expect(previewContainerElement.parentNode).toBe(previewContainer.nativeElement);
3119+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
3120+
expect(preview.parentNode).toBe(previewContainer.nativeElement);
31843121
}));
31853122

31863123
it('should remove the id from the placeholder', fakeAsync(() => {
@@ -3692,17 +3629,15 @@ describe('CdkDrag', () => {
36923629

36933630
startDraggingViaMouse(fixture, item);
36943631

3695-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
3632+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
36963633

3697-
expect(previewContainer.parentNode)
3698-
.withContext('Expected preview container to be in the DOM')
3699-
.toBeTruthy();
3634+
expect(preview.parentNode).withContext('Expected preview to be in the DOM').toBeTruthy();
37003635
expect(item.parentNode).withContext('Expected drag item to be in the DOM').toBeTruthy();
37013636

37023637
fixture.destroy();
37033638

3704-
expect(previewContainer.parentNode)
3705-
.withContext('Expected preview container to be removed from the DOM')
3639+
expect(preview.parentNode)
3640+
.withContext('Expected preview to be removed from the DOM')
37063641
.toBeFalsy();
37073642
expect(item.parentNode)
37083643
.withContext('Expected drag item to be removed from the DOM')

src/cdk/drag-drop/drag-drop.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,19 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Injectable, Inject, NgZone, ElementRef} from '@angular/core';
9+
import {
10+
Injectable,
11+
Inject,
12+
NgZone,
13+
ElementRef,
14+
Component,
15+
ViewEncapsulation,
16+
ChangeDetectionStrategy,
17+
ApplicationRef,
18+
inject,
19+
createComponent,
20+
EnvironmentInjector,
21+
} from '@angular/core';
1022
import {DOCUMENT} from '@angular/common';
1123
import {ViewportRuler} from '@angular/cdk/scrolling';
1224
import {DragRef, DragRefConfig} from './drag-ref';
@@ -19,11 +31,31 @@ const DEFAULT_CONFIG = {
1931
pointerDirectionChangeThreshold: 5,
2032
};
2133

34+
/** Keeps track of the apps currently containing badges. */
35+
const activeApps = new Set<ApplicationRef>();
36+
37+
/**
38+
* Component used to load the drag&drop reset styles.
39+
* @docs-private
40+
*/
41+
@Component({
42+
standalone: true,
43+
styleUrl: 'resets.css',
44+
encapsulation: ViewEncapsulation.None,
45+
template: '',
46+
changeDetection: ChangeDetectionStrategy.OnPush,
47+
host: {'cdk-drag-resets-container': ''},
48+
})
49+
export class _ResetsLoader {}
50+
2251
/**
2352
* Service that allows for drag-and-drop functionality to be attached to DOM elements.
2453
*/
2554
@Injectable({providedIn: 'root'})
2655
export class DragDrop {
56+
private _appRef = inject(ApplicationRef);
57+
private _environmentInjector = inject(EnvironmentInjector);
58+
2759
constructor(
2860
@Inject(DOCUMENT) private _document: any,
2961
private _ngZone: NgZone,
@@ -40,6 +72,7 @@ export class DragDrop {
4072
element: ElementRef<HTMLElement> | HTMLElement,
4173
config: DragRefConfig = DEFAULT_CONFIG,
4274
): DragRef<T> {
75+
this._loadResets();
4376
return new DragRef<T>(
4477
element,
4578
config,
@@ -63,4 +96,23 @@ export class DragDrop {
6396
this._viewportRuler,
6497
);
6598
}
99+
100+
// TODO(crisbeto): abstract this away into something reusable.
101+
/** Loads the CSS resets needed for the module to work correctly. */
102+
private _loadResets() {
103+
if (!activeApps.has(this._appRef)) {
104+
activeApps.add(this._appRef);
105+
106+
const componentRef = createComponent(_ResetsLoader, {
107+
environmentInjector: this._environmentInjector,
108+
});
109+
110+
this._appRef.onDestroy(() => {
111+
activeApps.delete(this._appRef);
112+
if (activeApps.size === 0) {
113+
componentRef.destroy();
114+
}
115+
});
116+
}
117+
}
66118
}

0 commit comments

Comments
 (0)