feat: Todo plugin UI improvements#103
Conversation
- Center list filter tabs on wrap to match main nav behavior - Move "Manage Lists" from todo header into Settings page "To-do" card - Replace quick-add form with Add button that opens full edit modal - Add sort dropdown (Manual, Priority, Due Date, Newest) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- To-do settings card in Settings shows/hides when plugin is toggled - Drag-to-reorder todo items in Manual sort mode (drag handles visible) - Added sort_order field to todo storage for persistent manual ordering Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces manual drag-and-drop reordering and customizable sorting options for the to-do list, alongside a unified modal for creating and editing tasks. The reviewer feedback suggests optimizing database performance by updating sort orders concurrently with Promise.all, ensuring stable sorting with a fallback key, providing user validation feedback for empty titles, and resetting the dragged item state on drag completion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for (let i = 0; i < ids.length; i++) { | ||
| await Storage.updateTodo(ids[i], { sort_order: i }); | ||
| } |
There was a problem hiding this comment.
Awaiting Storage.updateTodo sequentially inside a for loop creates a separate IndexedDB transaction for each item and waits for it to complete before starting the next one. This is a major performance bottleneck, especially for larger lists. Using Promise.all allows the browser to process these transactions concurrently, significantly improving drag-and-drop responsiveness.
| for (let i = 0; i < ids.length; i++) { | |
| await Storage.updateTodo(ids[i], { sort_order: i }); | |
| } | |
| await Promise.all(ids.map((id, i) => Storage.updateTodo(id, { sort_order: i }))); |
| const bPri = priOrder[b.priority] ?? 3; | ||
| if (aPri !== bPri) return aPri - bPri; | ||
| return new Date(a.created_at) - new Date(b.created_at); | ||
| return (a.sort_order ?? 999999) - (b.sort_order ?? 999999); |
There was a problem hiding this comment.
When sorting todos, if multiple items have the same sort_order (or if it is missing/undefined), the sort order can be unstable. Adding a fallback sort key like created_at ensures consistent and stable sorting.
const orderA = a.sort_order ?? 999999;
const orderB = b.sort_order ?? 999999;
if (orderA !== orderB) return orderA - orderB;
return new Date(a.created_at || 0) - new Date(b.created_at || 0);| list_id: document.getElementById('todo-edit-list').value || null | ||
| }); | ||
| }; | ||
| if (!data.title) return; |
There was a problem hiding this comment.
If the user attempts to save a todo with an empty title, the function currently returns silently. This can be confusing as the modal remains open without any visual feedback, making the app appear unresponsive. Adding a simple alert or validation message improves the user experience.
| if (!data.title) return; | |
| if (!data.title) { | |
| alert('Please enter a title.'); | |
| return; | |
| } |
| function handleTodoDragEnd(e) { | ||
| const item = e.target.closest('.todo-item'); | ||
| if (item) item.classList.remove('dragging'); | ||
| document.querySelectorAll('#todo-list-container .todo-item').forEach(el => el.classList.remove('drag-over')); | ||
| } |
There was a problem hiding this comment.
It is good practice to reset the global draggedTodoId variable to null when the drag operation ends to prevent holding stale references and avoid potential side effects in future drag-and-drop interactions.
| function handleTodoDragEnd(e) { | |
| const item = e.target.closest('.todo-item'); | |
| if (item) item.classList.remove('dragging'); | |
| document.querySelectorAll('#todo-list-container .todo-item').forEach(el => el.classList.remove('drag-over')); | |
| } | |
| function handleTodoDragEnd(e) { | |
| const item = e.target.closest('.todo-item'); | |
| if (item) item.classList.remove('dragging'); | |
| document.querySelectorAll('#todo-list-container .todo-item').forEach(el => el.classList.remove('drag-over')); | |
| draggedTodoId = null; | |
| } |
Summary
Test plan
🤖 Generated with Claude Code