fix(cdk/drag-drop): preview positioned incorrectly when RTL is set on…

… the body As of #28945 we use a popover to display the preview so that it's always on top. To do so we need to push the popover from its default position at the center to the top/left which is done using `margin: auto`. Since we were setting `margin: 0`, the element was ending up at top/right in RTL, if `dir="rtl"` is set on the `html` or `body`. These changes fix the issue by pushing the element to the top/left using `margin-right: auto`. Fixes #29604.
angular · Aug 19, 2024 · 996f1cf · 996f1cf
1 parent d32c826
commit 996f1cf
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 9 deletions.
diff --git a/src/cdk/drag-drop/directives/drop-list-shared.spec.ts b/src/cdk/drag-drop/directives/drop-list-shared.spec.ts
@@ -834,7 +834,6 @@ export function defineCommonDropListTests(config: {
 
       const preview = document.querySelector('.cdk-drag-preview') as HTMLElement;
       const previewRect = preview.getBoundingClientRect();
-      const zeroPxRegex = /^0(px)?$/;
 
       expect(item.parentNode)
         .withContext('Expected element to be moved out into the body')
@@ -846,10 +845,9 @@ export function defineCommonDropListTests(config: {
         .withContext('Expect element position to be !important')
         .toBe('important');
       // 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.
       expect(item.style.top)
         .withContext('Expected element to be removed from layout')
-        .toMatch(zeroPxRegex);
+        .toMatch(/^0(px)?$/);
       expect(item.style.left)
         .withContext('Expected element to be removed from layout')
         .toBe('-999em');
@@ -860,7 +858,7 @@ export function defineCommonDropListTests(config: {
         .toBe('manual');
       expect(preview.style.margin)
         .withContext('Expected preview to reset the margin')
-        .toMatch(zeroPxRegex);
+        .toMatch(/^0(px)? auto 0(px)? 0(px)?$/);
       expect(preview.textContent!.trim())
         .withContext('Expected preview content to match element')
         .toContain('One');
@@ -880,10 +878,9 @@ export function defineCommonDropListTests(config: {
         .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.
       expect(preview.style.margin)
         .withContext('Expected the preview margin to be reset.')
-        .toMatch(zeroPxRegex);
+        .toMatch(/^0(px)? auto 0(px)? 0(px)?$/);
 
       dispatchMouseEvent(document, 'mouseup');
       fixture.detectChanges();

diff --git a/src/cdk/drag-drop/preview-ref.ts b/src/cdk/drag-drop/preview-ref.ts
@@ -64,7 +64,7 @@ export class PreviewRef {
 
     // The null check is necessary for browsers that don't support the popover API.
     // Note that we use a string access for compatibility with Closure.
-    if ('showPopover' in this._preview) {
+    if (supportsPopover(this._preview)) {
       this._preview['showPopover']();
     }
   }
@@ -139,8 +139,12 @@ export class PreviewRef {
         // It's important that we disable the pointer events on the preview, because
         // it can throw off the `document.elementFromPoint` calls in the `CdkDropList`.
         'pointer-events': 'none',
-        // We have to reset the margin, because it can throw off positioning relative to the viewport.
-        'margin': '0',
+        // If the preview has a margin, it can throw off our positioning so we reset it. The reset
+        // value for `margin-right` needs to be `auto` when opened as a popover, because our
+        // positioning is always top/left based, but native popover seems to position itself
+        // to the top/right if `<html>` or `<body>` have `dir="rtl"` (see #29604). Setting it
+        // to `auto` pushed it to the top/left corner in RTL and is a noop in LTR.
+        'margin': supportsPopover(preview) ? '0 auto 0 0' : '0',
         'position': 'fixed',
         'top': '0',
         'left': '0',
@@ -165,3 +169,8 @@ export class PreviewRef {
     return preview;
   }
 }
+
+/** Checks whether a specific element supports the popover API. */
+function supportsPopover(element: HTMLElement): boolean {
+  return 'showPopover' in element;
+}