Skip to content

Add dnd-kit sorting with wheel scrolling support #14

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

Closed
0suu wants to merge 1 commit into from
Closed

Add dnd-kit sorting with wheel scrolling support #14

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
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@
"dependencies": {
"@emotion/react": "^11.14.0",
"@emotion/styled": "^11.14.0",
"@dnd-kit/core": "^6.3.1",
"@dnd-kit/sortable": "^8.0.1",
"@dnd-kit/utilities": "^3.2.2",
"@mui/icons-material": "^7.3.5",
"@mui/material": "^7.1.1",
"@reduxjs/toolkit": "^2.8.2",
Expand Down
205 changes: 125 additions & 80 deletions src/renderer/src/components/DeviceListScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@ import CircularProgress from "@mui/material/CircularProgress";
import Alert from "@mui/material/Alert";
import IconButton from "@mui/material/IconButton";
import { motion, AnimatePresence } from "framer-motion";
import {
DndContext,
DragEndEvent,
DragOverEvent,
DragStartEvent,
MouseSensor,
TouchSensor,
closestCenter,
useSensor,
useSensors,
} from "@dnd-kit/core";
import { SortableContext, rectSortingStrategy, useSortable } from "@dnd-kit/sortable";
import { CSS } from "@dnd-kit/utilities";

import { AppDispatch } from "../store/store";
import {
Expand Down Expand Up @@ -69,6 +82,11 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>
const [draggingId, setDraggingId] = useState<string | null>(null);
const [dragOverId, setDragOverId] = useState<string | null>(null);

const sensors = useSensors(
useSensor(MouseSensor, { activationConstraint: { distance: 5 } }),
useSensor(TouchSensor, { activationConstraint: { delay: 150, tolerance: 5 } })
);

const orderedDevices = useMemo(() => {
const idToDevice = new Map<string, AnyDevice>();
devices.forEach((device) => {
Expand Down Expand Up @@ -105,6 +123,18 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>
return map;
}, [filteredDevices]);

const displayedDevices = useMemo(
() =>
isReorderMode
? reorderList
.map((deviceId) => filteredDeviceMap.get(deviceId))
.filter((device): device is AnyDevice => !!device)
: filteredDevices,
[filteredDeviceMap, filteredDevices, isReorderMode, reorderList]
);

const sortableIds = useMemo(() => displayedDevices.map((device) => device.deviceId), [displayedDevices]);

const gridSx = useMemo(
() => ({
display: "grid",
Expand Down Expand Up @@ -163,23 +193,18 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>

// Enable wheel scrolling during drag
useEffect(() => {
if (!draggingId) return;
if (!draggingId || !isReorderMode) return;

const handleWheel = (e: WheelEvent) => {
// Allow default wheel behavior (scrolling) during drag
window.scrollBy(0, e.deltaY);
};

window.addEventListener('wheel', handleWheel, { passive: true });
window.addEventListener("wheel", handleWheel, { passive: true });

return () => {
window.removeEventListener('wheel', handleWheel);
window.removeEventListener("wheel", handleWheel);
};
}, [draggingId]);

const handleReorder = (visibleOrder: string[]) => {
setReorderList(visibleOrder);
};
}, [draggingId, isReorderMode]);

const moveIdInList = (list: string[], fromId: string, toId: string) => {
if (fromId === toId) return list;
Expand All @@ -192,37 +217,36 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>
return next;
};

const handleDragStart = (id: string) => (event: React.DragEvent<HTMLDivElement>) => {
const handleDragStart = (event: DragStartEvent) => {
if (!isReorderMode) return;
const id = event.active.id as string;

@gemini-code-assist gemini-code-assist Bot Nov 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using as string for the event ID is not type-safe, as dnd-kit's UniqueIdentifier type is string | number. If the ID were ever a number, this could cause issues with functions like moveIdInList that expect strings. It's safer to explicitly convert it to a string using String().

Suggested change
const id = event.active.id as string;
const id = String(event.active.id);

setDraggingId(id);
setDragOverId(null);
event.dataTransfer.effectAllowed = "move";
};

const handleDragOver = (overId: string) => (event: React.DragEvent<HTMLDivElement>) => {
event.preventDefault();
if (!draggingId || draggingId === overId) return;

// Only track hover state, don't update the list during drag
const handleDragOver = (event: DragOverEvent) => {
if (!isReorderMode) return;
const overId = event.over?.id as string | undefined;
const activeId = event.active.id as string;
if (!overId || activeId === overId) return;
setDragOverId(overId);
};
Comment on lines +227 to 233

@gemini-code-assist gemini-code-assist Bot Nov 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The current implementation of handleDragOver has a bug where the drag-over highlight (the outline) can get stuck on an item. This happens if you drag over an item and then drag away to an area with no droppable items, or if you drag over the item that is currently being dragged. To fix this and also improve type safety, I suggest replacing this handler with a more robust version.

  const handleDragOver = (event: DragOverEvent) => {
    if (!isReorderMode) return;
    const { active, over } = event;
    const overId = over ? String(over.id) : null;

    // Do not highlight the item being dragged
    if (String(active.id) === overId) {
      setDragOverId(null);
      return;
    }

    setDragOverId(overId);
  };


const handleDragLeave = () => {
setDragOverId(null);
};

const handleDrop = (overId: string) => (event: React.DragEvent<HTMLDivElement>) => {
event.preventDefault();

if (draggingId && overId && draggingId !== overId) {
// Update the list only once on drop
setReorderList((prev) => moveIdInList(prev, draggingId, overId));
const handleDragEnd = (event: DragEndEvent) => {
if (!isReorderMode) {
setDraggingId(null);
setDragOverId(null);
return;
}
const { active, over } = event;
if (over && active.id !== over.id) {
setReorderList((prev) => moveIdInList(prev, active.id as string, over.id as string));

@gemini-code-assist gemini-code-assist Bot Nov 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Similar to handleDragStart, using as string here is not type-safe. The IDs from the dnd-kit event should be explicitly converted to strings to ensure they match the expected type for moveIdInList.

      setReorderList((prev) => moveIdInList(prev, String(active.id), String(over.id)));

}

setDraggingId(null);
setDragOverId(null);
};

const handleDragEnd = () => {
const handleDragCancel = () => {
setDraggingId(null);
setDragOverId(null);
};
Expand Down Expand Up @@ -250,6 +274,60 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>
}
};

const SortableDeviceCard: React.FC<{ device: AnyDevice }> = ({ device }) => {

@gemini-code-assist gemini-code-assist Bot Nov 26, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For better code organization and maintainability, this SortableDeviceCard component should be extracted into its own file (e.g., src/renderer/src/components/SortableDeviceCard.tsx). This component contains significant presentational and behavioral logic, and placing it in a separate file will make DeviceListScreen cleaner and the new component more reusable.

const { attributes, listeners, setNodeRef, transform, transition } = useSortable({
id: device.deviceId,
disabled: !isReorderMode,
});

const style = {
transform: CSS.Transform.toString(transform),
transition,
};

return (
<motion.div variants={itemVariants} layout key={device.deviceId} style={style}>
<Box
ref={setNodeRef}
sx={{
position: "relative",
opacity: draggingId === device.deviceId ? 0.5 : 1,
transition: "opacity 0.2s ease",
outline: dragOverId === device.deviceId ? "2px solid" : "none",
outlineColor: "primary.main",
outlineOffset: "2px",
borderRadius: 2,
touchAction: isReorderMode ? "none" : "auto",
}}
{...attributes}
{...listeners}
>
{isReorderMode && (
<IconButton
size="small"
sx={{
position: "absolute",
top: 8,
right: 8,
bgcolor: "background.paper",
boxShadow: 1,
pointerEvents: "none",
}}
aria-label={t("Reorder")}
>
<DragIndicatorIcon fontSize="small" />
</IconButton>
)}
<DeviceCard
device={device}
status={statusMap[device.deviceId]}
onSelect={onDeviceSelect}
/>
</Box>
</motion.div>
);
};

if (!apiTokenSet) {
return (
<Container sx={{ mt: 4 }}>
Expand Down Expand Up @@ -327,58 +405,25 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>
initial="hidden"
animate="visible"
>
<Box sx={gridSx}>
<AnimatePresence>
{(isReorderMode
? reorderList
.map((deviceId) => filteredDeviceMap.get(deviceId))
.filter((device): device is AnyDevice => !!device)
: filteredDevices
).map((device) => (
<motion.div variants={itemVariants} layout key={device.deviceId}>
<Box
sx={{
position: "relative",
opacity: draggingId === device.deviceId ? 0.5 : 1,
transition: "opacity 0.2s ease",
outline: dragOverId === device.deviceId ? "2px solid" : "none",
outlineColor: "primary.main",
outlineOffset: "2px",
borderRadius: 2,
}}
draggable={isReorderMode}
onDragStart={isReorderMode ? handleDragStart(device.deviceId) : undefined}
onDragOver={isReorderMode ? handleDragOver(device.deviceId) : undefined}
onDragLeave={isReorderMode ? handleDragLeave : undefined}
onDrop={isReorderMode ? handleDrop(device.deviceId) : undefined}
onDragEnd={isReorderMode ? handleDragEnd : undefined}
>
{isReorderMode && (
<IconButton
size="small"
sx={{
position: "absolute",
top: 8,
right: 8,
bgcolor: "background.paper",
boxShadow: 1,
pointerEvents: "none",
}}
aria-label={t("Reorder")}
>
<DragIndicatorIcon fontSize="small" />
</IconButton>
)}
<DeviceCard
device={device}
status={statusMap[device.deviceId]}
onSelect={onDeviceSelect}
/>
</Box>
</motion.div>
))}
</AnimatePresence>
</Box>
<DndContext
sensors={sensors}
collisionDetection={closestCenter}
onDragStart={handleDragStart}
onDragOver={handleDragOver}
onDragEnd={handleDragEnd}
onDragCancel={handleDragCancel}
autoScroll={false}
>
<SortableContext items={sortableIds} strategy={rectSortingStrategy} disabled={!isReorderMode}>
<Box sx={gridSx}>
<AnimatePresence>
{displayedDevices.map((device) => (
<SortableDeviceCard key={device.deviceId} device={device} />
))}
</AnimatePresence>
</Box>
</SortableContext>
</DndContext>
</motion.div>
</Container>
);
Expand Down
Loading