dragging by bart-jaskulski · Pull Request #2 · bart-jaskulski/micro-step

bart-jaskulski · 2025-11-25T22:10:25Z

No description provided.

Copilot

Pull request overview

This PR implements drag-and-drop functionality for task reordering in a hierarchical task list. The implementation introduces a new DragProvider component that manages drag state, visual feedback, and drop calculations.

Key Changes:

New DragProvider component with pointer-based drag-and-drop state management, auto-scrolling, and accessibility announcements
Integration of drag-and-drop into the task list with drop position calculation (above/below/inside) and tree validation to prevent invalid moves
Visual enhancements including drag overlays, drop indicators, and transition animations

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
src/components/DragProvider.tsx	New context provider managing drag state, pointer events, auto-scroll, and accessibility features
src/components/TasksList.tsx	Integrated DragProvider with drop handling logic and drag overlay rendering
src/components/TaskItem.tsx	Added drag handle with pointer event binding
src/components/TaskItem.css	Added drag handle styling with touch-action and user-select properties, plus unused drag-shadow class
src/components/TasksList.css	Added visual feedback for drag states, drop indicators, overlay styling, and sr-only utility class

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-25T22:17:19Z

src/components/DragProvider.tsx

+      if (y > height - HOT_ZONE) {
+        window.scrollBy(0, 6 + (y - (height - HOT_ZONE)) / 8);
+      } else if (y < HOT_ZONE) {
+        window.scrollBy(0, -(6 + (HOT_ZONE - y) / 8));


[nitpick] The auto-scroll calculation uses magic numbers 6 and 8 without explanation. The formula 6 + (y - (height - HOT_ZONE)) / 8 determines scroll speed but the constants lack clarity. Consider extracting these as named constants (e.g., BASE_SCROLL_SPEED = 6, SCROLL_ACCELERATION = 8) to improve code readability and maintainability.

Copilot · 2025-11-25T22:17:20Z

src/components/TaskItem.css

+.task-item__drag-shadow {
+  background: var(--surface, #fff);
+  border-radius: var(--radius-lg, 16px);
+  padding: 0.75rem 1rem;
+  box-shadow: 0 24px 60px -32px rgba(15, 23, 42, 0.5);
+  border: 1px solid var(--border-subtle, #d8e2ec);
+}


The .task-item__drag-shadow class is defined but never appears to be used in the codebase. The drag overlay uses .drag-overlay__card instead (defined in TasksList.css). This unused CSS should be removed to avoid confusion and reduce code bloat.

Suggested change

.task-item__drag-shadow {

background: var(--surface, #fff);

border-radius: var(--radius-lg, 16px);

padding: 0.75rem 1rem;

box-shadow: 0 24px 60px -32px rgba(15, 23, 42, 0.5);

border: 1px solid var(--border-subtle, #d8e2ec);

}

Copilot · 2025-11-25T22:17:20Z

src/components/TasksList.css

+.sr-only {
+  position: absolute;
+  width: 1px;
+  height: 1px;
+  padding: 0;
+  margin: -1px;
+  overflow: hidden;
+  clip: rect(0, 0, 0, 0);
+  white-space: nowrap;
+  border-width: 0;
+}


[nitpick] The .sr-only class is defined in TasksList.css but is used for the announcer element in DragProvider.tsx (line 248). This creates a dependency between the CSS file and the component that isn't obvious. Consider moving this utility class to a global CSS file or creating a dedicated accessibility utilities CSS file to make the dependency clearer and allow reuse across components.

Copilot · 2025-11-25T22:17:20Z

src/components/DragProvider.tsx

+    window.addEventListener("pointermove", handlePointerMove, { passive: false });
+    window.addEventListener("pointerup", finishDrag);


The finishDrag function is added as a pointerup listener but also tries to release pointer capture. However, if the user moves the pointer outside the window and releases, or if there's an interaction that causes the pointer capture to be lost (like a browser dialog), the event listeners could remain attached. Consider also listening for pointercancel events to handle these edge cases and ensure cleanup happens correctly.

Copilot · 2025-11-25T22:17:20Z

src/components/TasksList.tsx

+  const renderOverlay = (id: string | null) => {
+    if (!id) return null;
+
+    const findNode = (nodes: TreeNode[]): TreeNode | undefined => {
+      for (const node of nodes) {
+        if (node.id === id) return node;
+        const nested = findNode(node.children);
+        if (nested) return nested;
+      }
+      return undefined;
+    };
+
+    const node = findNode(tasksStore());
+    if (!node) return null;
+
+    return (
+      <div class="drag-overlay__card">
+        {node.text}
+      </div>
+    );
+  };


The renderOverlay function performs a recursive tree search on every render when state.draggedId is not null. Since this is called within the Portal's Show component, it will be re-evaluated whenever the drag state changes. This could be optimized by memoizing the result or by indexing the tree once during drag start rather than searching repeatedly.

Copilot · 2025-11-25T22:17:22Z

src/components/TasksList.tsx

+  const handleDrop = (draggedId: string, targetId: string, position: Exclude<DropPosition, null>) => {
+    const tree = tasksStore();
+
+    const nodeIndex = new Map<string, TreeNode>();
+    const parentIndex = new Map<string, string | null>();
+
+    const indexTree = (nodes: TreeNode[], parentId: string | null) => {
+      nodes.forEach(node => {
+        nodeIndex.set(node.id, node);
+        parentIndex.set(node.id, parentId);
+        indexTree(node.children, node.id);
+      });
+    };
+
+    indexTree(tree, null);


[nitpick] The handleDrop function rebuilds the node and parent indexes from scratch on every drop operation. While this ensures data freshness, the same indexing logic is duplicated in renderOverlay (lines 89-96). Consider extracting this indexing logic into a shared utility function or memoizing it to avoid code duplication and improve maintainability.

Copilot · 2025-11-25T22:17:23Z

src/components/DragProvider.tsx

+    if (state.status === "DRAGGING") {
+      setState("status", "DROPPING");
+      announce("Drop complete");
+      window.setTimeout(resetState, 220);


[nitpick] Magic number for timeout duration. The timeout value 220 ms is used without explanation and should match or relate to the transition durations elsewhere in the code. For example, the "DROPPING" animation transition is 160ms (line 204), but the reset happens after 220ms. While this provides a buffer, using a named constant like DROP_ANIMATION_DURATION + BUFFER would make the relationship and intent clearer.

Copilot · 2025-11-25T22:17:23Z

src/components/DragProvider.tsx

+const calculateDropPosition = (pointerY: number, rect: DOMRect): Exclude<DropPosition, null> => {
+  const relativeY = (pointerY - rect.top) / rect.height;
+  if (relativeY < 0.25) return "above";
+  if (relativeY > 0.75) return "below";


[nitpick] The drop position thresholds (0.25 and 0.75) are hardcoded magic numbers. These define when the drop target switches from "above" to "inside" to "below". Consider extracting these as named constants like DROP_ABOVE_THRESHOLD = 0.25 and DROP_BELOW_THRESHOLD = 0.75 to make the behavior more explicit and easier to adjust.

Suggested change

const calculateDropPosition = (pointerY: number, rect: DOMRect): Exclude<DropPosition, null> => {

const relativeY = (pointerY - rect.top) / rect.height;

if (relativeY < 0.25) return "above";

if (relativeY > 0.75) return "below";

const DROP_ABOVE_THRESHOLD = 0.25;

const DROP_BELOW_THRESHOLD = 0.75;

const calculateDropPosition = (pointerY: number, rect: DOMRect): Exclude<DropPosition, null> => {

const relativeY = (pointerY - rect.top) / rect.height;

if (relativeY < DROP_ABOVE_THRESHOLD) return "above";

if (relativeY > DROP_BELOW_THRESHOLD) return "below";

Copilot · 2025-11-25T22:17:23Z

src/components/TasksList.css

  display: flex;
  flex-direction: column;
-  gap: 0.6rem;
+  gap: 0;


[nitpick] The gap was changed from 0.6rem to 0 which removes all spacing between task nodes. This could negatively impact visual hierarchy and readability. The drop indicator positioning (lines 47-52) assumes specific spacing with absolute positioning at -6px and bottom: -6px, which may not work correctly with zero gap. Consider if this spacing change is intentional and whether the drop indicators need adjustment.

Copilot · 2025-11-25T22:17:23Z

src/components/DragProvider.tsx

+                Dragging
+              </div>
+            }>
+              {props.renderOverlay ? props.renderOverlay(state.draggedId) : null}


Redundant condition in the fallback JSX. The Show component already checks when={props.renderOverlay}, but inside the children, there's another check {props.renderOverlay ? props.renderOverlay(state.draggedId) : null}. This is redundant because the Show component's children only execute when the condition is true. The children should simply be {props.renderOverlay!(state.draggedId)} with the non-null assertion operator.

Suggested change

{props.renderOverlay ? props.renderOverlay(state.draggedId) : null}

{props.renderOverlay!(state.draggedId)}

Signed-off-by: Bart Jaskulski <bjaskulski@protonmail.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 8 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-22T15:48:38Z

src/stores/taskStore.ts

+export const deleteTask = async (id: string) => {
+  const db = await getDb();
+
+  const toRemove: string[] = [id];
+  let found = true;
+
+  while (found) {
+    found = false;
+    const rows = db.exec("SELECT * FROM tasks");
+    if (Array.isArray(rows)) {
+      rows.forEach((row: any) => {
+        if (toRemove.includes(row.parent_id) && !toRemove.includes(row.id)) {
+          toRemove.push(row.id);
+          found = true;
        }
      });
    }
+  }

-    toRemove.forEach(taskId => yTasks.delete(taskId));
+  toRemove.forEach(taskId => {
+    db.exec("DELETE FROM tasks WHERE id = ?", [taskId]);
  });
+
+  await refreshTasks();
 };


The deleteTask function has a performance issue. It loads ALL tasks from the database in each iteration of the while loop using db.exec("SELECT * FROM tasks"). This is inefficient - you should use a recursive SQL query with a CTE (Common Table Expression) to find all descendants in a single query, or at minimum filter by parent_id in the SELECT statement.

Copilot · 2026-01-22T15:48:38Z

src/stores/taskStore.ts

+export const updateTask = async (id: string, fields: Partial<Task>) => {
+  const db = await getDb();
+  const rows = await db.execO("SELECT * FROM tasks WHERE id = ?", [id]);
+
+  if (!rows || !Array.isArray(rows) || rows.length === 0) return;
+
+  const current = rows[0];
+
+  const updates: string[] = [];
+  const values: any[] = [];
+
+  if (fields.parentId !== undefined) {
+    updates.push("parent_id = ?");
+    values.push(fields.parentId);
+  }
+  if (fields.text !== undefined) {
+    updates.push("text = ?");
+    values.push(fields.text);
+  }
+  if (fields.completed !== undefined) {
+    updates.push("completed = ?");
+    values.push(fields.completed ? 1 : 0);
+  }
+  if (fields.dueAt !== undefined) {
+    updates.push("due_at = ?");
+    values.push(fields.dueAt);
+  }
+  if (fields.rank !== undefined) {
+    updates.push("rank = ?");
+    values.push(fields.rank);
+  }
+
+  if (updates.length) {
+    values.push(id);
+    db.exec(`UPDATE tasks SET ${updates.join(", ")} WHERE id = ?`, values);
+    await refreshTasks();
+  }
 };

-export const moveTask = (
+export const moveTask = async (
  taskId: string, 
  newParentId: string | null, 
  prevSiblingRank?: string, 
  nextSiblingRank?: string
 ) => {
-  doc.transact(() => {
-    const task = yTasks.get(taskId);
-    if (!task) return;
-
-    // Calculate new rank between siblings
-    let newRank;
-    if (!prevSiblingRank && !nextSiblingRank) {
-      newRank = LexoRank.middle();
-    } else if (!prevSiblingRank && nextSiblingRank) {
-      newRank = LexoRank.parse(nextSiblingRank).genPrev();
-    } else if (prevSiblingRank && !nextSiblingRank) {
-      newRank = LexoRank.parse(prevSiblingRank).genNext();
-    } else {
-      newRank = LexoRank.parse(prevSiblingRank!).between(LexoRank.parse(nextSiblingRank!));
-    }
-
-    yTasks.set(taskId, { 
-      ...task, 
-      parentId: newParentId, 
-      rank: newRank.toString() 
-    });
-  });
-};
-
-export const deleteTask = (id: string) => {
-  doc.transact(() => {
-    if (!yTasks.has(id)) return;
+  const db = await getDb();
+  const rows = await db.execO("SELECT * FROM tasks WHERE id = ?", [taskId]);
+
+  if (!rows || !Array.isArray(rows) || rows.length === 0) return;
+
+  const task = rows[0];
+
+  let newRank;
+  if (!prevSiblingRank && !nextSiblingRank) {
+    newRank = LexoRank.middle().toString();
+  } else if (!prevSiblingRank && nextSiblingRank) {
+    newRank = LexoRank.parse(nextSiblingRank).genPrev().toString();
+  } else if (prevSiblingRank && !nextSiblingRank) {
+    newRank = LexoRank.parse(prevSiblingRank).genNext().toString();
+  } else {
+    newRank = LexoRank.parse(prevSiblingRank!).between(LexoRank.parse(nextSiblingRank!)).toString();
+  }

-    const snapshot = yTasks.toJSON();
-    const toRemove: string[] = [];
-    const stack = [id];
+  db.exec(
+    "UPDATE tasks SET parent_id = ?, rank = ? WHERE id = ?",
+    [newParentId, newRank, taskId]
+  );

-    while (stack.length) {
-      const currentId = stack.pop()!;
-      toRemove.push(currentId);
+  await refreshTasks();
+};

-      Object.entries(snapshot).forEach(([taskId, task]) => {
-        if (task.parentId === currentId) {
-          stack.push(taskId);
+export const deleteTask = async (id: string) => {
+  const db = await getDb();
+
+  const toRemove: string[] = [id];
+  let found = true;
+
+  while (found) {
+    found = false;
+    const rows = db.exec("SELECT * FROM tasks");
+    if (Array.isArray(rows)) {
+      rows.forEach((row: any) => {
+        if (toRemove.includes(row.parent_id) && !toRemove.includes(row.id)) {
+          toRemove.push(row.id);
+          found = true;
        }
      });
    }
+  }

-    toRemove.forEach(taskId => yTasks.delete(taskId));
+  toRemove.forEach(taskId => {
+    db.exec("DELETE FROM tasks WHERE id = ?", [taskId]);
  });
+
+  await refreshTasks();
 };


Missing error handling. The exec calls at lines 256, 286, and 313 should be wrapped in try-catch blocks or have error handling. Database operations can fail and should be handled gracefully.

Copilot · 2026-01-22T15:48:38Z

src/lib/sync.ts

+  });
+
+  if (!response.ok) {
+    throw new Error(`Upload failed: ${response.status}`);


The error message at line 129 only includes the status code but not the response body or statusText. This makes debugging upload failures more difficult. Consider including more context like ${response.status} ${response.statusText} or the response body if available.

Suggested change

throw new Error(`Upload failed: ${response.status}`);

let responseBody = "";

try {

responseBody = await response.text();

} catch {

// Ignore errors while reading the response body

}

const statusText = response.statusText || "";

const bodySuffix = responseBody ? ` - ${responseBody}` : "";

throw new Error(

`Upload failed: ${response.status}${statusText ? " " + statusText : ""}${bodySuffix}`

);

Copilot · 2026-01-22T15:48:39Z

src/routes/pair.tsx

@@ -0,0 +1,54 @@
+import { Component, createSignal, onMount } from "solid-js";


Unused import Component.

Suggested change

import { Component, createSignal, onMount } from "solid-js";

import { createSignal, onMount } from "solid-js";

Copilot · 2026-01-22T15:48:39Z

src/routes/settings.tsx

@@ -0,0 +1,114 @@
+import { Component, createSignal } from "solid-js";


Unused import Component.

Suggested change

import { Component, createSignal } from "solid-js";

import { createSignal } from "solid-js";

Copilot · 2026-01-22T15:48:39Z

src/stores/taskStore.ts

+  return { state, setState, refreshTasks, initializeTaskStore, tasks };
 });

+const { state, setState, refreshTasks, initializeTaskStore, tasks } = taskStore;


Unused variable setState.

Suggested change

const { state, setState, refreshTasks, initializeTaskStore, tasks } = taskStore;

const { state, refreshTasks, initializeTaskStore, tasks } = taskStore;

Copilot · 2026-01-22T15:48:40Z

src/stores/taskStore.ts

+  const current = rows[0];
+


Unused variable current.

Suggested change

const current = rows[0];

Copilot · 2026-01-22T15:48:40Z

src/stores/taskStore.ts

+  const task = rows[0];
+


Unused variable task.

Suggested change

const task = rows[0];

dragging

3d2505f

bart-jaskulski requested a review from Copilot November 25, 2025 22:10

Copilot started reviewing on behalf of bart-jaskulski November 25, 2025 22:10 View session

Copilot finished reviewing on behalf of bart-jaskulski November 25, 2025 22:14

Copilot AI reviewed Nov 25, 2025

View reviewed changes

bart-jaskulski added 2 commits January 22, 2026 14:02

improve look

b434004

Signed-off-by: Bart Jaskulski <bjaskulski@protonmail.com>

use crsqlite instead of yjs

20d58d9

bart-jaskulski requested a review from Copilot January 22, 2026 15:45

Copilot started reviewing on behalf of bart-jaskulski January 22, 2026 15:45 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

		window.addEventListener("pointermove", handlePointerMove, { passive: false });
		window.addEventListener("pointerup", finishDrag);

	{props.renderOverlay ? props.renderOverlay(state.draggedId) : null}
	{props.renderOverlay!(state.draggedId)}

-    throw new Error(`Upload failed: ${response.status}`);
+    let responseBody = "";
+    try {
+      responseBody = await response.text();
+    } catch {
+      // Ignore errors while reading the response body
+    }
+    const statusText = response.statusText || "";
+    const bodySuffix = responseBody ? ` - ${responseBody}` : "";
+    throw new Error(
+      `Upload failed: ${response.status}${statusText ? " " + statusText : ""}${bodySuffix}`
+    );

		@@ -0,0 +1,54 @@
		import { Component, createSignal, onMount } from "solid-js";

	import { Component, createSignal, onMount } from "solid-js";
	import { createSignal, onMount } from "solid-js";

		@@ -0,0 +1,114 @@
		import { Component, createSignal } from "solid-js";

	import { Component, createSignal } from "solid-js";
	import { createSignal } from "solid-js";

	const { state, setState, refreshTasks, initializeTaskStore, tasks } = taskStore;
	const { state, refreshTasks, initializeTaskStore, tasks } = taskStore;

Conversation

bart-jaskulski commented Nov 25, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant