Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(cdk/drag-drop): preview positioned incorrectly when RTL is set on the body #29606

Merged
merged 1 commit into from
Aug 20, 2024
Merged

fix(cdk/drag-drop): preview positioned incorrectly when RTL is set on the body #29606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/cdk/drag-drop/directives/drop-list-shared.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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');
Expand All @@ -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)?)|(0(px)?)$/);
expect(preview.textContent!.trim())
.withContext('Expected preview content to match element')
.toContain('One');
Expand All @@ -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)?)|(0(px)?)$/);

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
Expand Down
15 changes: 12 additions & 3 deletions src/cdk/drag-drop/preview-ref.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']();
}
}
Expand Down Expand Up @@ -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',
Expand All @@ -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;
}
Loading