feat: Add Dark Mode, Pixel Art theme, Drag-and-Drop, and Local Storage persistence - #5
Open
PrimeDeV-824 wants to merge 3 commits into
Open
feat: Add Dark Mode, Pixel Art theme, Drag-and-Drop, and Local Storage persistence#5PrimeDeV-824 wants to merge 3 commits into
PrimeDeV-824 wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe todo app is restyled as “QUEST LOG,” with light and synthwave themes, updated labels and controls, draggable todo reordering, revised empty and footer states, and responsive themed components. ChangesQuest Log refresh
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TodoListDOM
participant todos
participant saveTodos
User->>TodoListDOM: Drag a todo item
TodoListDOM->>TodoListDOM: Calculate insertion position
TodoListDOM->>todos: Synchronize reordered items
todos->>saveTodos: Persist order when filter is all
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@08-todo-app/index.html`:
- Around line 29-34: Add aria-pressed="false" to the palette-toggle and
theme-toggle buttons, then update each button’s aria-pressed value whenever its
corresponding body class changes so assistive technology reflects the current
mode state.
- Around line 42-44: Add the accessible name “Add quest” to the icon-only
`#add-task` button in 08-todo-app/index.html:42-44. In
08-todo-app/script.js:115-118, update the deleteBtn creation in the
todo-rendering logic to set its aria-label to “Delete ” followed by todo.text.
In `@08-todo-app/script.js`:
- Around line 89-94: Update the todo item setup around todoItem.draggable and
the reorder handling near the existing persistence logic so drag operations are
enabled only in the “all” view, or ensure reordered filtered items are merged
back into todos before saving. Preserve drag behavior and persistence
consistency so the DOM cannot show a reorder that is later discarded.
- Around line 24-37: Update the themeToggle and paletteToggle handlers to
persist their respective states in localStorage, then restore both preferences
during script bootstrap. Restoration must synchronize body classes, the theme
icon’s moon/sun state, and each toggle’s aria-pressed value, while preserving
the existing toggle behavior.
In `@08-todo-app/style.css`:
- Around line 61-69: The body.theme-synthwave.light-mode palette needs higher
normal-text contrast. Add separate foreground variables for tile and accent
surfaces, update the affected todo, filter, and header text rules to use them,
and adjust the palette values so every foreground/background pairing reaches at
least 4.5:1 while preserving the existing theme structure.
- Around line 320-340: Update the checkbox focus styling around `.todo-checkbox`
and `.checkmark` so keyboard focus on the transparent input produces a visible
indicator on the associated checkmark, using an appropriate focus-visible
selector and preserving the existing checkbox appearance otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ce943e9-32c1-490e-bc3f-25d71e9ca4e8
⛔ Files ignored due to path filters (1)
08-todo-app/todo.pngis excluded by!**/*.png
📒 Files selected for processing (3)
08-todo-app/index.html08-todo-app/script.js08-todo-app/style.css
Comment on lines
+29
to
+34
| <button id="palette-toggle" class="pixel-btn" title="Change Color Palette"> | ||
| <i class="fas fa-palette"></i> | ||
| </button> | ||
| <button id="theme-toggle" class="pixel-btn" title="Toggle Light/Dark"> | ||
| <i class="fas fa-moon"></i> | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Expose the toggle state to assistive technology.
These controls visually change modes but never communicate whether each mode is active. Add aria-pressed="false" initially and update it alongside the body classes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@08-todo-app/index.html` around lines 29 - 34, Add aria-pressed="false" to the
palette-toggle and theme-toggle buttons, then update each button’s aria-pressed
value whenever its corresponding body class changes so assistive technology
reflects the current mode state.
Comment on lines
+42
to
44
| <button id="add-task" class="pixel-btn"> | ||
| <i class="fas fa-plus"></i> | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Give icon-only action buttons accessible names.
Neither button exposes its purpose when the icon is unavailable to assistive technology.
08-todo-app/index.html#L42-L44: addaria-label="Add quest".08-todo-app/script.js#L115-L118: setdeleteBtn.setAttribute("aria-label", \Delete ${todo.text}`)`.
📍 Affects 2 files
08-todo-app/index.html#L42-L44(this comment)08-todo-app/script.js#L115-L118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@08-todo-app/index.html` around lines 42 - 44, Add the accessible name “Add
quest” to the icon-only `#add-task` button in 08-todo-app/index.html:42-44. In
08-todo-app/script.js:115-118, update the deleteBtn creation in the
todo-rendering logic to set its aria-label to “Delete ” followed by todo.text.
Comment on lines
+24
to
+37
| themeToggle.addEventListener("click", () => { | ||
| document.body.classList.toggle("light-mode"); | ||
| const icon = themeToggle.querySelector("i"); | ||
| if (document.body.classList.contains("light-mode")) { | ||
| icon.classList.replace("fa-moon", "fa-sun"); | ||
| } else { | ||
| icon.classList.replace("fa-sun", "fa-moon"); | ||
| } | ||
| }); | ||
|
|
||
| // Synthwave Palette Toggle | ||
| paletteToggle.addEventListener("click", () => { | ||
| document.body.classList.toggle("theme-synthwave"); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Persist and restore the selected theme.
Both toggles only mutate body classes, so palette and dark-mode preferences reset on every reload. Store both states in localStorage and restore them, including the moon/sun icon and aria-pressed values, during bootstrap.
As required by the PR objective: “Local storage persistence for todos, completion statuses, and theme preferences.”
Also applies to: 239-242
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@08-todo-app/script.js` around lines 24 - 37, Update the themeToggle and
paletteToggle handlers to persist their respective states in localStorage, then
restore both preferences during script bootstrap. Restoration must synchronize
body classes, the theme icon’s moon/sun state, and each toggle’s aria-pressed
value, while preserving the existing toggle behavior.
Comment on lines
+89
to
+94
| // Setup for Drag and Drop Tile | ||
| todoItem.draggable = true; | ||
| todoItem.dataset.id = todo.id; | ||
|
|
||
| todoItem.addEventListener("dragstart", handleDragStart); | ||
| todoItem.addEventListener("dragend", handleDragEnd); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not enable drag operations whose result is discarded.
Todos remain draggable in active and completed views, but Line 180 refuses to persist those reorders. The DOM appears reordered until the next render and then snaps back. Either disable dragging outside "all" or merge the filtered order back into todos.
Minimal fix
- todoItem.draggable = true;
+ todoItem.draggable = currentFilter === "all";Also applies to: 179-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@08-todo-app/script.js` around lines 89 - 94, Update the todo item setup
around todoItem.draggable and the reorder handling near the existing persistence
logic so drag operations are enabled only in the “all” view, or ensure reordered
filtered items are merged back into todos before saving. Preserve drag behavior
and persistence consistency so the DOM cannot show a reorder that is later
discarded.
Comment on lines
+61
to
+69
| body.theme-synthwave.light-mode { | ||
| --bg-color: #4cc9f0; | ||
| --app-bg: #4895ef; | ||
| --text-main: #000000; | ||
| --text-muted: #3a0ca3; | ||
| --border-color: #3f37c9; | ||
| --accent: #f72585; | ||
| --tile-bg: #4361ee; | ||
| --tile-hover: #3f37c9; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Increase contrast in the synthwave light palette.
Several foreground/background pairs fail normal-text contrast: black on --tile-bg is about 4.15:1, --text-muted about 2.35:1, and pink --accent about 1.34:1. This affects todo text, filters, and header text. Introduce separate tile/accent foreground variables and verify each pairing reaches 4.5:1.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@08-todo-app/style.css` around lines 61 - 69, The
body.theme-synthwave.light-mode palette needs higher normal-text contrast. Add
separate foreground variables for tile and accent surfaces, update the affected
todo, filter, and header text rules to use them, and adjust the palette values
so every foreground/background pairing reaches at least 4.5:1 while preserving
the existing theme structure.
Comment on lines
320
to
+340
| .todo-checkbox { | ||
| opacity: 0; | ||
| position: absolute; | ||
| width: 100%; | ||
| height: 100%; | ||
| cursor: pointer; | ||
| z-index: 2; | ||
| } | ||
|
|
||
| .checkmark { | ||
| display: inline-block; | ||
| width: 20px; | ||
| height: 20px; | ||
| border: 2px solid #dee2e6; | ||
| border-radius: 4px; | ||
| position: relative; | ||
| cursor: pointer; | ||
| transition: all 0.2s; | ||
| position: absolute; | ||
| top: 0; | ||
| left: 0; | ||
| width: 28px; | ||
| height: 28px; | ||
| background-color: var(--bg-color); | ||
| border: 2px solid var(--border-color); | ||
| box-shadow: inset 2px 2px 0px rgba(0, 0, 0, 0.5); | ||
| transition: | ||
| background-color 0.2s, | ||
| border-color 0.4s ease; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add a visible keyboard-focus indicator for checkboxes.
The focused input is fully transparent, and .checkmark has no corresponding focus style, leaving keyboard users unable to see which task checkbox is selected.
Proposed fix
+.todo-checkbox:focus-visible + .checkmark {
+ outline: 3px solid var(--accent);
+ outline-offset: 3px;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .todo-checkbox { | |
| opacity: 0; | |
| position: absolute; | |
| width: 100%; | |
| height: 100%; | |
| cursor: pointer; | |
| z-index: 2; | |
| } | |
| .checkmark { | |
| display: inline-block; | |
| width: 20px; | |
| height: 20px; | |
| border: 2px solid #dee2e6; | |
| border-radius: 4px; | |
| position: relative; | |
| cursor: pointer; | |
| transition: all 0.2s; | |
| position: absolute; | |
| top: 0; | |
| left: 0; | |
| width: 28px; | |
| height: 28px; | |
| background-color: var(--bg-color); | |
| border: 2px solid var(--border-color); | |
| box-shadow: inset 2px 2px 0px rgba(0, 0, 0, 0.5); | |
| transition: | |
| background-color 0.2s, | |
| border-color 0.4s ease; | |
| .todo-checkbox { | |
| opacity: 0; | |
| position: absolute; | |
| width: 100%; | |
| height: 100%; | |
| cursor: pointer; | |
| z-index: 2; | |
| } | |
| .todo-checkbox:focus-visible + .checkmark { | |
| outline: 3px solid var(--accent); | |
| outline-offset: 3px; | |
| } | |
| .checkmark { | |
| position: absolute; | |
| top: 0; | |
| left: 0; | |
| width: 28px; | |
| height: 28px; | |
| background-color: var(--bg-color); | |
| border: 2px solid var(--border-color); | |
| box-shadow: inset 2px 2px 0px rgba(0, 0, 0, 0.5); | |
| transition: | |
| background-color 0.2s, | |
| border-color 0.4s ease; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@08-todo-app/style.css` around lines 320 - 340, Update the checkbox focus
styling around `.todo-checkbox` and `.checkmark` so keyboard focus on the
transparent input produces a visible indicator on the associated checkmark,
using an appropriate focus-visible selector and preserving the existing checkbox
appearance otherwise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR focuses on upgrading the overall user experience and adding personalization options to the Todo app. The main additions include a dynamic theme engine (featuring Dark Mode and a retro Pixel Art style), intuitive drag-and-drop task reordering, and robust local storage persistence to keep user data intact across sessions.
What's New?
Dynamic Theme Engine (including Dark Mode & Pixel Art):
Built out a flexible styling system supporting multiple themes (Main and Other).
Added a dedicated Dark Mode toggle for better low-light usability.
Designed a custom, retro Pixel Art theme for a unique, nostalgic aesthetic.
Interactive Drag-and-Drop Tiles:
Implemented seamless drag-and-drop functionality, allowing users to intuitively reorder and prioritize tasks.
State Persistence via Local Storage:
Hooked the application state into localStorage. Todos, completion statuses, and theme preferences now persist across page refreshes.
Technical Implementation & Edge Cases Handled
State Syncing: Side effects handle syncing state to local storage smoothly, preventing any blocking UI lags during rapid updates.
UX & Interaction: Finetuned pointer events on the todo tiles to ensure dragging feels fluid and doesn't conflict with normal click/tap interactions (like toggling a todo or hitting delete).
CSS Architecture: Kept the Pixel Art theme lightweight by relying on clean, native CSS configurations rather than heavy external asset dependencies.
How to Test
Persistence: Create a few tasks, switch to Dark Mode or the Pixel Art theme, and refresh your browser. Everything should restore exactly as you left it.
Drag-and-Drop: Drag a tile from the top to the bottom of the list. Reordering should feel responsive and look smooth.
Theme Toggle: Cycle through the configurations (Main, Other, Dark, Pixel Art) to verify that colors, borders, and text scale correctly without UI breakage.
Note to Reviewer:
I wanted to give the app a bit more personality—let me know what you think of the pixel art styling! Also, keep an eye on the drag-and-drop transitions during your review and let me know if the animations feel smooth on your setup.
Summary by CodeRabbit