Skip to content

Commit

Permalink
Always set proxy (#1499)
Browse files Browse the repository at this point in the history
* refactor: move generateUniqueId to dom utilities

* fix: always set proxy for unique droppables that share elements

The IDENTIFIER_ATTRIBUTE was getting overwritten when multiple droppables shared the same element. Instead, we generate a unique data attribute for each droppable using generateUniqueId. We can't reuse the element's ID, since this may contain illegal characters.
  • Loading branch information
chrisvxd authored Oct 11, 2024
1 parent 50a1f35 commit d436037
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/always-set-proxy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@dnd-kit/dom': patch
---

Fix a bug that prevented all unique droppables that share an element from each receiving the cloned proxy.
4 changes: 2 additions & 2 deletions packages/dom/src/core/plugins/accessibility/Accessibility.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {effects} from '@dnd-kit/state';
import {Plugin} from '@dnd-kit/abstract';
import {getWindow, isSafari} from '@dnd-kit/dom/utilities';
import {getWindow, isSafari, generateUniqueId} from '@dnd-kit/dom/utilities';

import type {DragDropManager} from '../../manager/index.ts';
import {
Expand All @@ -11,7 +11,7 @@ import {
defaultScreenReaderInstructions,
} from './defaults.ts';
import type {Announcements, ScreenReaderInstructions} from './types.ts';
import {generateUniqueId, isFocusable} from './utilities.ts';
import {isFocusable} from './utilities.ts';
import {createHiddenText} from './HiddenText.ts';
import {createLiveRegion} from './LiveRegion.ts';

Expand Down
9 changes: 0 additions & 9 deletions packages/dom/src/core/plugins/accessibility/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,3 @@ export function isFocusable(element: Element) {
element instanceof window.HTMLAreaElement
);
}

const ids: Record<string, number> = {};

export function generateUniqueId(prefix: string) {
const id = ids[prefix] == null ? 0 : ids[prefix] + 1;
ids[prefix] = id;

return `${prefix}-${id}`;
}
24 changes: 12 additions & 12 deletions packages/dom/src/core/plugins/feedback/Feedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ import {
isSafari,
getWindow,
type Transform,
generateUniqueId,
} from '@dnd-kit/dom/utilities';
import {Coordinates} from '@dnd-kit/geometry';

import type {DragDropManager} from '../../manager/index.ts';
import type {Draggable} from '../../entities/index.ts';
import type {Draggable, Droppable} from '../../entities/index.ts';

const ATTR_PREFIX = 'data-dnd-';
const CSS_PREFIX = '--dnd-';
const ATTRIBUTE = `${ATTR_PREFIX}dragging`;
const cssRules = `[${ATTRIBUTE}] {position: fixed !important;pointer-events: none !important;touch-action: none !important;z-index: calc(infinity);will-change: translate;top: var(${CSS_PREFIX}top, 0px) !important;left: var(${CSS_PREFIX}left, 0px) !important;width: var(${CSS_PREFIX}width, auto) !important;height: var(${CSS_PREFIX}height, auto) !important;box-sizing:border-box;}[${ATTRIBUTE}] *{pointer-events: none !important;}[${ATTRIBUTE}][style*="${CSS_PREFIX}translate"] {translate: var(${CSS_PREFIX}translate) !important;}[style*="${CSS_PREFIX}transition"] {transition: var(${CSS_PREFIX}transition) !important;}*:where([${ATTRIBUTE}][popover]){overflow:visible;background:var(${CSS_PREFIX}background);border:var(${CSS_PREFIX}border);margin:unset;padding:unset;color:inherit;}[${ATTRIBUTE}]::backdrop {display: none}html:has([${ATTRIBUTE}]) * {user-select:none;-webkit-user-select:none;}`;
const PLACEHOLDER_ATTRIBUTE = `${ATTR_PREFIX}placeholder`;
const IDENTIFIER_ATTRIBUTE = `${ATTR_PREFIX}id`;
const IGNORED_ATTRIBUTES = [ATTRIBUTE, PLACEHOLDER_ATTRIBUTE, 'popover'];
const IGNORED_STYLES = ['view-transition-name'];

Expand Down Expand Up @@ -511,7 +511,7 @@ function createPlaceholder(source: Draggable) {
if (!element || !manager) return;

const {droppables} = manager.registry;
const containedDroppables = [];
const containedDroppables = new Map<Droppable, string>();

for (const droppable of droppables) {
if (!droppable.element) continue;
Expand All @@ -520,34 +520,34 @@ function createPlaceholder(source: Draggable) {
element === droppable.element ||
element.contains(droppable.element)
) {
droppable.element.setAttribute(
IDENTIFIER_ATTRIBUTE,
`${JSON.stringify(droppable.id)}`
);
containedDroppables.push(droppable);
const identifierAttribute = `${ATTR_PREFIX}${generateUniqueId('dom-id')}`;

droppable.element.setAttribute(identifierAttribute, '');

containedDroppables.set(droppable, identifierAttribute);
}
}

const cleanup: CleanupFunction[] = [];
const placeholder = cloneElement(element);
const {remove} = placeholder;

for (const droppable of containedDroppables) {
for (const [droppable, identifierAttribute] of containedDroppables) {
if (!droppable.element) continue;

const selector = `[${IDENTIFIER_ATTRIBUTE}='${JSON.stringify(droppable.id)}']`;
const selector = `[${identifierAttribute}]`;
const clonedElement = placeholder.matches(selector)
? placeholder
: placeholder.querySelector(selector);

droppable.element?.removeAttribute(IDENTIFIER_ATTRIBUTE);
droppable.element?.removeAttribute(identifierAttribute);

if (!clonedElement) continue;

let current = droppable.element;

droppable.proxy = clonedElement;
clonedElement.removeAttribute(IDENTIFIER_ATTRIBUTE);
clonedElement.removeAttribute(identifierAttribute);

ProxiedElements.set(current, clonedElement);

Expand Down
2 changes: 2 additions & 0 deletions packages/dom/src/utilities/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,5 @@ export {
parseTranslate,
} from './transform/index.ts';
export type {Transform} from './transform/index.ts';

export {generateUniqueId} from './misc/generateUniqueId.ts';
8 changes: 8 additions & 0 deletions packages/dom/src/utilities/misc/generateUniqueId.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const ids: Record<string, number> = {};

export function generateUniqueId(prefix: string) {
const id = ids[prefix] == null ? 0 : ids[prefix] + 1;
ids[prefix] = id;

return `${prefix}-${id}`;
}

0 comments on commit d436037

Please sign in to comment.