Skip to content

Commit 3c8abfb

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 a03a47c commit 3c8abfb

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
@@ -22,7 +22,6 @@ import {
2222
ViewEncapsulation,
2323
} from '@angular/core';
2424
import {TestBed, ComponentFixture, fakeAsync, flush, tick} from '@angular/core/testing';
25-
import {DOCUMENT} from '@angular/common';
2625
import {ViewportRuler, CdkScrollableModule} from '@angular/cdk/scrolling';
2726
import {_supportsShadowDom} from '@angular/cdk/platform';
2827
import {of as observableOf} from 'rxjs';
@@ -2483,7 +2482,6 @@ describe('CdkDrag', () => {
24832482
startDraggingViaMouse(fixture, item);
24842483

24852484
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
2486-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
24872485
const previewRect = preview.getBoundingClientRect();
24882486
const zeroPxRegex = /^0(px)?$/;
24892487

@@ -2505,23 +2503,18 @@ describe('CdkDrag', () => {
25052503
.withContext('Expected element to be removed from layout')
25062504
.toBe('-999em');
25072505
expect(item.style.opacity).withContext('Expected element to be invisible').toBe('0');
2508-
expect(previewContainer)
2509-
.withContext('Expected preview container to be in the DOM')
2510-
.toBeTruthy();
2511-
expect(previewContainer.style.color)
2512-
.withContext('Expected preview container to reset user agent color')
2513-
.toBe('inherit');
2514-
expect(previewContainer.style.margin)
2515-
.withContext('Expected preview container to reset user agent margin')
2516-
.toMatch(zeroPxRegex);
2517-
expect(previewContainer.style.padding)
2518-
.withContext('Expected preview container to reset user agent padding')
2506+
expect(preview).withContext('Expected preview to be in the DOM').toBeTruthy();
2507+
expect(preview.getAttribute('popover'))
2508+
.withContext('Expected preview to be a popover')
2509+
.toBe('manual');
2510+
expect(preview.style.margin)
2511+
.withContext('Expected preview to reset the margin')
25192512
.toMatch(zeroPxRegex);
25202513
expect(preview.textContent!.trim())
25212514
.withContext('Expected preview content to match element')
25222515
.toContain('One');
2523-
expect(previewContainer.getAttribute('dir'))
2524-
.withContext('Expected preview container element to inherit the directionality.')
2516+
expect(preview.getAttribute('dir'))
2517+
.withContext('Expected preview element to inherit the directionality.')
25252518
.toBe('ltr');
25262519
expect(previewRect.width)
25272520
.withContext('Expected preview width to match element')
@@ -2532,8 +2525,8 @@ describe('CdkDrag', () => {
25322525
expect(preview.style.pointerEvents)
25332526
.withContext('Expected pointer events to be disabled on the preview')
25342527
.toBe('none');
2535-
expect(previewContainer.style.zIndex)
2536-
.withContext('Expected preview container to have a high default zIndex.')
2528+
expect(preview.style.zIndex)
2529+
.withContext('Expected preview to have a high default zIndex.')
25372530
.toBe('1000');
25382531
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
25392532
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
@@ -2554,8 +2547,8 @@ describe('CdkDrag', () => {
25542547
expect(item.style.top).withContext('Expected element to be within the layout').toBeFalsy();
25552548
expect(item.style.left).withContext('Expected element to be within the layout').toBeFalsy();
25562549
expect(item.style.opacity).withContext('Expected element to be visible').toBeFalsy();
2557-
expect(previewContainer.parentNode)
2558-
.withContext('Expected preview container to be removed from the DOM')
2550+
expect(preview.parentNode)
2551+
.withContext('Expected preview to be removed from the DOM')
25592552
.toBeFalsy();
25602553
}));
25612554

@@ -2573,59 +2566,10 @@ describe('CdkDrag', () => {
25732566
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
25742567
startDraggingViaMouse(fixture, item);
25752568

2576-
const preview = document.querySelector('.cdk-drag-preview-container')! as HTMLElement;
2569+
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
25772570
expect(preview.style.zIndex).toBe('3000');
25782571
}));
25792572

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

2924-
expect(document.querySelector('.cdk-drag-preview-container')!.getAttribute('dir'))
2925-
.withContext('Expected preview container to inherit the directionality.')
2868+
expect(document.querySelector('.cdk-drag-preview')!.getAttribute('dir'))
2869+
.withContext('Expected preview to inherit the directionality.')
29262870
.toBe('rtl');
29272871
}));
29282872

@@ -2934,7 +2878,6 @@ describe('CdkDrag', () => {
29342878
startDraggingViaMouse(fixture, item);
29352879

29362880
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
2937-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
29382881

29392882
// Add a duration since the tests won't include one.
29402883
preview.style.transitionDuration = '500ms';
@@ -2947,13 +2890,13 @@ describe('CdkDrag', () => {
29472890
fixture.detectChanges();
29482891
tick(250);
29492892

2950-
expect(previewContainer.parentNode)
2893+
expect(preview.parentNode)
29512894
.withContext('Expected preview to be in the DOM mid-way through the transition')
29522895
.toBeTruthy();
29532896

29542897
tick(500);
29552898

2956-
expect(previewContainer.parentNode)
2899+
expect(preview.parentNode)
29572900
.withContext('Expected preview to be removed from the DOM if the transition timed out')
29582901
.toBeFalsy();
29592902
}));
@@ -3057,7 +3000,6 @@ describe('CdkDrag', () => {
30573000
startDraggingViaMouse(fixture, item);
30583001

30593002
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
3060-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
30613003
preview.style.transition = 'opacity 500ms ease';
30623004

30633005
dispatchMouseEvent(document, 'mousemove', 50, 50);
@@ -3067,8 +3009,8 @@ describe('CdkDrag', () => {
30673009
fixture.detectChanges();
30683010
tick(0);
30693011

3070-
expect(previewContainer.parentNode)
3071-
.withContext('Expected preview container to be removed from the DOM immediately')
3012+
expect(preview.parentNode)
3013+
.withContext('Expected preview to be removed from the DOM immediately')
30723014
.toBeFalsy();
30733015
}));
30743016

@@ -3080,7 +3022,6 @@ describe('CdkDrag', () => {
30803022
startDraggingViaMouse(fixture, item);
30813023

30823024
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
3083-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
30843025
preview.style.transition = 'opacity 500ms ease, transform 1000ms ease';
30853026

30863027
dispatchMouseEvent(document, 'mousemove', 50, 50);
@@ -3090,17 +3031,15 @@ describe('CdkDrag', () => {
30903031
fixture.detectChanges();
30913032
tick(500);
30923033

3093-
expect(previewContainer.parentNode)
3094-
.withContext(
3095-
'Expected preview container to be in the DOM at the end of the opacity transition',
3096-
)
3034+
expect(preview.parentNode)
3035+
.withContext('Expected preview to be in the DOM at the end of the opacity transition')
30973036
.toBeTruthy();
30983037

30993038
tick(1000);
31003039

3101-
expect(previewContainer.parentNode)
3040+
expect(preview.parentNode)
31023041
.withContext(
3103-
'Expected preview container to be removed from the DOM at the end of the transform transition',
3042+
'Expected preview to be removed from the DOM at the end of the transform transition',
31043043
)
31053044
.toBeFalsy();
31063045
}));
@@ -3142,8 +3081,8 @@ describe('CdkDrag', () => {
31423081
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
31433082

31443083
startDraggingViaMouse(fixture, item);
3145-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
3146-
expect(previewContainer.parentNode).toBe(document.body);
3084+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
3085+
expect(preview.parentNode).toBe(document.body);
31473086
}));
31483087

31493088
it('should insert the preview into the parent node if previewContainer is set to `parent`', fakeAsync(() => {
@@ -3154,9 +3093,9 @@ describe('CdkDrag', () => {
31543093
const list = fixture.nativeElement.querySelector('.drop-list');
31553094

31563095
startDraggingViaMouse(fixture, item);
3157-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
3096+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
31583097
expect(list).toBeTruthy();
3159-
expect(previewContainer.parentNode).toBe(list);
3098+
expect(preview.parentNode).toBe(list);
31603099
}));
31613100

31623101
it('should insert the preview into a particular element, if specified', fakeAsync(() => {
@@ -3170,10 +3109,8 @@ describe('CdkDrag', () => {
31703109
fixture.detectChanges();
31713110

31723111
startDraggingViaMouse(fixture, item);
3173-
const previewContainerElement = document.querySelector(
3174-
'.cdk-drag-preview-container',
3175-
) as HTMLElement;
3176-
expect(previewContainerElement.parentNode).toBe(previewContainer.nativeElement);
3112+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
3113+
expect(preview.parentNode).toBe(previewContainer.nativeElement);
31773114
}));
31783115

31793116
it('should remove the id from the placeholder', fakeAsync(() => {
@@ -3685,17 +3622,15 @@ describe('CdkDrag', () => {
36853622

36863623
startDraggingViaMouse(fixture, item);
36873624

3688-
const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
3625+
const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
36893626

3690-
expect(previewContainer.parentNode)
3691-
.withContext('Expected preview container to be in the DOM')
3692-
.toBeTruthy();
3627+
expect(preview.parentNode).withContext('Expected preview to be in the DOM').toBeTruthy();
36933628
expect(item.parentNode).withContext('Expected drag item to be in the DOM').toBeTruthy();
36943629

36953630
fixture.destroy();
36963631

3697-
expect(previewContainer.parentNode)
3698-
.withContext('Expected preview container to be removed from the DOM')
3632+
expect(preview.parentNode)
3633+
.withContext('Expected preview to be removed from the DOM')
36993634
.toBeFalsy();
37003635
expect(item.parentNode)
37013636
.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)