Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.1.0",
"@dnd-kit/sortable": "^7.1.0",
"@dnd-kit/utilities": "^3.2.2",
"@mui/icons-material": "^7.3.5",
"@mui/material": "^7.1.1",
"@reduxjs/toolkit": "^2.8.2",
Expand Down
245 changes: 153 additions & 92 deletions src/renderer/src/components/DeviceListScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@ 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,
PointerSensor,
closestCenter,
useSensor,
useSensors,
} from "@dnd-kit/core";
import {
SortableContext,
arrayMove,
rectSortingStrategy,
useSortable,
} from "@dnd-kit/sortable";
import { CSS } from "@dnd-kit/utilities";

import { AppDispatch } from "../store/store";
import {
Expand Down Expand Up @@ -68,6 +85,11 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>
const [reorderList, setReorderList] = useState<string[]>([]);
const [draggingId, setDraggingId] = useState<string | null>(null);
const [dragOverId, setDragOverId] = useState<string | null>(null);
const sensors = useSensors(
useSensor(PointerSensor, {
activationConstraint: { distance: 5 },
})
);

const orderedDevices = useMemo(() => {
const idToDevice = new Map<string, AnyDevice>();
Expand Down Expand Up @@ -163,66 +185,66 @@ 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);
};

const moveIdInList = (list: string[], fromId: string, toId: string) => {
if (fromId === toId) return list;
const fromIndex = list.indexOf(fromId);
const toIndex = list.indexOf(toId);
if (fromIndex === -1 || toIndex === -1) return list;
const next = [...list];
next.splice(fromIndex, 1);
next.splice(toIndex, 0, fromId);
return next;
};
}, [draggingId, isReorderMode]);

const handleDragStart = (id: string) => (event: React.DragEvent<HTMLDivElement>) => {
setDraggingId(id);
const handleDragStart = (event: DragStartEvent) => {
if (!isReorderMode) return;
setDraggingId(event.active.id as string);
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;
if (!overId || overId === draggingId) {
setDragOverId(null);
return;
}
setDragOverId(overId);
};

const handleDragLeave = () => {
setDragOverId(null);
};
const handleDragEnd = (event: DragEndEvent) => {
const { active, over } = event;
if (!isReorderMode) {
setDraggingId(null);
setDragOverId(null);
return;
}

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

if (draggingId && overId && draggingId !== overId) {
// Update the list only once on drop
setReorderList((prev) => moveIdInList(prev, draggingId, overId));
if (!over) {
setDragOverId(null);
return;
}

setDraggingId(null);
const activeId = active.id as string;
const overId = over.id as string;

setDragOverId(null);

if (activeId !== overId) {
setReorderList((prev) => {
const oldIndex = prev.indexOf(activeId);
const newIndex = prev.indexOf(overId);
if (oldIndex === -1 || newIndex === -1) return prev;
return arrayMove(prev, oldIndex, newIndex);
});
}
};

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

const SortableDeviceCard: React.FC<{ device: AnyDevice }> = ({ device }) => {
const { attributes, listeners, setNodeRef, transform, transition, isDragging, isOver } = useSortable({
id: device.deviceId,
disabled: !isReorderMode,
});

return (
<Box
ref={setNodeRef}
{...attributes}
{...listeners}
sx={{
position: "relative",
opacity: isDragging || draggingId === device.deviceId ? 0.5 : 1,

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

The opacity style is conditional on isDragging || draggingId === device.deviceId. The isDragging property from the useSortable hook is sufficient to determine if the item is being dragged. Using draggingId from the parent component's state is redundant here and can be removed for clarity.

Suggested change
opacity: isDragging || draggingId === device.deviceId ? 0.5 : 1,
opacity: isDragging ? 0.5 : 1,

transition: "opacity 0.2s ease",
outline: dragOverId === device.deviceId || isOver ? "2px solid" : "none",

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

The outline style is conditional on dragOverId === device.deviceId || isOver. The isOver property from the useSortable hook is sufficient to determine if a draggable item is over the current sortable item. Using dragOverId from the parent component's state is redundant here and can be removed for clarity.

Suggested change
outline: dragOverId === device.deviceId || isOver ? "2px solid" : "none",
outline: isOver ? "2px solid" : "none",

outlineColor: "primary.main",
outlineOffset: "2px",
borderRadius: 2,
cursor: "grab",
}}
style={{
transform: CSS.Transform.toString(transform),
transition,
}}
>
{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>
);
};

if (!apiTokenSet) {
return (
<Container sx={{ mt: 4 }}>
Expand Down Expand Up @@ -322,64 +395,52 @@ export const DeviceListScreen: React.FC<{ onDeviceSelect: (deviceId: string) =>
</Box>
)}

<motion.div
variants={containerVariants}
initial="hidden"
animate="visible"
<DndContext
sensors={sensors}
collisionDetection={closestCenter}
onDragStart={handleDragStart}
onDragOver={handleDragOver}
onDragEnd={handleDragEnd}
onDragCancel={handleDragCancel}
>
<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"
<motion.div variants={containerVariants} initial="hidden" animate="visible">
<Box sx={gridSx}>
<AnimatePresence>
{isReorderMode ? (
<SortableContext items={reorderList} strategy={rectSortingStrategy}>
{reorderList
.map((deviceId) => filteredDeviceMap.get(deviceId))
.filter((device): device is AnyDevice => !!device)
.map((device) => (
<motion.div variants={itemVariants} layout key={device.deviceId}>
<SortableDeviceCard device={device} />
</motion.div>
))}
</SortableContext>
) : (
filteredDevices.map((device) => (
<motion.div variants={itemVariants} layout key={device.deviceId}>
<Box
sx={{
position: "absolute",
top: 8,
right: 8,
bgcolor: "background.paper",
boxShadow: 1,
pointerEvents: "none",
position: "relative",
opacity: draggingId === device.deviceId ? 0.5 : 1,
transition: "opacity 0.2s ease",
Comment on lines +426 to +427

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

In the rendering logic for when isReorderMode is false, the Box component has an opacity style that depends on draggingId. However, draggingId is only set when isReorderMode is true, so it will always be null in this case. This makes the opacity and transition styles redundant and they can be removed for clarity.

borderRadius: 2,
}}
aria-label={t("Reorder")}
>
<DragIndicatorIcon fontSize="small" />
</IconButton>
)}
<DeviceCard
device={device}
status={statusMap[device.deviceId]}
onSelect={onDeviceSelect}
/>
</Box>
</motion.div>
))}
</AnimatePresence>
</Box>
</motion.div>
<DeviceCard
device={device}
status={statusMap[device.deviceId]}
onSelect={onDeviceSelect}
/>
</Box>
</motion.div>
))
)}
</AnimatePresence>
</Box>
</motion.div>
</DndContext>
</Container>
);
};
Loading