Skip to content
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

Fix: Navbar links issue on the login page #944

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

Fix: Navbar links issue on the login page #944

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
26 changes: 17 additions & 9 deletions login.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,23 @@
<!-- Navbar section -->
<header>
<a href="index.html"><h1>Resum Resume<span>.</span></h1></a>
<nav>
<ul id="nav-menu">
<li><a href="#home">Home</a></li>
<li><a href="#about.html">About Us</a></li>
<li><a href="#resume.html">Resume</a></li>
<li><a href="#signup.html">Sign Up</a></li>
<li><a href="login.html">Login</a></li>
</ul>
</nav>
<nav>
<ul id="nav-menu">
<li><a href="index.html">Home</a></li>
<li><a href="about.html">About Us</a></li>
<li><a href="resume.html">Resume</a></li>
<li><a href="signup.html">Sign Up</a></li>
<li id="login-link"><a href="login.html">Login</a></li>
</ul>
</nav>

<script>
// Hide the "Login" link if the user is on the login page
if (window.location.pathname.endsWith("login.html")) {
document.getElementById("login-link").style.display = "none";
}
</script>
Comment on lines +29 to +34
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve login link visibility handling.

The current implementation has several potential issues:

  1. Might not work with different URL structures (e.g., query parameters, different paths)
  2. Could cause layout shift if executed after page load
  3. No fallback if script fails

Consider this more robust implementation:

   <script>
-      // Hide the "Login" link if the user is on the login page
-      if (window.location.pathname.endsWith("login.html")) {
-          document.getElementById("login-link").style.display = "none";
-      }
+      // Hide login link immediately to prevent layout shift
+      document.documentElement.style.visibility = 'hidden';
+      
+      // More robust page detection
+      document.addEventListener('DOMContentLoaded', function() {
+          try {
+              const loginLink = document.getElementById("login-link");
+              if (window.location.pathname.toLowerCase().includes('login')) {
+                  loginLink.style.display = "none";
+              }
+              // Ensure link is visible if script fails to detect page
+              loginLink.style.visibility = 'visible';
+          } catch (e) {
+              console.error('Error handling login link visibility:', e);
+          } finally {
+              document.documentElement.style.visibility = 'visible';
+          }
+      });
   </script>

Committable suggestion was skipped due to low confidence.


Comment on lines +19 to +35
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Several navigation inconsistencies need attention

  • Multiple pages still contain placeholder href="#" links that need proper destinations
  • The login-link implementation varies across pages:
    • login.html: <li id="login-link"><a href="login.html">
    • Other pages: <a href="login.html" id="login-link"> (without <li> wrapper)
  • Some referenced files are missing:
    • cv.html
    • comment.html
    • privacy-policy.html
🔗 Analysis chain

Verify navbar functionality across pages.

The changes look good, but let's verify the implementation across different scenarios.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify navbar implementation across pages
# Expected results: All pages should have consistent navbar structure and working links

# Check for any remaining hash-only href attributes
echo "Checking for remaining hash-only hrefs..."
rg 'href="#"' --type html

# Verify that all referenced HTML files exist
echo "Verifying referenced HTML files..."
for file in index.html about.html resume.html signup.html cv.html Faqs.html comment.html privacy-policy.html; do
  if [ ! -f "$file" ]; then
    echo "Warning: Referenced file $file does not exist"
  fi
done

# Check for consistent login-link implementation across pages
echo "Checking login link implementation..."
rg 'id="login-link"' --type html

Length of output: 7730

<div class="nav-controls">
<a href="about.html" class="fas fa-info-circle"></a>
<a href="login.html" id="loginIcon" class="fas fa-user"></a>
Expand Down