Skip to content

Commit 0ffbf80

Browse files
committed
Improve dropdown menu styling and fix event listener memory leaks
1 parent cc49a3c commit 0ffbf80

File tree

1 file changed

+105
-37
lines changed

1 file changed

+105
-37
lines changed

public/js/components/pf-header.js

Lines changed: 105 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ class PfHeader extends HTMLElement {
66
super();
77
this.attachShadow({ mode: 'open' });
88
this.currentTheme = document.documentElement.getAttribute('data-theme') || 'light';
9+
10+
// Initialize properties
11+
this._documentClickListenerAdded = false;
12+
this._dropdownEventListeners = new Map();
913
}
1014

1115
connectedCallback() {
@@ -224,7 +228,8 @@ class PfHeader extends HTMLElement {
224228
width: 200px;
225229
background-color: var(--background-color, white);
226230
border-radius: 5px;
227-
box-shadow: 0 2px 10px rgba(0, 0, 0, 0.1);
231+
box-shadow: 0 2px 10px rgba(0, 0, 0, 0.2);
232+
border: 1px solid var(--border-color, #ddd);
228233
padding: 8px 0;
229234
display: none;
230235
z-index: 100;
@@ -451,9 +456,12 @@ class PfHeader extends HTMLElement {
451456
const loginLink = navLinks.querySelector('.login-link');
452457
const registerLink = navLinks.querySelector('.register-link');
453458

454-
// Clear existing dynamic auth-related elements
455-
const authElements = navLinks.querySelectorAll('.user-dropdown');
456-
authElements.forEach(el => el.remove());
459+
// Only remove existing dropdown if we're going to add a new one
460+
// (when user is logged in) or if we're switching from logged in to logged out
461+
if (isLoggedIn || navLinks.querySelector('.user-dropdown')) {
462+
const authElements = navLinks.querySelectorAll('.user-dropdown');
463+
authElements.forEach(el => el.remove());
464+
}
457465

458466
// Update mobile navigation
459467
const mobileMenu = this.shadowRoot.querySelector('.mobile-menu');
@@ -488,53 +496,113 @@ class PfHeader extends HTMLElement {
488496
`;
489497

490498
// Insert the dropdown before the theme toggle
491-
const themeToggle = navLinks.querySelector('.theme-toggle');
492-
if (themeToggle) {
493-
themeToggle.insertAdjacentHTML('beforebegin', dropdownHtml);
494-
} else {
495-
navLinks.insertAdjacentHTML('beforeend', dropdownHtml);
499+
try {
500+
const themeToggle = navLinks.querySelector('.theme-toggle');
501+
if (themeToggle) {
502+
// Insert before the theme toggle
503+
themeToggle.insertAdjacentHTML('beforebegin', dropdownHtml);
504+
console.log('Dropdown inserted before theme toggle');
505+
} else {
506+
// Fallback: append to the end of nav links
507+
navLinks.insertAdjacentHTML('beforeend', dropdownHtml);
508+
console.log('Dropdown appended to nav links (theme toggle not found)');
509+
}
510+
511+
// Verify the dropdown was added
512+
const dropdown = navLinks.querySelector('.user-dropdown');
513+
if (!dropdown) {
514+
console.error('Failed to insert dropdown menu');
515+
}
516+
} catch (error) {
517+
console.error('Error inserting dropdown menu:', error);
518+
// Fallback: try one more time with direct append
519+
try {
520+
navLinks.insertAdjacentHTML('beforeend', dropdownHtml);
521+
} catch (e) {
522+
console.error('Final attempt to insert dropdown failed:', e);
523+
}
496524
}
497525

498526
// Add user-related links to mobile menu
499-
if (mobileMenu && !mobileMenu.querySelector('.mobile-settings-link')) {
500-
const mobileUserLinksHtml = `
501-
<a href="/settings" class="nav-link mobile-settings-link">Settings</a>
502-
<a href="#" class="nav-link mobile-logout-link">Logout</a>
503-
`;
504-
mobileMenu.insertAdjacentHTML('beforeend', mobileUserLinksHtml);
505-
506-
// Add event listener for mobile logout
507-
const mobileLogoutLink = mobileMenu.querySelector('.mobile-logout-link');
508-
if (mobileLogoutLink) {
509-
mobileLogoutLink.addEventListener('click', (e) => {
510-
e.preventDefault();
511-
this.logout();
512-
});
527+
if (mobileMenu) {
528+
// Only add if they don't already exist
529+
if (!mobileMenu.querySelector('.mobile-settings-link')) {
530+
try {
531+
const mobileUserLinksHtml = `
532+
<a href="/settings" class="nav-link mobile-settings-link">Settings</a>
533+
<a href="#" class="nav-link mobile-logout-link">Logout</a>
534+
`;
535+
mobileMenu.insertAdjacentHTML('beforeend', mobileUserLinksHtml);
536+
console.log('Mobile menu items added');
537+
538+
// Add event listener for mobile logout
539+
const mobileLogoutLink = mobileMenu.querySelector('.mobile-logout-link');
540+
if (mobileLogoutLink) {
541+
const handleMobileLogout = (e) => {
542+
e.preventDefault();
543+
this.logout();
544+
};
545+
546+
// Remove any existing event listeners (to prevent duplicates)
547+
mobileLogoutLink.removeEventListener('click', handleMobileLogout);
548+
// Add the event listener
549+
mobileLogoutLink.addEventListener('click', handleMobileLogout);
550+
} else {
551+
console.error('Mobile logout link not found after insertion');
552+
}
553+
} catch (error) {
554+
console.error('Error adding mobile menu items:', error);
555+
}
556+
} else {
557+
console.log('Mobile menu items already exist');
513558
}
559+
} else {
560+
console.error('Mobile menu element not found');
514561
}
515562

516563
// Add event listener for dropdown toggle
517564
const dropdownButton = navLinks.querySelector('.dropdown-button');
518565
const dropdownMenu = navLinks.querySelector('.dropdown-menu');
519566

520-
dropdownButton.addEventListener('click', (e) => {
521-
e.preventDefault();
522-
dropdownMenu.classList.toggle('show');
523-
});
524-
525-
// Close dropdown when clicking outside
526-
document.addEventListener('click', (e) => {
527-
if (!dropdownButton.contains(e.target) && !dropdownMenu.contains(e.target)) {
528-
dropdownMenu.classList.remove('show');
567+
if (dropdownButton && dropdownMenu) {
568+
// Use a named function so we can remove it if needed
569+
const toggleDropdown = (e) => {
570+
e.preventDefault();
571+
dropdownMenu.classList.toggle('show');
572+
};
573+
574+
// Remove any existing event listeners (to prevent duplicates)
575+
dropdownButton.removeEventListener('click', toggleDropdown);
576+
// Add the event listener
577+
dropdownButton.addEventListener('click', toggleDropdown);
578+
579+
// Close dropdown when clicking outside
580+
const closeDropdown = (e) => {
581+
if (!dropdownButton.contains(e.target) && !dropdownMenu.contains(e.target)) {
582+
dropdownMenu.classList.remove('show');
583+
}
584+
};
585+
586+
// Use a flag in the DOM to track if we've already added the document listener
587+
if (!this._documentClickListenerAdded) {
588+
document.addEventListener('click', closeDropdown);
589+
this._documentClickListenerAdded = true;
529590
}
530-
});
591+
}
531592

532593
// Add event listener for logout
533594
const logoutButton = navLinks.querySelector('.logout-button');
534-
logoutButton.addEventListener('click', (e) => {
535-
e.preventDefault();
536-
this.logout();
537-
});
595+
if (logoutButton) {
596+
const handleLogout = (e) => {
597+
e.preventDefault();
598+
this.logout();
599+
};
600+
601+
// Remove any existing event listeners (to prevent duplicates)
602+
logoutButton.removeEventListener('click', handleLogout);
603+
// Add the event listener
604+
logoutButton.addEventListener('click', handleLogout);
605+
}
538606
} else {
539607
// User is not logged in
540608

0 commit comments

Comments
 (0)