Add drag-and-drop reordering for highlight types by serpsaipong-nav · Pull Request #126 · UdaraJay/Pile

serpsaipong-nav · 2026-01-17T13:13:30Z

Add @dnd-kit packages for drag-and-drop functionality
Add order field to highlight data structure with migration for existing data
Add reorder IPC handler and context function
Implement sortable highlight items in Settings UI with drag handles
Order persists across app restarts

- Add @dnd-kit packages for drag-and-drop functionality - Add order field to highlight data structure with migration for existing data - Add reorder IPC handler and context function - Implement sortable highlight items in Settings UI with drag handles - Order persists across app restarts

UdaraJay · 2026-01-18T23:14:12Z

thank you for the contribution!

Can you exclude the lock files from the commit please?

Copilot

Pull request overview

This pull request adds drag-and-drop reordering functionality for highlight types in the Settings UI. It introduces the @dnd-kit library for managing drag-and-drop interactions, adds an order field to the highlight data structure with automatic migration for existing data, and implements a complete UI for creating, editing, deleting, and reordering highlights.

Changes:

Added @dnd-kit packages for drag-and-drop functionality
Implemented sortable highlight items with drag handles in Settings UI
Added order field to highlights with migration logic for existing data
Created IPC handlers and context functions for CRUD and reordering operations

Reviewed changes

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

Show a summary per file

File	Description
package.json	Added @dnd-kit dependencies and removed devEngines configuration
release/app/package-lock.json	Version bump to 1.0.0 and simplified structure
src/renderer/pages/Pile/Settings/index.jsx	Implemented SortableHighlightItem component and highlight management UI with drag-and-drop
src/renderer/pages/Pile/Settings/Settings.module.scss	Added comprehensive styling for highlight list, drag handles, and editor UI
src/renderer/context/HighlightsContext.js	Updated context with createHighlight, updateHighlight, and reorderHighlights functions
src/main/handlers/highlights.ts	Added IPC handlers for create, update, and reorder operations
src/main/utils/pileHighlights.js	Implemented CRUD operations, order field migration, and reordering logic

Files not reviewed (1)

release/app/package-lock.json: Language not supported

Comments suppressed due to low confidence (1)

src/main/utils/pileHighlights.js:172

The save function reassigns this.highlights to a sorted version on line 165. This mutation happens inside the save function, which might not be expected by callers. Additionally, the check on line 168 (if (!entries) return;) will never be true because Map.prototype.entries() always returns an iterator, never null or undefined. This check is ineffective and should be removed.

  save() {
    if (!this.pilePath) return;
    if (!fs.existsSync(this.pilePath)) {
      fs.mkdirSync(this.pilePath, { recursive: true });
    }

    const highlightsFilePath = path.join(this.pilePath, this.fileName);
    const sortedHighlights = this.sortMap(this.highlights);

    this.highlights = sortedHighlights;
    const entries = this.highlights.entries();

    if (!entries) return;

    let strMap = JSON.stringify(Array.from(entries));

    fs.writeFileSync(highlightsFilePath, strMap);

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

Copilot · 2026-01-19T03:45:45Z

+          onClick={() => onEdit(name, data)}
+        >
+          <EditIcon className={styles.actionIcon} />
+        </button>
+        <button
+          className={styles.highlightActionBtn}
+          onClick={() => onDelete(name)}


The edit and delete buttons lack accessible labels. Screen reader users won't know what these buttons do. Add aria-label attributes such as aria-label="Edit highlight" and aria-label="Delete highlight" to improve accessibility.

Suggested change

onClick={() => onEdit(name, data)}

>

<EditIcon className={styles.actionIcon} />

</button>

<button

className={styles.highlightActionBtn}

onClick={() => onDelete(name)}

onClick={() => onEdit(name, data)}

aria-label="Edit highlight"

>

<EditIcon className={styles.actionIcon} />

</button>

<button

className={styles.highlightActionBtn}

onClick={() => onDelete(name)}

aria-label="Delete highlight"

Copilot · 2026-01-19T03:45:46Z

+      let index = 0;
+      loadedHighlights.forEach((value, key) => {
+        if (value.order === undefined) {
+          value.order = index;
+          needsSave = true;
+        }
+        index++;


The migration logic has a bug. The index counter increments for all items (line 44), but order is only assigned to items without an order field (line 41). This means if some highlights already have order values and others don't, the new order values won't be sequential. For example, if highlight A has order=5, highlight B is missing order (gets index=0), and highlight C is missing order (gets index=1), they will have orders 5, 0, 1 which isn't sequential. Consider assigning order values based only on items that need migration, or reassigning all order values during migration.

Suggested change

let index = 0;

loadedHighlights.forEach((value, key) => {

if (value.order === undefined) {

value.order = index;

needsSave = true;

}

index++;

// First, find the maximum existing order value

let maxOrder = -1;

loadedHighlights.forEach((value) => {

if (typeof value.order === 'number') {

if (value.order > maxOrder) {

maxOrder = value.order;

}

}

});

// Then, assign sequential order values only to items that lack one

let nextOrder = maxOrder + 1;

loadedHighlights.forEach((value) => {

if (value.order === undefined) {

value.order = nextOrder;

nextOrder++;

needsSave = true;

}

Copilot · 2026-01-19T03:45:46Z

+  const handleSaveHighlight = () => {
+    if (!highlightName.trim()) return;
+
+    if (isAddingNew) {
+      createHighlight({ name: highlightName.trim(), color: highlightColor });
+    } else if (editingHighlight) {
+      updateHighlight({
+        oldName: editingHighlight,
+        name: highlightName.trim(),
+        color: highlightColor,
+      });
+    }
+
+    handleCancelEdit();
+  };


The handleSaveHighlight function doesn't check if a highlight with the new name already exists before creating or updating. When adding a new highlight with a name that already exists, the backend's create function will silently return without adding (line 80-82 in pileHighlights.js), but the UI won't provide any feedback to the user. Similarly, when updating a highlight to a name that already exists, the update function will silently fail (line 109-110 in pileHighlights.js). Consider adding validation to provide user feedback when duplicate names are attempted.

Copilot · 2026-01-19T03:45:48Z

+  reorder(orderedNames) {
+    // Update order field based on array position
+    orderedNames.forEach((name, index) => {
+      if (this.highlights.has(name)) {
+        const highlight = this.highlights.get(name);
+        highlight.order = index;
+        this.highlights.set(name, highlight);
+      }
+    });

+    this.save();
+
+    return this.highlights;
+  }


The reorder function modifies the order field of existing highlights but doesn't handle highlights that might not be in the orderedNames array. If a highlight exists in this.highlights but is not included in orderedNames (for example, if it was added concurrently or if the frontend sends an incomplete list), its order field will remain unchanged, potentially causing ordering conflicts. Consider either validating that all highlights are included in orderedNames, or resetting the order of any highlights not in the array to a default value.

Copilot · 2026-01-19T03:45:48Z

+    if (active.id !== over?.id) {
+      const highlightEntries = Array.from(highlights.entries());
+      const oldIndex = highlightEntries.findIndex(([name]) => name === active.id);
+      const newIndex = highlightEntries.findIndex(([name]) => name === over.id);
+
+      const reorderedEntries = arrayMove(highlightEntries, oldIndex, newIndex);
+      const orderedNames = reorderedEntries.map(([name]) => name);
+
+      reorderHighlights(orderedNames);
+    }


The handleDragEnd function doesn't check if over is null before accessing over.id on line 231. If a drag operation ends outside a valid drop zone, over will be null, and this will cause a runtime error. The optional chaining on line 231 prevents the error on that line, but the function will continue executing and try to find newIndex with over.id on line 234, which will fail.

Suggested change

if (active.id !== over?.id) {

const highlightEntries = Array.from(highlights.entries());

const oldIndex = highlightEntries.findIndex(([name]) => name === active.id);

const newIndex = highlightEntries.findIndex(([name]) => name === over.id);

const reorderedEntries = arrayMove(highlightEntries, oldIndex, newIndex);

const orderedNames = reorderedEntries.map(([name]) => name);

reorderHighlights(orderedNames);

}

if (!over || active.id === over.id) {

return;

}

const highlightEntries = Array.from(highlights.entries());

const oldIndex = highlightEntries.findIndex(([name]) => name === active.id);

const newIndex = highlightEntries.findIndex(([name]) => name === over.id);

const reorderedEntries = arrayMove(highlightEntries, oldIndex, newIndex);

const orderedNames = reorderedEntries.map(([name]) => name);

reorderHighlights(orderedNames);

@UdaraJay

As @UdaraJay requested, I've removed lock file for you. Signed-off-by: Serpsaipong Navanuraksa <38547838+funamorikeitou@users.noreply.github.com>

serpsaipong-nav · 2026-01-19T08:21:13Z

@UdaraJay I've already remove lock file as you asked.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Udara Jay <UdaraJay@users.noreply.github.com>

UdaraJay requested a review from Copilot January 19, 2026 03:40

Copilot started reviewing on behalf of UdaraJay January 19, 2026 03:40 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

Delete package-lock.json

b422c98

As @UdaraJay requested, I've removed lock file for you. Signed-off-by: Serpsaipong Navanuraksa <38547838+funamorikeitou@users.noreply.github.com>

Update src/renderer/pages/Pile/Settings/index.jsx

d9c28bb

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Udara Jay <UdaraJay@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add drag-and-drop reordering for highlight types#126

Add drag-and-drop reordering for highlight types#126
serpsaipong-nav wants to merge 3 commits into
UdaraJay:mainfrom
serpsaipong-nav:feature/highlight-drag-reorder

serpsaipong-nav commented Jan 17, 2026

Uh oh!

UdaraJay commented Jan 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

serpsaipong-nav commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-      let index = 0;
-      loadedHighlights.forEach((value, key) => {
-        if (value.order === undefined) {
-          value.order = index;
-          needsSave = true;
-        }
-        index++;
+      // First, find the maximum existing order value
+      let maxOrder = -1;
+      loadedHighlights.forEach((value) => {
+        if (typeof value.order === 'number') {
+          if (value.order > maxOrder) {
+            maxOrder = value.order;
+          }
+        }
+      });
+      // Then, assign sequential order values only to items that lack one
+      let nextOrder = maxOrder + 1;
+      loadedHighlights.forEach((value) => {
+        if (value.order === undefined) {
+          value.order = nextOrder;
+          nextOrder++;
+          needsSave = true;
+        }

Conversation

serpsaipong-nav commented Jan 17, 2026

Uh oh!

UdaraJay commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

serpsaipong-nav commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

UdaraJay commented Jan 18, 2026 •

edited

Loading