feat: Add dark mode toggle with local storage persistence #4
Conversation
|
@sandaru-sdm is attempting to deploy a commit to the Eshita's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
""" WalkthroughThe changes implement a comprehensive redesign of the application's UI and JavaScript logic. CSS files are refactored for theme support, responsiveness, and animation. HTML files are modernized for semantic structure, accessibility, and new UI controls like a theme toggle. JavaScript is modularized, event handling is centralized, and preview synchronization is improved. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant JS (script.js)
participant Storage
User->>Browser: Loads page (index.html or invoice.html)
Browser->>JS (script.js): Executes on DOMContentLoaded
JS (script.js)->>Storage: Reads saved theme from localStorage
JS (script.js)->>Browser: Applies theme class, updates icons
JS (script.js)->>Browser: Sets up event listeners (theme toggle, form, buttons)
User->>Browser: Interacts with form (inputs, add/delete rows)
Browser->>JS (script.js): Triggers input/change events
JS (script.js)->>Browser: Syncs form values to preview
User->>Browser: Clicks theme toggle
Browser->>JS (script.js): Toggles theme, updates localStorage
JS (script.js)->>Browser: Updates theme class and icons
User->>Browser: Clicks Download PDF
Browser->>JS (script.js): Triggers PDF generation
JS (script.js)->>Browser: Uses html2pdf to download preview
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
invoice.html (1)
309-317: Consider adding loading state for PDF generationWhile the download button placement is good, consider adding a loading indicator since PDF generation can take time.
You might want to add a loading spinner or disable the button during PDF generation to provide better user feedback.
style.css (1)
374-381: Consider theme-aware invoice previewThe invoice preview has hardcoded white background and dark text colors. While this ensures printability, users might expect the preview to follow the theme.
Consider adding a print-specific class or media query to ensure white background only for printing:
.invoice-box { - background-color: white; - color: #111827; + background-color: var(--preview-bg); + color: var(--text-color); padding: 2rem; border-radius: 8px; font-family: Arial, sans-serif; font-size: 0.9rem; line-height: 1.4; } +@media print { + .invoice-box { + background-color: white; + color: #111827; + } +}script.js (2)
11-11: Consider fallback for optional chainingThe optional chaining operator
?.might not be supported in older browsers.For broader browser compatibility:
- const savedTheme = window.localStorage?.getItem("theme") || "light"; + const savedTheme = (window.localStorage && window.localStorage.getItem("theme")) || "light";
41-49: Add error handling for localStorageConsider wrapping localStorage operations in try-catch blocks to handle potential exceptions (e.g., when storage is full or disabled).
if (document.body.classList.contains("dark-theme")) { - if (window.localStorage) { - window.localStorage.setItem("theme", "dark"); - } + try { + window.localStorage?.setItem("theme", "dark"); + } catch (e) { + console.warn("Failed to save theme preference:", e); + } moonIcon.style.display = "none"; sunIcon.style.display = "block"; } else { - if (window.localStorage) { - window.localStorage.setItem("theme", "light"); - } + try { + window.localStorage?.setItem("theme", "light"); + } catch (e) { + console.warn("Failed to save theme preference:", e); + } moonIcon.style.display = "block"; sunIcon.style.display = "none"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
home.css(1 hunks)index.html(1 hunks)invoice.html(1 hunks)script.js(4 hunks)style.css(1 hunks)
🔇 Additional comments (15)
index.html (4)
17-19: Good semantic structure for logo stylingWrapping the logo in a div provides better control for styling and animations.
21-24: Excellent semantic navigation structureThe use of a semantic
<nav>element improves accessibility and removes unnecessary inline styles.
25-40: Well-implemented theme toggle buttonThe SVG icons are properly embedded with appropriate initial visibility states. The implementation aligns perfectly with the PR objectives for dark mode toggle functionality.
56-61: Good script organizationMoving the navigation function inline and adding the external script reference improves code organization and enables the theme toggle functionality.
invoice.html (3)
26-41: Consistent theme toggle implementationThe theme toggle button matches the implementation in index.html, ensuring a consistent user experience across pages.
54-94: Excellent form structure with CSS GridThe transition from table-based to grid-based layout significantly improves responsiveness and maintainability. The semantic grouping of form fields enhances accessibility.
182-197: Smart use of HTML template elementUsing a
<template>element for the row template is a best practice that prevents the template from being rendered and provides clean cloning for new rows.home.css (3)
1-31: Well-structured theme variablesExcellent implementation of CSS custom properties for theming. The variable naming is clear and the color palette provides good visual hierarchy for both light and dark themes.
112-129: Creative theme toggle animationThe scale and rotation effects on hover provide nice visual feedback. The background color on hover enhances the interactive feel.
162-250: Smooth entrance animationsThe staggered animations create an elegant page load experience. The use of
bothfill mode ensures elements retain their final state.style.css (2)
1-55: Consistent theme implementationThe theme variables perfectly align with home.css, ensuring a cohesive experience across the application. The dark theme colors are well-chosen for readability.
569-678: Comprehensive responsive designThe media queries provide excellent coverage for various device sizes. The layout adjustments ensure usability across all viewports.
script.js (3)
9-28: Robust theme persistence implementationGood implementation with localStorage and proper null checks. The theme application on page load ensures persistence across sessions as per PR objectives.
190-202: Excellent use of template cloningUsing the template element and cloning its content is the correct approach for adding new rows. This ensures clean, event-free elements.
282-285: Good defensive programmingChecking for both the DOM element and the html2pdf library presence prevents runtime errors. The user-friendly alert message is helpful.
| background: linear-gradient(135deg, var(--text-color), var(--highlight-color)); | ||
| -webkit-background-clip: text; | ||
| -webkit-text-fill-color: transparent; | ||
| background-clip: text; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify text gradient browser compatibility
The gradient text effect using -webkit-background-clip: text might not work in all browsers.
🌐 Web query:
What is the current browser support for CSS gradient text using -webkit-background-clip: text property?
💡 Result:
CSS gradient text using -webkit-background-clip: text is fully supported in all major Chromium-based browsers (Chrome, Edge, Opera, Samsung Internet) and in Safari (macOS and iOS), but remains unsupported in Firefox as of July 2025.[10]
Details:
-
Supported browsers:
- Chrome (desktop and Android): Full support for many years. Requires both
-webkit-background-clip: textand-webkit-text-fill-color: transparentfor the gradient to show through text[10]. - Edge (Chromium-based): Same support as Chrome.
- Safari (macOS and iOS): Supports the property with the
-webkit-prefix[10]. - Opera (desktop and Android): Same as Chrome, supports the required prefixes[10].
- Samsung Internet: Built on Chromium, thus also supports these properties[10].
- Chrome (desktop and Android): Full support for many years. Requires both
-
Not supported:
- Firefox: As of July 2025, Firefox does not support
background-clip: text(neither standard nor prefixed), making gradient text via this method not possible in Firefox[10]. There is ongoing interest and discussion, but there is no production support[10]. - Internet Explorer: Not supported and will not be supported as the browser is deprecated.
- Firefox: As of July 2025, Firefox does not support
Syntax for gradient text (for best browser coverage):
.gradient-text {
background: linear-gradient(to right, hotpink, cyan);
-webkit-background-clip: text;
-webkit-text-fill-color: transparent;
}- Standard
background-clip: text(without prefix) has experimental or no support; always use the prefixed version for cross-browser coverage[10].
Fallbacks and alternatives:
- For Firefox and legacy browsers, you may provide solid color text as a fallback or use SVG/text-image workarounds.
- No purely CSS-based solution for gradient text works in Firefox as of the current date[10].
Key points:
- Safari, Chrome, Edge, Opera: Use the
-webkit-prefix. - Firefox: No support as of July 2025.
- Standardization: Although
background-clip: textis part of a spec draft, only WebKit/Blink browsers implement it for text; no confirmed plans for Firefox as of now.
References:
- The latest guidance from front-end experts and code snippets as of 2025[10].
- Caniuse and browser documentation indicate no Firefox support for this feature as of mid-2025[10].
Citations:
- 1: https://www.browserstack.com/guide/browser-compatibility-with-css-gradients
- 2: https://connect.mozilla.org/t5/ideas/support-text-fragments/idi-p/21797
- 3: https://webdirections.org/blog/css3-linear-gradients/
- 4: https://caniuse.com/css-gradients
- 5: https://caniuse.com/text-stroke
- 6: https://tryhoverify.com/blog/css-browser-compatibility-fixes-and-hacks/
- 7: https://elementor.com/blog/css-gradients/
- 8: https://www.sitepoint.com/community/t/text-shadow-browser-compatibility/64660
- 9: https://learn.shayhowe.com/html-css/setting-backgrounds-and-gradients/
- 10: https://nerdy.dev/6-css-snippets-every-front-end-developer-should-know-in-2025
Add fallback for browsers without gradient-text support
Firefox and Internet Explorer don’t support -webkit-background-clip: text, so you should provide a solid‐color fallback or wrap the gradient styles in a feature query. For example, in home.css around lines 194–197:
• Prepend a fallback color
• Wrap the gradient block in an @supports rule or include both fallback and gradient styles
• Remove the unprefixed background-clip: text; (no production support)
Proposed diff:
.gradient-text {
+ /* Fallback for non-WebKit browsers */
+ color: var(--text-color);
+ /* Gradient text for WebKit/Blink/Safari */
+ background: linear-gradient(135deg, var(--text-color), var(--highlight-color));
+ -webkit-background-clip: text;
+ -webkit-text-fill-color: transparent;
- background-clip: text;
}Or using a feature query:
.gradient-text {
color: var(--text-color);
}
@supports (-webkit-background-clip: text) {
.gradient-text {
background: linear-gradient(135deg, var(--text-color), var(--highlight-color));
-webkit-background-clip: text;
-webkit-text-fill-color: transparent;
}
}🤖 Prompt for AI Agents
In home.css around lines 194 to 197, add a solid color fallback by setting the
color property before the gradient styles. Wrap the gradient background,
-webkit-background-clip, and -webkit-text-fill-color properties inside an
@supports rule checking for -webkit-background-clip: text support. Remove the
unprefixed background-clip: text property as it lacks production support. This
ensures browsers without gradient-text support display a solid color fallback.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
script.js (3)
9-29: Consider caching DOM elements and extracting icon toggle logic.The function is well-implemented with good error handling and fallbacks. The localStorage compatibility check and early return pattern are excellent practices.
Consider this refactor for better performance and maintainability:
function applySavedTheme() { const savedTheme = (window.localStorage && window.localStorage.getItem("theme")) || "light"; const themeToggleBtn = document.getElementById("theme-toggle"); const moonIcon = document.querySelector(".moon-icon"); const sunIcon = document.querySelector(".sun-icon"); if (!themeToggleBtn || !moonIcon || !sunIcon) return; - if (savedTheme === "dark") { - document.body.classList.add("dark-theme"); - moonIcon.style.display = "none"; - sunIcon.style.display = "block"; - } else { - document.body.classList.remove("dark-theme"); - moonIcon.style.display = "block"; - sunIcon.style.display = "none"; - } + const isDark = savedTheme === "dark"; + document.body.classList.toggle("dark-theme", isDark); + moonIcon.style.display = isDark ? "none" : "block"; + sunIcon.style.display = isDark ? "block" : "none"; }
31-64: Extract shared DOM queries and icon toggle logic to reduce duplication.The theme toggle implementation is robust with excellent error handling and localStorage fallbacks. However, there's code duplication with
applySavedTheme()- both functions query the same DOM elements and perform similar icon toggling.Consider extracting shared logic:
+function getThemeElements() { + return { + themeToggleBtn: document.getElementById("theme-toggle"), + moonIcon: document.querySelector(".moon-icon"), + sunIcon: document.querySelector(".sun-icon") + }; +} + +function toggleThemeIcons(moonIcon, sunIcon, isDark) { + moonIcon.style.display = isDark ? "none" : "block"; + sunIcon.style.display = isDark ? "block" : "none"; +} function initializeEventListeners() { - const themeToggleBtn = document.getElementById("theme-toggle"); - const moonIcon = document.querySelector(".moon-icon"); - const sunIcon = document.querySelector(".sun-icon"); + const { themeToggleBtn, moonIcon, sunIcon } = getThemeElements(); if (themeToggleBtn && moonIcon && sunIcon) { themeToggleBtn.addEventListener("click", () => { document.body.classList.toggle("dark-theme"); const isDark = document.body.classList.contains("dark-theme"); try { window.localStorage?.setItem("theme", isDark ? "dark" : "light"); } catch (e) { console.warn("Failed to save theme preference:", e); } - if (document.body.classList.contains("dark-theme")) { - try { - window.localStorage?.setItem("theme", "dark"); - } catch (e) { - console.warn("Failed to save theme preference:", e); - } - moonIcon.style.display = "none"; - sunIcon.style.display = "block"; - } else { - try { - window.localStorage?.setItem("theme", "light"); - } catch (e) { - console.warn("Failed to save theme preference:", e); - } - moonIcon.style.display = "block"; - sunIcon.style.display = "none"; - } + toggleThemeIcons(moonIcon, sunIcon, isDark); }); }
125-162: Consider more robust placeholder generation.The form synchronization approach using a mapping array is excellent for maintainability and clarity. The implementation is clean and well-structured.
Consider making the placeholder generation more robust:
function syncInput(inputId, previewId) { const input = document.getElementById(inputId); const preview = document.getElementById(previewId); if (input && preview) { input.addEventListener("input", () => { - const placeholder = `[${previewId.replace("preview", "").replace("bottom", "")}]`; + const fieldName = previewId + .replace(/^(preview|bottom)/, "") + .replace(/([A-Z])/g, " $1") + .toLowerCase() + .trim(); + const placeholder = `[${fieldName}]`; preview.innerText = input.value || placeholder; }); } }This makes placeholders more readable (e.g.,
[company name]instead of[CompanyName]).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
invoice.html(1 hunks)script.js(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- invoice.html
🔇 Additional comments (5)
script.js (5)
1-7: LGTM! Well-structured initialization sequence.The DOMContentLoaded event handler follows a logical order: setting initial values, applying saved theme, and then initializing event listeners. This is a clean and maintainable approach.
65-107: LGTM! Excellent event listener organization and delegation.The event listener setup demonstrates good practices:
- Proper null checks before attaching listeners
- Effective use of event delegation for dynamic elements
- Clean separation of concerns with helper functions
- Logical initialization sequence
164-223: LGTM! Excellent row management improvements.The row management refactoring demonstrates several best practices:
- Safe parsing with fallbacks in
calculateRowTotal()- Template-based row creation in
addNewRow()(more reliable than cloning existing rows)- Proper validation to prevent deletion of the last row
- Clean separation of concerns across functions
The switch to template cloning is particularly well done - it ensures new rows are always clean and consistent.
225-283: LGTM! Robust preview update with excellent safety checks.The
updatePreview()function demonstrates excellent defensive programming:
- Proper null checks for required elements
- Safe parsing with fallbacks for numeric inputs
- Dynamic tax label updates reflecting user input
- Consistent currency symbol handling throughout the preview
The implementation handles edge cases well and maintains data integrity.
285-344: LGTM! Comprehensive PDF download implementation with excellent UX.The
downloadPDF()function demonstrates production-ready code with excellent user experience:
- Proper loading state management with visual feedback
- Comprehensive error handling with user-friendly alerts
- Library availability checks to prevent runtime errors
- Enhanced html2pdf options with CORS support
- Consistent button state restoration in all scenarios (success, error, unavailable)
The implementation handles edge cases gracefully and provides clear feedback to users throughout the process.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
script.js (3)
19-23: Consider using CSS classes instead of inline styles.While the current implementation works, consider using CSS classes to toggle icon visibility for better separation of concerns and easier maintenance.
-function toggleThemeIcons(moonIcon, sunIcon, isDark) { - moonIcon.style.display = isDark ? "none" : "block"; - sunIcon.style.display = isDark ? "block" : "none"; -} +function toggleThemeIcons(moonIcon, sunIcon, isDark) { + moonIcon.classList.toggle("hidden", isDark); + sunIcon.classList.toggle("hidden", !isDark); +}Then add this CSS rule:
.hidden { display: none; }
25-35: Add theme value validation for robustness.The function handles localStorage access well with fallbacks, but consider validating the saved theme value to prevent unexpected behavior.
function applySavedTheme() { - const savedTheme = (window.localStorage && window.localStorage.getItem("theme")) || "light"; + const rawTheme = window.localStorage && window.localStorage.getItem("theme"); + const savedTheme = (rawTheme === "dark" || rawTheme === "light") ? rawTheme : "light"; const { themeToggleBtn, moonIcon, sunIcon, body } = getThemeElements();
81-91: Consider breaking down the large event listener function.While the consolidated approach works well, the
initializeEventListeners()function is becoming quite large. Consider extracting specific listener groups into separate functions for better maintainability.function initializeEventListeners() { const { themeToggleBtn, moonIcon, sunIcon, body } = getThemeElements(); - - // Theme toggle logic... - // Currency selector logic... - // Tax rate logic... - // Add row button logic... - // Download button logic... - // Event delegation... + + setupThemeToggle(themeToggleBtn, moonIcon, sunIcon, body); + setupFormControls(); + setupEventDelegation(); bindInitialRows(); setupFormSync(); updatePreview(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
script.js(4 hunks)
🔇 Additional comments (7)
script.js (7)
9-17: LGTM! Clean theme element caching.The
getThemeElements()function provides a clean way to cache DOM references and prevent repeated queries. The destructuring pattern makes the code readable and maintainable.
37-53: Excellent error handling for localStorage operations.The theme toggle implementation correctly handles localStorage failures and includes proper error logging. The defensive programming approach with null checks prevents runtime errors.
114-157: Excellent refactoring of form synchronization.The centralized mapping array approach and improved placeholder generation logic are significant improvements. The regular expression for converting camelCase to readable format is well-implemented and handles edge cases properly.
195-207: Great improvement using template-based row cloning.The switch from cloning existing rows to using a template element is a significant improvement. This ensures clean new rows without inherited data and provides more predictable behavior.
181-193: Robust calculation with proper error handling.The
calculateRowTotal()function properly handles missing inputs and edge cases. The conditional formatting of the total value (showing empty string for zero) provides good UX.
220-278: Comprehensive and robust preview update implementation.The
updatePreview()function handles all edge cases well, includes proper safety checks, and maintains data consistency. The dynamic tax label updates and currency symbol synchronization are particularly well-implemented.
280-335: Excellent enhancement of PDF download functionality.The improved error handling, user feedback during generation, and enhanced html2pdf options significantly improve reliability. The button state management and CORS support address common PDF generation issues effectively.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2 participants
🎯 Fixes Issue #3
This PR implements a dark/light theme toggle feature with localStorage persistence as requested.
✨ Features Implemented
🎨 Theme Design
🧪 Testing Done
📱 User Experience
The theme toggle improves UX by:
Ready for review! This enhancement makes the GenInvoice app more accessible and user-friendly. 🚀
Summary by CodeRabbit
New Features
Style
Bug Fixes
Refactor
Chores