-
Notifications
You must be signed in to change notification settings - Fork 5
Updated the logo on the home and invoice page when user selects dark mode #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@NicholasCloud4 is attempting to deploy a commit to the Eshita's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdded id="logoImage" to header logos in HTML, reorganized index.html and invoice.html into standard HTML5 structures, and introduced updateLogo(isDark) in script.js. The theme toggle now calls updateLogo to switch logo images based on theme. No other behavioral changes; existing navigation and invoice logic remain. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Toggle as Theme Toggle (UI)
participant JS as script.js
participant DOM as Document
participant Logo as #logoImage
User->>Toggle: Click
Toggle->>JS: initializeEventListeners handler
JS->>DOM: Toggle body.dark-theme
JS->>JS: compute isDark
note right of JS: New: updateLogo(isDark)
JS->>Logo: updateLogo(isDark)<br/>• isDark=true → gilogo.png<br/>• isDark=false → gil.png
Logo-->>User: Logo reflects current theme
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script.js (1)
25-35: Logo doesn’t sync on initial load with saved dark theme.
applySavedTheme sets icons/body but not the logo; users who previously chose dark mode will still see the light logo until they toggle. Also, the early return blocks applying the saved theme if icons are missing.function applySavedTheme() { // Retrieve the theme from local storage. Default is "light". const savedTheme = (window.localStorage && window.localStorage.getItem("theme")) || "light"; const { themeToggleBtn, moonIcon, sunIcon, body } = getThemeElements(); - if (!themeToggleBtn || !moonIcon || !sunIcon || !body) return; + if (!body) return; const isDark = savedTheme === "dark"; - body.classList.toggle("dark-theme", isDark); - toggleThemeIcons(moonIcon, sunIcon, isDark); + body.classList.toggle("dark-theme", isDark); + if (moonIcon && sunIcon) toggleThemeIcons(moonIcon, sunIcon, isDark); + if (themeToggleBtn) themeToggleBtn.setAttribute("aria-pressed", String(isDark)); + updateLogo(isDark); }
🧹 Nitpick comments (5)
index.html (2)
26-33: Make theme toggle button accessible and non-submitting.
Add type and ARIA for better a11y and to avoid accidental form submits if the button is ever placed inside a form.- <button id="theme-toggle" class="theme-toggle-btn"> + <button id="theme-toggle" class="theme-toggle-btn" type="button" aria-label="Toggle dark mode" aria-pressed="false">
9-14: Preload both logo assets to avoid a visible swap on first toggle.
Preloading minimizes flicker when switching themes.<link rel="preconnect" href="https://fonts.googleapis.com" /> <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin /> + <link rel="preload" as="image" href="gil.png" /> + <link rel="preload" as="image" href="gilogo.png" /> <linkscript.js (1)
44-60: Expose theme state to assistive tech on toggle.
Keep aria-pressed in sync when users switch themes.const isDark = body.classList.contains("dark-theme"); try { window.localStorage?.setItem("theme", isDark ? "dark" : "light"); } catch (e) { console.warn("Failed to save theme preference:", e); } + themeToggleBtn.setAttribute("aria-pressed", String(isDark)); toggleThemeIcons(moonIcon, sunIcon, isDark); updateLogo(isDark);invoice.html (2)
27-34: Mirror accessibility and behavior improvements for the theme toggle here.- <button id="theme-toggle" class="theme-toggle-btn"> + <button id="theme-toggle" class="theme-toggle-btn" type="button" aria-label="Toggle dark mode" aria-pressed="false">
11-15: Preload logo assets to reduce swap flicker on toggle.<link rel="preconnect" href="https://fonts.googleapis.com" /> <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin /> + <link rel="preload" as="image" href="gil.png" /> + <link rel="preload" as="image" href="gilogo.png" /> <link
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
index.html(1 hunks)invoice.html(1 hunks)script.js(3 hunks)
🔇 Additional comments (3)
index.html (1)
19-20: Header logo id addition aligns with JS hook. LGTM.
This enables programmatic theme-based swapping via updateLogo().invoice.html (2)
20-21: Header logo id addition aligns with JS hook. LGTM.
This will swap correctly when dark mode is toggled.
233-236: Should the invoice preview logo follow theme as well?
Currently the preview always shows gilogo.png regardless of theme. Confirm whether that’s intentional (e.g., brand color fixed in PDFs) or if it should mirror the header behavior.
Updated the logo on the home and invoice page when user selects dark mode, it will change to the purple logo.
Summary by CodeRabbit
New Features
Style