Skip to content

fix(DragDropSort): enable mobile drag-and-drop support #12418

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

Open
logonoff wants to merge 1 commit into
base: main
Choose a base branch
from
+8 −3
Open

fix(DragDropSort): enable mobile drag-and-drop support #12418

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
6 changes: 5 additions & 1 deletion packages/react-drag-drop/src/components/DragDrop/DragDropContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
DragCancelEvent
} from '@dnd-kit/core';
import { arrayMove, sortableKeyboardCoordinates } from '@dnd-kit/sortable';
import { restrictToWindowEdges } from '@dnd-kit/modifiers';
import { Draggable } from './Draggable';
import { DraggableDataListItem } from './DraggableDataListItem';
import { DraggableDualListSelectorListItem } from './DraggableDualListSelectorListItem';
Expand Down Expand Up @@ -103,7 +104,9 @@ export const DragDropContainer: React.FunctionComponent<DragDropContainerProps>
);

const sensors = useSensors(
useSensor(PointerSensor),
useSensor(PointerSensor, {
activationConstraint: { distance: 8 }
}),
useSensor(KeyboardSensor, {
coordinateGetter: sortableKeyboardCoordinates
})
Expand Down Expand Up @@ -298,6 +301,7 @@ export const DragDropContainer: React.FunctionComponent<DragDropContainerProps>
<DndContext
sensors={sensors}
collisionDetection={collisionDetectionStrategy}
modifiers={[restrictToWindowEdges]}
onDragEnd={handleDragEnd}
onDragOver={handleDragOver}
onDragStart={handleDragStart}
Expand Down
5 changes: 3 additions & 2 deletions packages/react-drag-drop/src/components/DragDrop/Draggable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export const Draggable: React.FunctionComponent<DraggableProps> = ({

const style = {
transform: CSS.Transform.toString(transform),
transition
transition,
...(!useDragButton && { touchAction: 'none' as const })
};
Comment on lines 29 to 33
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether Draggable is currently used with an explicit style prop in this repository.
# If matches exist, those call sites are directly exposed to this override bug.
rg -nP --type=tsx '<Draggable\b[^>]*\bstyle\s*='
rg -nP --type=tsx 'items\s*=\s*{[\s\S]*?\bprops\s*:\s*{[\s\S]*?\bstyle\s*:' -U

Repository: patternfly/patternfly-react

Length of output: 1970

🏁 Script executed:

# Locate the Draggable.tsx file
find . -name "Draggable.tsx" -type f 2>/dev/null | head -20

Repository: patternfly/patternfly-react

Length of output: 207

🏁 Script executed:

# Once located, read the full component to see lines 29-53 and verify the structure
# This will help verify the spreading order concern
cat -n packages/react-drag-drop/src/components/DragDrop/Draggable.tsx 2>/dev/null || echo "File not found at expected path"

Repository: patternfly/patternfly-react

Length of output: 2155

🏁 Script executed:

# Search for DraggableProps definition
rg "interface DraggableProps|type DraggableProps" -A 10 2>/dev/null || echo "Definition not found with rg"

Repository: patternfly/patternfly-react

Length of output: 2294

🏁 Script executed:

# Search for Draggable component usages with explicit style prop
rg '<Draggable\b[^>]*\bstyle\s*=' --type=tsx --type=ts

Repository: patternfly/patternfly-react

Length of output: 99

🏁 Script executed:

# Also search in JSX/TSX for Draggable without explicit opening, to find all usages
rg 'Draggable' --type=tsx --type=ts -B 2 -A 5 | grep -A 7 '<Draggable'

Repository: patternfly/patternfly-react

Length of output: 99

🏁 Script executed:

# Search for Draggable component usages with style prop using globbing
rg '<Draggable\b[^>]*\bstyle\s*=' -g '*.tsx' -g '*.ts'

Repository: patternfly/patternfly-react

Length of output: 365

🏁 Script executed:

# Also search more broadly for any Draggable usage
rg '<Draggable' -g '*.tsx' -g '*.ts' -B 1 -A 3 | head -100

Repository: patternfly/patternfly-react

Length of output: 10297

🏁 Script executed:

# Check where Draggable is exported/imported from
rg "from.*Draggable|import.*Draggable" -g '*.tsx' -g '*.ts' | head -30

Repository: patternfly/patternfly-react

Length of output: 3263

🏁 Script executed:

# Check React.CSSProperties type usage in codebase for reference
rg "React\.CSSProperties" -g '*.tsx' -g '*.ts' | head -10

Repository: patternfly/patternfly-react

Length of output: 1146

🏁 Script executed:

# Check how other components handle style prop merging in this repo
rg "propStyle|styleProps|\.style" -g '*.tsx' -g '*.ts' -B 2 -A 2 | grep -A 4 "propStyle\|const.*style.*=" | head -40

Repository: patternfly/patternfly-react

Length of output: 3612

props.style can override drag-critical styles (including touchAction)

Because ...props is applied after style on Line 40 and Line 52, any caller-provided style replaces transform, transition, and touchAction. This breaks dragging and reintroduces the mobile failure this PR is fixing.

Suggested fix 
 export const Draggable: React.FunctionComponent<DraggableProps> = ({
   children,
   id,
   className,
   useDragButton = false,
+  style: propStyle,
   ...props
 }: DraggableProps) => {
@@
-  const style = {
-    transform: CSS.Transform.toString(transform),
-    transition,
-    ...(!useDragButton && { touchAction: 'none' as const })
-  };
+  const style = {
+    ...(propStyle as React.CSSProperties),
+    transform: CSS.Transform.toString(transform),
+    transition,
+    ...(!useDragButton ? { touchAction: 'none' as const } : {})
+  };

Also applies to: 49-52

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react-drag-drop/src/components/DragDrop/Draggable.tsx` around lines
29 - 33, The current style object lets caller-provided props.style overwrite
drag-critical CSS (transform, transition, touchAction) because props is spread
after computed style; fix by ensuring computed drag styles take precedence: when
building the style object (the local variable named style in Draggable.tsx),
spread props.style first and then set transform:
CSS.Transform.toString(transform), transition, and ...( !useDragButton && {
touchAction: 'none' }), so the computed values override any caller values; apply
the same change to both places where style is constructed so
transform/transition/touchAction cannot be overridden by props.style.


return useDragButton ? (
Expand All @@ -38,7 +39,7 @@ export const Draggable: React.FunctionComponent<DraggableProps> = ({
style={style}
{...props}
>
<DragButton {...attributes} {...listeners} />
<DragButton {...attributes} {...listeners} style={{ touchAction: 'none' }} />
{children}
</div>
) : (
Expand Down
Loading