Skip to content

Commit 93fb4fa

Browse files
arjunkomatharjunkomath
committed
Address code review
1 parent 8a61735 commit 93fb4fa

3 files changed

Lines changed: 93 additions & 52 deletions

File tree

‎apps/web/components/roadmap/RoadmapBoard.tsx‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
IRoadmapColumn,
55
} from "@changes-page/supabase/types/page";
66
import { useMemo, useState } from "react";
7-
import { useUserData } from "../../utils/useUser";
87
import RoadmapColumn from "./RoadmapColumn";
98
import RoadmapItemModal from "./RoadmapItemModal";
109
import { useRoadmapDragDrop } from "./hooks/useRoadmapDragDrop";
@@ -22,7 +21,6 @@ export default function RoadmapBoard({
2221
items: RoadmapItemWithRelations[];
2322
categories: IRoadmapCategory[];
2423
}) {
25-
const { supabase } = useUserData();
2624
const [boardItems, setBoardItems] = useState(items);
2725

2826
const itemsByColumn: ItemsByColumn = useMemo(() => {
@@ -36,13 +34,11 @@ export default function RoadmapBoard({
3634
}, [columns, boardItems]);
3735

3836
const dragDropHandlers = useRoadmapDragDrop({
39-
supabase,
4037
itemsByColumn,
4138
setBoardItems,
4239
});
4340

4441
const itemHandlers = useRoadmapItems({
45-
supabase,
4642
board,
4743
categories,
4844
itemsByColumn,

‎apps/web/components/roadmap/hooks/useRoadmapDragDrop.ts‎

Lines changed: 86 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,29 @@
1-
import { useState } from "react";
2-
import { SupabaseClient } from "@supabase/supabase-js";
3-
import { RoadmapItemWithRelations, ItemsByColumn, DragOverPosition } from "../types";
4-
5-
interface UseRoadmapDragDropProps {
6-
supabase: SupabaseClient;
7-
itemsByColumn: ItemsByColumn;
8-
setBoardItems: React.Dispatch<React.SetStateAction<RoadmapItemWithRelations[]>>;
9-
}
1+
import { Dispatch, SetStateAction, useState } from "react";
2+
import { useUserData } from "../../../utils/useUser";
3+
import {
4+
DragOverPosition,
5+
ItemsByColumn,
6+
RoadmapItemWithRelations,
7+
} from "../types";
108

119
export function useRoadmapDragDrop({
12-
supabase,
1310
itemsByColumn,
1411
setBoardItems,
15-
}: UseRoadmapDragDropProps) {
16-
const [draggedItem, setDraggedItem] = useState<RoadmapItemWithRelations | null>(null);
12+
}: {
13+
itemsByColumn: ItemsByColumn;
14+
setBoardItems: Dispatch<SetStateAction<RoadmapItemWithRelations[]>>;
15+
}) {
16+
const { supabase } = useUserData();
17+
const [draggedItem, setDraggedItem] =
18+
useState<RoadmapItemWithRelations | null>(null);
1719
const [dragOverColumn, setDragOverColumn] = useState<string | null>(null);
18-
const [dragOverPosition, setDragOverPosition] = useState<DragOverPosition | null>(null);
20+
const [dragOverPosition, setDragOverPosition] =
21+
useState<DragOverPosition | null>(null);
1922

20-
const handleDragStart = (e: React.DragEvent, item: RoadmapItemWithRelations) => {
23+
const handleDragStart = (
24+
e: React.DragEvent,
25+
item: RoadmapItemWithRelations
26+
) => {
2127
setDraggedItem(item);
2228
e.dataTransfer.effectAllowed = "move";
2329
(e.target as HTMLElement).style.opacity = "0.5";
@@ -73,9 +79,16 @@ export function useRoadmapDragDrop({
7379
const targetColumnItems = itemsByColumn[targetColumnId] || [];
7480

7581
if (sourceColumnId === targetColumnId) {
76-
await handleSameColumnReorder(sourceColumnItems, currentDragOverPosition);
82+
await handleSameColumnReorder(
83+
sourceColumnItems,
84+
currentDragOverPosition
85+
);
7786
} else {
78-
await handleCrossColumnMove(targetColumnItems, targetColumnId, currentDragOverPosition);
87+
await handleCrossColumnMove(
88+
targetColumnItems,
89+
targetColumnId,
90+
currentDragOverPosition
91+
);
7992
}
8093
} catch (error) {
8194
console.error("Error moving item:", error);
@@ -93,10 +106,18 @@ export function useRoadmapDragDrop({
93106
return;
94107
}
95108

96-
const draggedIndex = sourceColumnItems.findIndex(item => item.id === draggedItem.id);
97-
const targetIndex = sourceColumnItems.findIndex(item => item.id === currentDragOverPosition.itemId);
109+
const draggedIndex = sourceColumnItems.findIndex(
110+
(item) => item.id === draggedItem.id
111+
);
112+
const targetIndex = sourceColumnItems.findIndex(
113+
(item) => item.id === currentDragOverPosition.itemId
114+
);
98115

99-
if (draggedIndex === -1 || targetIndex === -1 || draggedIndex === targetIndex) {
116+
if (
117+
draggedIndex === -1 ||
118+
targetIndex === -1 ||
119+
draggedIndex === targetIndex
120+
) {
100121
return;
101122
}
102123

@@ -107,18 +128,26 @@ export function useRoadmapDragDrop({
107128
if (currentDragOverPosition.position === "after") {
108129
insertIndex = targetIndex + 1;
109130
}
110-
if (draggedIndex < targetIndex && currentDragOverPosition.position === "before") {
131+
if (
132+
draggedIndex < targetIndex &&
133+
currentDragOverPosition.position === "before"
134+
) {
111135
insertIndex = targetIndex - 1;
112136
}
113-
if (draggedIndex < targetIndex && currentDragOverPosition.position === "after") {
137+
if (
138+
draggedIndex < targetIndex &&
139+
currentDragOverPosition.position === "after"
140+
) {
114141
insertIndex = targetIndex;
115142
}
116143

117144
reorderedItems.splice(insertIndex, 0, draggedItemData);
118145

119-
setBoardItems(prev => {
120-
return prev.map(item => {
121-
const updatedIndex = reorderedItems.findIndex(reorderedItem => reorderedItem.id === item.id);
146+
setBoardItems((prev) => {
147+
return prev.map((item) => {
148+
const updatedIndex = reorderedItems.findIndex(
149+
(reorderedItem) => reorderedItem.id === item.id
150+
);
122151
if (updatedIndex !== -1) {
123152
return { ...item, position: updatedIndex + 1 };
124153
}
@@ -153,18 +182,25 @@ export function useRoadmapDragDrop({
153182
let newPosition = 1;
154183

155184
if (!currentDragOverPosition) {
156-
newPosition = targetColumnItems.length > 0
157-
? Math.max(...targetColumnItems.map(item => item.position || 0)) + 1
158-
: 1;
185+
newPosition =
186+
targetColumnItems.length > 0
187+
? Math.max(...targetColumnItems.map((item) => item.position || 0)) + 1
188+
: 1;
159189
} else {
160-
const targetItem = targetColumnItems.find(item => item.id === currentDragOverPosition.itemId);
190+
const targetItem = targetColumnItems.find(
191+
(item) => item.id === currentDragOverPosition.itemId
192+
);
161193
if (targetItem) {
162194
if (currentDragOverPosition.position === "before") {
163195
newPosition = targetItem.position;
164-
const itemsToShift = targetColumnItems.filter(item => item.position >= targetItem.position);
196+
const itemsToShift = targetColumnItems.filter(
197+
(item) => item.position >= targetItem.position
198+
);
165199
if (itemsToShift.length > 0) {
166200
// Sort items in descending position order to avoid uniqueness conflicts
167-
const sortedItems = itemsToShift.sort((a, b) => (b.position || 0) - (a.position || 0));
201+
const sortedItems = itemsToShift.sort(
202+
(a, b) => (b.position || 0) - (a.position || 0)
203+
);
168204
for (const item of sortedItems) {
169205
await supabase
170206
.from("roadmap_items")
@@ -174,10 +210,14 @@ export function useRoadmapDragDrop({
174210
}
175211
} else {
176212
newPosition = targetItem.position + 1;
177-
const itemsToShift = targetColumnItems.filter(item => item.position > targetItem.position);
213+
const itemsToShift = targetColumnItems.filter(
214+
(item) => item.position > targetItem.position
215+
);
178216
if (itemsToShift.length > 0) {
179217
// Sort items in descending position order to avoid uniqueness conflicts
180-
const sortedItems = itemsToShift.sort((a, b) => (b.position || 0) - (a.position || 0));
218+
const sortedItems = itemsToShift.sort(
219+
(a, b) => (b.position || 0) - (a.position || 0)
220+
);
181221
for (const item of sortedItems) {
182222
await supabase
183223
.from("roadmap_items")
@@ -201,8 +241,8 @@ export function useRoadmapDragDrop({
201241

202242
if (error) throw error;
203243

204-
setBoardItems(prev =>
205-
prev.map(item => {
244+
setBoardItems((prev) =>
245+
prev.map((item) => {
206246
if (item.id === draggedItem.id) {
207247
return {
208248
...item,
@@ -211,12 +251,20 @@ export function useRoadmapDragDrop({
211251
};
212252
}
213253
if (item.column_id === targetColumnId && currentDragOverPosition) {
214-
const targetItem = targetColumnItems.find(ti => ti.id === currentDragOverPosition.itemId);
254+
const targetItem = targetColumnItems.find(
255+
(ti) => ti.id === currentDragOverPosition.itemId
256+
);
215257
if (targetItem) {
216-
if (currentDragOverPosition.position === "before" && item.position >= targetItem.position) {
258+
if (
259+
currentDragOverPosition.position === "before" &&
260+
item.position >= targetItem.position
261+
) {
217262
return { ...item, position: item.position + 1 };
218263
}
219-
if (currentDragOverPosition.position === "after" && item.position > targetItem.position) {
264+
if (
265+
currentDragOverPosition.position === "after" &&
266+
item.position > targetItem.position
267+
) {
220268
return { ...item, position: item.position + 1 };
221269
}
222270
}
@@ -238,4 +286,4 @@ export function useRoadmapDragDrop({
238286
handleItemDragOver,
239287
handleDrop,
240288
};
241-
}
289+
}

‎apps/web/components/roadmap/hooks/useRoadmapItems.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,25 @@ import {
22
IRoadmapBoard,
33
IRoadmapCategory,
44
} from "@changes-page/supabase/types/page";
5-
import { SupabaseClient } from "@supabase/supabase-js";
65
import { useState } from "react";
6+
import { useUserData } from "../../../utils/useUser";
77
import {
88
FormErrors,
99
ItemForm,
1010
ItemsByColumn,
1111
RoadmapItemWithRelations,
1212
} from "../types";
1313

14-
interface UseRoadmapItemsProps {
15-
supabase: SupabaseClient;
16-
board: IRoadmapBoard;
17-
categories: IRoadmapCategory[];
18-
itemsByColumn: ItemsByColumn;
19-
}
20-
2114
export function useRoadmapItems({
22-
supabase,
2315
board,
2416
categories,
2517
itemsByColumn,
26-
}: UseRoadmapItemsProps) {
18+
}: {
19+
board: IRoadmapBoard;
20+
categories: IRoadmapCategory[];
21+
itemsByColumn: ItemsByColumn;
22+
}) {
23+
const { supabase } = useUserData();
2724
const [showItemModal, setShowItemModal] = useState(false);
2825
const [selectedColumnId, setSelectedColumnId] = useState<string | null>(null);
2926
const [editingItem, setEditingItem] =

0 commit comments

Comments
 (0)