Skip to content

Enhance JS: Add theme memory, mobile nav auto-close, email validation… #6

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Enhance JS: Add theme memory, mobile nav auto-close, email validation… #6

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 79 additions & 20 deletions assets/js/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,100 @@
'use strict';

// navbar variables
// ===== Navbar Toggle =====
const nav = document.querySelector('.mobile-nav');
const navMenuBtn = document.querySelector('.nav-menu-btn');
const navCloseBtn = document.querySelector('.nav-close-btn');


// navToggle function
const navToggleFunc = function () { nav.classList.toggle('active'); }
const navToggleFunc = () => {
nav.classList.toggle('active');
};

navMenuBtn.addEventListener('click', navToggleFunc);
navCloseBtn.addEventListener('click', navToggleFunc);
Comment on lines +8 to 13
Copy link

@coderabbitai coderabbitai bot Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Guard potential nulls for nav elements to avoid runtime errors

If any of .mobile-nav, .nav-menu-btn, or .nav-close-btn is missing on a page, the current code can throw at runtime. Minimal hardening below.

-const navToggleFunc = () => {
-  nav.classList.toggle('active');
-};
+const navToggleFunc = () => {
+  nav?.classList.toggle('active');
+};

-navMenuBtn.addEventListener('click', navToggleFunc);
-navCloseBtn.addEventListener('click', navToggleFunc);
+navMenuBtn?.addEventListener('click', navToggleFunc);
+navCloseBtn?.addEventListener('click', navToggleFunc);
📝 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
const navToggleFunc = () => {
nav.classList.toggle('active');
};
navMenuBtn.addEventListener('click', navToggleFunc);
navCloseBtn.addEventListener('click', navToggleFunc);
const navToggleFunc = () => {
nav?.classList.toggle('active');
};
navMenuBtn?.addEventListener('click', navToggleFunc);
navCloseBtn?.addEventListener('click', navToggleFunc);
🤖 Prompt for AI Agents 
In assets/js/script.js around lines 8 to 13, the code assumes elements
.mobile-nav, .nav-menu-btn, and .nav-close-btn always exist which can throw if
any are missing; add null guards: check that nav, navMenuBtn, and navCloseBtn
are non-null before using them (either return early if nav is missing or wrap
nav.classList.toggle in a conditional/optional chaining), and only call
addEventListener when the corresponding button element exists so event wiring is
skipped safely on pages without those elements.


// Close mobile nav on nav link click
const mobileNavLinks = document.querySelectorAll('.mobile-nav .nav-link');
mobileNavLinks.forEach(link => {
link.addEventListener('click', () => {
nav.classList.remove('active');
});
});


// theme toggle variables
// ===== Theme Toggle with LocalStorage =====
const themeBtn = document.querySelectorAll('.theme-btn');
const body = document.body;

// Apply saved theme on load
const savedTheme = localStorage.getItem('theme');
if (savedTheme === 'dark') {
body.classList.add('dark-theme');
body.classList.remove('light-theme');
themeBtn.forEach(btn => {
btn.classList.add('dark');
btn.classList.remove('light');
});
} else {
body.classList.add('light-theme');
themeBtn.forEach(btn => {
btn.classList.add('light');
btn.classList.remove('dark');
});
}
Comment on lines +28 to +43
Copy link

@coderabbitai coderabbitai bot Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Theme persistence: storage safety + exclusive classes + better default

  • localStorage access can throw in some contexts; wrap in try/catch.
  • Ensure exclusivity by removing the opposite theme in both branches.
  • Optional: default to system preference when no saved theme.
-// Apply saved theme on load
-const savedTheme = localStorage.getItem('theme');
+// Apply saved theme on load (guard storage access)
+let savedTheme;
+try { savedTheme = localStorage.getItem('theme'); } catch (_) { savedTheme = null; }
+const prefersDark = window.matchMedia?.('(prefers-color-scheme: dark)').matches;

-if (savedTheme === 'dark') {
+if (savedTheme === 'dark' || (!savedTheme && prefersDark)) {
   body.classList.add('dark-theme');
   body.classList.remove('light-theme');
   themeBtn.forEach(btn => {
     btn.classList.add('dark');
     btn.classList.remove('light');
   });
 } else {
   body.classList.add('light-theme');
+  body.classList.remove('dark-theme');
   themeBtn.forEach(btn => {
     btn.classList.add('light');
     btn.classList.remove('dark');
   });
 }

Also guard setItem:

-    localStorage.setItem('theme', isDark ? 'dark' : 'light');
+    try { localStorage.setItem('theme', isDark ? 'dark' : 'light'); } catch (_) {}

Also applies to: 56-56


// Toggle theme on button click
themeBtn.forEach(btn => {
btn.addEventListener('click', () => {
const isDark = body.classList.toggle('dark-theme');
body.classList.toggle('light-theme');

themeBtn.forEach(btn => {
btn.classList.toggle('dark');
btn.classList.toggle('light');
});

localStorage.setItem('theme', isDark ? 'dark' : 'light');
});
});


// ===== Newsletter Form Validation =====
const newsletterForm = document.querySelector('.newsletter form');

if (newsletterForm) {
newsletterForm.addEventListener('submit', function (e) {
const emailInput = this.querySelector('input[name="email"]');
const email = emailInput.value.trim();

if (!email || !email.includes('@')) {
e.preventDefault(); // Stop form submission
alert('Please enter a valid email address.');
}
});
}
Comment on lines +61 to +74
Copy link

@coderabbitai coderabbitai bot Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Email validation: use HTML5 validity and null guards

includes('@') is too weak and emailInput may be null. Use the browser’s validator when possible.

-  newsletterForm.addEventListener('submit', function (e) {
-    const emailInput = this.querySelector('input[name="email"]');
-    const email = emailInput.value.trim();
-
-    if (!email || !email.includes('@')) {
-      e.preventDefault(); // Stop form submission
-      alert('Please enter a valid email address.');
-    }
-  });
+  newsletterForm.addEventListener('submit', function (e) {
+    const emailInput = this.querySelector('input[type="email"], input[name="email"]');
+    const email = emailInput?.value.trim() || '';
+    const isValid = emailInput
+      ? (emailInput.type === 'email' ? emailInput.checkValidity() : /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email))
+      : false;
+    if (!isValid) {
+      e.preventDefault();
+      alert('Please enter a valid email address.');
+      emailInput?.focus();
+    }
+  });

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In assets/js/script.js around lines 61 to 74, the current validation uses
emailInput without guarding for null and only checks for '@', which is weak;
update the handler to first guard that emailInput exists, then rely on the
browser’s HTML5 validation by ensuring the input is treated as type="email" (or
check its type) and use emailInput.checkValidity() or emailInput.validity.valid
(and emailInput.reportValidity() to show the built-in message) after trimming
the value; if invalid call e.preventDefault() and optionally
setCustomValidity('Please enter a valid email address') before reportValidity(),
clearing custom validity when valid.



for (let i = 0; i < themeBtn.length; i++) {
// ===== Load More Blog Cards =====
const blogCards = document.querySelectorAll('.blog-card');
const loadMoreBtn = document.querySelector('.load-more');

themeBtn[i].addEventListener('click', function () {
if (blogCards.length && loadMoreBtn) {
let visibleCards = 3;

// toggle `light-theme` & `dark-theme` class from `body`
// when clicked `theme-btn`
document.body.classList.toggle('light-theme');
document.body.classList.toggle('dark-theme');
const showCards = count => {
blogCards.forEach((card, i) => {
card.style.display = i < count ? 'block' : 'none';
});
};

for (let i = 0; i < themeBtn.length; i++) {
showCards(visibleCards);

// When the `theme-btn` is clicked,
// it toggles classes between `light` & `dark` for all `theme-btn`.
themeBtn[i].classList.toggle('light');
themeBtn[i].classList.toggle('dark');
loadMoreBtn.addEventListener('click', () => {
visibleCards += 3;
showCards(visibleCards);

if (visibleCards >= blogCards.length) {
loadMoreBtn.style.display = 'none';
}

})

}
});
}