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.

Author: crisbeto

src/cdk/drag-drop/BUILD.bazel

@@ -4,6 +4,7 @@ load(
     "ng_module",
     "ng_test_library",
     "ng_web_test_suite",
+    "sass_binary",
 )
 
 package(default_visibility = ["//visibility:public"])
@@ -14,6 +15,9 @@ ng_module(
         ["**/*.ts"],
         exclude = ["**/*.spec.ts"],
     ),
+    assets = [
+        ":resets_scss",
+    ],
     deps = [
         "//src:dev_mode_types",
         "//src/cdk/a11y",
@@ -44,6 +48,11 @@ ng_test_library(
     ],
 )
 
+sass_binary(
+    name = "resets_scss",
+    src = "resets.scss",
+)
+
 ng_web_test_suite(
     name = "unit_tests",
     deps = [":unit_test_sources"],

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

@@ -44,5 +44,4 @@ export interface DragDropConfig extends Partial<DragRefConfig> {
   listOrientation?: DropListOrientation;
   zIndex?: number;
   previewContainer?: 'global' | 'parent';
-  disablePreviewPopover?: boolean;
 }

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

@@ -23,7 +23,6 @@ import {
   ViewEncapsulation,
 } from '@angular/core';
 import {TestBed, ComponentFixture, fakeAsync, flush, tick} from '@angular/core/testing';
-import {DOCUMENT} from '@angular/common';
 import {ViewportRuler, CdkScrollableModule} from '@angular/cdk/scrolling';
 import {_supportsShadowDom} from '@angular/cdk/platform';
 import {of as observableOf} from 'rxjs';
@@ -2490,7 +2489,6 @@ describe('CdkDrag', () => {
       startDraggingViaMouse(fixture, item);
 
       const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
-      const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
       const previewRect = preview.getBoundingClientRect();
       const zeroPxRegex = /^0(px)?$/;
 
@@ -2512,23 +2510,18 @@ describe('CdkDrag', () => {
         .withContext('Expected element to be removed from layout')
         .toBe('-999em');
       expect(item.style.opacity).withContext('Expected element to be invisible').toBe('0');
-      expect(previewContainer)
-        .withContext('Expected preview container to be in the DOM')
-        .toBeTruthy();
-      expect(previewContainer.style.color)
-        .withContext('Expected preview container to reset user agent color')
-        .toBe('inherit');
-      expect(previewContainer.style.margin)
-        .withContext('Expected preview container to reset user agent margin')
-        .toMatch(zeroPxRegex);
-      expect(previewContainer.style.padding)
-        .withContext('Expected preview container to reset user agent padding')
+      expect(preview).withContext('Expected preview to be in the DOM').toBeTruthy();
+      expect(preview.getAttribute('popover'))
+        .withContext('Expected preview to be a popover')
+        .toBe('manual');
+      expect(preview.style.margin)
+        .withContext('Expected preview to reset the margin')
         .toMatch(zeroPxRegex);
       expect(preview.textContent!.trim())
         .withContext('Expected preview content to match element')
         .toContain('One');
-      expect(previewContainer.getAttribute('dir'))
-        .withContext('Expected preview container element to inherit the directionality.')
+      expect(preview.getAttribute('dir'))
+        .withContext('Expected preview element to inherit the directionality.')
         .toBe('ltr');
       expect(previewRect.width)
         .withContext('Expected preview width to match element')
@@ -2539,8 +2532,8 @@ describe('CdkDrag', () => {
       expect(preview.style.pointerEvents)
         .withContext('Expected pointer events to be disabled on the preview')
         .toBe('none');
-      expect(previewContainer.style.zIndex)
-        .withContext('Expected preview container to have a high default zIndex.')
+      expect(preview.style.zIndex)
+        .withContext('Expected preview to have a high default zIndex.')
         .toBe('1000');
       // Use a regex here since some browsers normalize 0 to 0px, but others don't.
       // Use a regex here since some browsers normalize 0 to 0px, but others don't.
@@ -2561,8 +2554,8 @@ describe('CdkDrag', () => {
       expect(item.style.top).withContext('Expected element to be within the layout').toBeFalsy();
       expect(item.style.left).withContext('Expected element to be within the layout').toBeFalsy();
       expect(item.style.opacity).withContext('Expected element to be visible').toBeFalsy();
-      expect(previewContainer.parentNode)
-        .withContext('Expected preview container to be removed from the DOM')
+      expect(preview.parentNode)
+        .withContext('Expected preview to be removed from the DOM')
         .toBeFalsy();
     }));
 
@@ -2580,59 +2573,10 @@ describe('CdkDrag', () => {
       const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
       startDraggingViaMouse(fixture, item);
 
-      const preview = document.querySelector('.cdk-drag-preview-container')! as HTMLElement;
+      const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
       expect(preview.style.zIndex).toBe('3000');
     }));
 
-    it('should create the preview inside the fullscreen element when in fullscreen mode', fakeAsync(() => {
-      // Provide a limited stub of the document since we can't trigger fullscreen
-      // mode in unit tests and there are some issues with doing it in e2e tests.
-      const fakeDocument = {
-        body: document.body,
-        documentElement: document.documentElement,
-        fullscreenElement: document.createElement('div'),
-        ELEMENT_NODE: Node.ELEMENT_NODE,
-        querySelectorAll: (...args: [string]) => document.querySelectorAll(...args),
-        querySelector: (...args: [string]) => document.querySelector(...args),
-        createElement: (...args: [string]) => document.createElement(...args),
-        createTextNode: (...args: [string]) => document.createTextNode(...args),
-        addEventListener: (
-          ...args: [
-            string,
-            EventListenerOrEventListenerObject,
-            (boolean | AddEventListenerOptions | undefined)?,
-          ]
-        ) => document.addEventListener(...args),
-        removeEventListener: (
-          ...args: [
-            string,
-            EventListenerOrEventListenerObject,
-            (boolean | AddEventListenerOptions | undefined)?,
-          ]
-        ) => document.addEventListener(...args),
-        createComment: (text: string) => document.createComment(text),
-      };
-      const fixture = createComponent(DraggableInDropZone, [
-        {
-          provide: DOCUMENT,
-          useFactory: () => fakeDocument,
-        },
-      ]);
-      fixture.detectChanges();
-      const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
-
-      document.body.appendChild(fakeDocument.fullscreenElement);
-      startDraggingViaMouse(fixture, item);
-      flush();
-
-      const previewContainer = document.querySelector(
-        '.cdk-drag-preview-container',
-      )! as HTMLElement;
-
-      expect(previewContainer.parentNode).toBe(fakeDocument.fullscreenElement);
-      fakeDocument.fullscreenElement.remove();
-    }));
-
     it('should be able to constrain the preview position', fakeAsync(() => {
       const fixture = createComponent(DraggableInDropZone);
       fixture.componentInstance.boundarySelector = '.cdk-drop-list';
@@ -2928,8 +2872,8 @@ describe('CdkDrag', () => {
       const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
       startDraggingViaMouse(fixture, item);
 
-      expect(document.querySelector('.cdk-drag-preview-container')!.getAttribute('dir'))
-        .withContext('Expected preview container to inherit the directionality.')
+      expect(document.querySelector('.cdk-drag-preview')!.getAttribute('dir'))
+        .withContext('Expected preview to inherit the directionality.')
         .toBe('rtl');
     }));
 
@@ -2941,7 +2885,6 @@ describe('CdkDrag', () => {
       startDraggingViaMouse(fixture, item);
 
       const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
-      const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
 
       // Add a duration since the tests won't include one.
       preview.style.transitionDuration = '500ms';
@@ -2954,13 +2897,13 @@ describe('CdkDrag', () => {
       fixture.detectChanges();
       tick(250);
 
-      expect(previewContainer.parentNode)
+      expect(preview.parentNode)
         .withContext('Expected preview to be in the DOM mid-way through the transition')
         .toBeTruthy();
 
       tick(500);
 
-      expect(previewContainer.parentNode)
+      expect(preview.parentNode)
         .withContext('Expected preview to be removed from the DOM if the transition timed out')
         .toBeFalsy();
     }));
@@ -3064,7 +3007,6 @@ describe('CdkDrag', () => {
       startDraggingViaMouse(fixture, item);
 
       const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
-      const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
       preview.style.transition = 'opacity 500ms ease';
 
       dispatchMouseEvent(document, 'mousemove', 50, 50);
@@ -3074,8 +3016,8 @@ describe('CdkDrag', () => {
       fixture.detectChanges();
       tick(0);
 
-      expect(previewContainer.parentNode)
-        .withContext('Expected preview container to be removed from the DOM immediately')
+      expect(preview.parentNode)
+        .withContext('Expected preview to be removed from the DOM immediately')
         .toBeFalsy();
     }));
 
@@ -3087,7 +3029,6 @@ describe('CdkDrag', () => {
       startDraggingViaMouse(fixture, item);
 
       const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
-      const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
       preview.style.transition = 'opacity 500ms ease, transform 1000ms ease';
 
       dispatchMouseEvent(document, 'mousemove', 50, 50);
@@ -3097,17 +3038,15 @@ describe('CdkDrag', () => {
       fixture.detectChanges();
       tick(500);
 
-      expect(previewContainer.parentNode)
-        .withContext(
-          'Expected preview container to be in the DOM at the end of the opacity transition',
-        )
+      expect(preview.parentNode)
+        .withContext('Expected preview to be in the DOM at the end of the opacity transition')
         .toBeTruthy();
 
       tick(1000);
 
-      expect(previewContainer.parentNode)
+      expect(preview.parentNode)
         .withContext(
-          'Expected preview container to be removed from the DOM at the end of the transform transition',
+          'Expected preview to be removed from the DOM at the end of the transform transition',
         )
         .toBeFalsy();
     }));
@@ -3149,8 +3088,8 @@ describe('CdkDrag', () => {
       const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
 
       startDraggingViaMouse(fixture, item);
-      const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
-      expect(previewContainer.parentNode).toBe(document.body);
+      const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
+      expect(preview.parentNode).toBe(document.body);
     }));
 
     it('should insert the preview into the parent node if previewContainer is set to `parent`', fakeAsync(() => {
@@ -3161,9 +3100,9 @@ describe('CdkDrag', () => {
       const list = fixture.nativeElement.querySelector('.drop-list');
 
       startDraggingViaMouse(fixture, item);
-      const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
+      const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
       expect(list).toBeTruthy();
-      expect(previewContainer.parentNode).toBe(list);
+      expect(preview.parentNode).toBe(list);
     }));
 
     it('should insert the preview into a particular element, if specified', fakeAsync(() => {
@@ -3176,25 +3115,9 @@ describe('CdkDrag', () => {
       fixture.componentInstance.previewContainer = previewContainer;
       fixture.detectChanges();
 
-      startDraggingViaMouse(fixture, item);
-      const previewContainerElement = document.querySelector(
-        '.cdk-drag-preview-container',
-      ) as HTMLElement;
-      expect(previewContainerElement.parentNode).toBe(previewContainer.nativeElement);
-    }));
-
-    it('should not create a popover wrapper if disablePreviewPopover is enabled', fakeAsync(() => {
-      const fixture = createComponent(DraggableInDropZone);
-      fixture.componentInstance.previewContainer = 'global';
-      fixture.componentInstance.disablePreviewPopover = true;
-      fixture.detectChanges();
-      const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
-
       startDraggingViaMouse(fixture, item);
       const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
-      expect(document.querySelector('.cdk-drag-preview-container')).toBeFalsy();
-      expect(preview).toBeTruthy();
-      expect(preview.parentElement).toBe(document.body);
+      expect(preview.parentNode).toBe(previewContainer.nativeElement);
     }));
 
     it('should remove the id from the placeholder', fakeAsync(() => {
@@ -3706,17 +3629,15 @@ describe('CdkDrag', () => {
 
       startDraggingViaMouse(fixture, item);
 
-      const previewContainer = document.querySelector('.cdk-drag-preview-container') as HTMLElement;
+      const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
 
-      expect(previewContainer.parentNode)
-        .withContext('Expected preview container to be in the DOM')
-        .toBeTruthy();
+      expect(preview.parentNode).withContext('Expected preview to be in the DOM').toBeTruthy();
       expect(item.parentNode).withContext('Expected drag item to be in the DOM').toBeTruthy();
 
       fixture.destroy();
 
-      expect(previewContainer.parentNode)
-        .withContext('Expected preview container to be removed from the DOM')
+      expect(preview.parentNode)
+        .withContext('Expected preview to be removed from the DOM')
         .toBeFalsy();
       expect(item.parentNode)
         .withContext('Expected drag item to be removed from the DOM')
@@ -6950,7 +6871,6 @@ const DROP_ZONE_FIXTURE_TEMPLATE = `
       [cdkDragBoundary]="boundarySelector"
       [cdkDragPreviewClass]="previewClass"
       [cdkDragPreviewContainer]="previewContainer"
-      [cdkDragDisablePreviewPopover]="disablePreviewPopover"
       [style.height.px]="item.height"
       [style.margin-bottom.px]="item.margin"
       (cdkDragStarted)="startedSpy($event)"
@@ -6980,7 +6900,6 @@ class DraggableInDropZone implements AfterViewInit {
   });
   startedSpy = jasmine.createSpy('started spy');
   previewContainer: PreviewContainer = 'global';
-  disablePreviewPopover = false;
 
   constructor(protected _elementRef: ElementRef) {}
 

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

@@ -159,20 +159,6 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
    */
   @Input('cdkDragPreviewContainer') previewContainer: PreviewContainer;
 
-  /**
-   * By default the preview element is wrapped in a native popover in order to be compatible
-   * with other native popovers and to avoid issues with `overflow: hidden`. In some edge cases
-   * this can interfere with styling (e.g. CSS selectors targeting direct descendants). Enable
-   * this option to remove the wrapper around the preview, but note that it can cause the following
-   * issues when used with  `cdkDragPreviewContainer` set to `parent` or a specific DOM node:
-   * - The preview may be clipped by a parent with `overflow: hidden`.
-   * - The preview isn't guaranteed to be on top of other elements, despite its `z-index`.
-   * - Transforms on the parent of the preview can affect its positioning.
-   * - The preview may be positioned under native `<dialog>` or popover elements.
-   */
-  @Input({alias: 'cdkDragDisablePreviewPopover', transform: booleanAttribute})
-  disablePreviewPopover: boolean;
-
   /** Emits when the user starts dragging the item. */
   @Output('cdkDragStarted') readonly started: EventEmitter<CdkDragStart> =
     new EventEmitter<CdkDragStart>();
@@ -472,7 +458,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
           .withBoundaryElement(this._getBoundaryElement())
           .withPlaceholderTemplate(placeholder)
           .withPreviewTemplate(preview)
-          .withPreviewContainer(this.previewContainer || 'global', this.disablePreviewPopover);
+          .withPreviewContainer(this.previewContainer || 'global');
 
         if (dir) {
           ref.withDirection(dir.value);
@@ -573,12 +559,10 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
       draggingDisabled,
       rootElementSelector,
       previewContainer,
-      disablePreviewPopover,
     } = config;
 
     this.disabled = draggingDisabled == null ? false : draggingDisabled;
     this.dragStartDelay = dragStartDelay || 0;
-    this.disablePreviewPopover = disablePreviewPopover || false;
 
     if (lockAxis) {
       this.lockAxis = lockAxis;