Optimize sidebar transitions, add smart todo cleanup, and improve wea…#135
Optimize sidebar transitions, add smart todo cleanup, and improve wea…#135codeCraft-Ritik wants to merge 2 commits intoprem-k-r:mainfrom
Conversation
…ther UI responsiveness
📝 WalkthroughWalkthroughCentralizes bookmark sidebar toggling via a new Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeCraft-Ritik
left a comment
There was a problem hiding this comment.
To implement Material Design 3 "Expressive" Motion in your project, the following summary outlines the UI/UX enhancements made to the To-Do list:
- Spring Animation for Completion: Replaced the static checkbox toggle with a "spring" animation using cubic-bezier(0.175, 0.885, 0.32, 1.275). This allows checked items to slightly overshoot their final size, mimicking physical movement.
- Animated Strike-through: Introduced a progressive strike-through effect that grows across the text when a task is completed, providing a clear visual confirmation of the action.
- Tactile Feedback Layers: Added a dedicated :hover state that lifts the item slightly and an :active state that scales the item down to simulate a physical press.
- Button Transitions: Applied smooth scaling transitions to the Pin, Remove, and Edit buttons, ensuring icons react instantly and smoothly to user interaction.
- Interactive Guidance: These micro-interactions are designed to help users understand the interface up to four times faster by providing immediate, expressive feedback for every interaction.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
scripts/weather.js (3)
94-128: Saved API key/location won’t take effect with the current refresh path.
fetchWeather()closes over the initialapiKeyandcurrentUserLocation, so saving a new key or manual location won’t be used until a full re-init. Update the captured values before callingfetchWeather().🐛 Suggested fix
- const savedApiKey = localStorage.getItem("weatherApiKey"); + let savedApiKey = localStorage.getItem("weatherApiKey"); ... - const apiKey = savedApiKey || defaultApiKey; + let apiKey = savedApiKey || defaultApiKey; ... saveAPIButton.addEventListener("click", () => { - const apiKey = userAPIInput.value.trim(); - localStorage.setItem("weatherApiKey", apiKey); + const apiKeyInput = userAPIInput.value.trim(); + localStorage.setItem("weatherApiKey", apiKeyInput); + savedApiKey = apiKeyInput; + apiKey = apiKeyInput || defaultApiKey; userAPIInput.value = ""; fetchWeather(); // Refresh data without full reload }); ... saveLocButton.addEventListener("click", () => { const userLocation = userLocInput.value.trim(); localStorage.setItem("weatherLocation", userLocation); localStorage.setItem("useGPS", false); + currentUserLocation = userLocation; userLocInput.value = ""; fetchWeather(); });
101-118: Avoid re-runninggetWeatherData()on GPS toggle.Calling
getWeatherData()here re-binds all listeners each time, so toggling GPS can stack handlers and duplicate fetches/prompts. Extract a refresh-only path (e.g., re-evaluate location + callfetchWeather) or guard against multiple initializations.
455-503: Min/Max toggle uses a stale snapshot ofisMinMaxEnabled.
updateTemperatureDisplay()reads the initial value, so UI won’t reflect checkbox changes without reload. Read the current checkbox state each render.🐛 Suggested fix
- if (isMinMaxEnabled) { + const minMaxEnabled = minMaxTempCheckbox?.checked ?? isMinMaxEnabled; + if (minMaxEnabled) { ... - if (isMinMaxEnabled) { + if (minMaxEnabled) {
🤖 Fix all issues with AI agents
In `@scripts/weather.js`:
- Around line 591-610: The event handlers at module scope reference
UpdateWeather and minMaxTempCheckbox which are defined only inside
getWeatherData, causing a ReferenceError; either move the fahrenheitCheckbox,
hideWeatherCard and minMaxTempCheckbox addEventListener blocks into
getWeatherData after UpdateWeather and minMaxTempCheckbox are declared, or hoist
the declarations of UpdateWeather (function) and minMaxTempCheckbox
(element/variable) to module scope and update
saveCheckboxState/loadCheckboxState calls to use those hoisted symbols so the
listeners can safely reference UpdateWeather, fahrenheitCheckbox,
hideWeatherCard and minMaxTempCheckbox at runtime.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@style.css`:
- Around line 5401-5412: The .todolistitem.checked::after pseudo-element is
adding an animated strike-through on top of the existing text-decoration:
line-through in the .todolistitem.checked rule, producing a double
strike-through; decide which effect you want and either remove the
text-decoration from the existing .todolistitem.checked rule or remove/disable
the .todolistitem.checked::after block (or guard it with a modifier class like
.animated-strike) so only a single strike-through is rendered (reference
.todolistitem.checked and .todolistitem.checked::after to locate and modify the
rules).
- Around line 5433-5435: The CSS rule for selectors .todopinbtn:hover,
.todoremovebtn:hover, .todoeditbtn:hover is missing its closing brace which
causes a parse error; fix it by adding the missing closing curly brace '}' to
terminate that rule (ensure the rule block wrapping transform: scale(1.2); is
properly closed) so subsequent CSS is parsed correctly.
- Around line 5393-5426: The CSS uses undefined Material3 variables
(--md-sys-color-outline, --md-sys-color-surface-container-high,
--md-sys-color-surface-container-highest) inside the .todolistitem.checked,
.todolistitem.checked::after, .todolistitem:hover and .todolistitem:active
rules; fix by either defining these tokens in :root or replacing them with your
project's existing variables (e.g., --darkColor-blue, --textColorDark-blue) so
the color/background values have valid fallbacks—update all occurrences in the
.todolistitem selectors to use the chosen project variables consistently.
prem-k-r
left a comment
There was a problem hiding this comment.
Hi @codeCraft-Ritik, could you please split this PR into three separate PRs, each with an independent improvement?
This will help speed up review and merging.
🛠️ Summary of Changes
To-Do List: Smart Daily Cleanup
**File:
MaterialYouNewTab/scripts/todo-list.jsChange: Modified the daily reset logic. Instead of permanently deleting unpinned completed tasks every morning, they are now marked as archived: true.
Benefit: Prevents accidental data loss and allows the potential for a "Recently Deleted" view in the future.**
2. Weather: Dynamic Settings Update
MaterialYouNewTab/scripts/weather.js3. Bookmarks: Sidebar & Overlay Synchronization
1. File:
MaterialYouNewTab/scripts/bookmarks.js2. Change: Integrated a unified toggleBookmarkSidebar(open) function that synchronizes the background overlay (bookmarksContainer) with the sidebar slide animation.
3. Benefit: Fixes "jank" where the blur would appear or disappear at a different speed than the sidebar.
Stability: Retained all 500+ lines of original logic, including context menus, sorting, and browser-specific API handling for Firefox and Chromium.
This PR improves UI responsiveness and data safety across bookmarks, the to‑do list, weather, and adds motion polish.
Bookmarks
To‑Do List
Weather
Style / Motion
Scope & Stability
+97/-257), todo (+12/-8), weather (~+17/-25), style (+74/-0). Estimated review effort: Medium–High.