Fix: Navbar links issue on the login page #944
Conversation
WalkthroughThe changes made in Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 2
🧹 Outside diff range and nitpick comments (2)
login.html (2)
Line range hint 89-92: Update GitHub profile link placeholder.
The GitHub authentication link contains a placeholder URL that needs to be updated.
- <a href="https://github.com/YourGitHubProfile" target="_blank">
+ <a href="https://github.com/login/oauth/authorize?client_id=YOUR_CLIENT_ID" target="_blank">Line range hint 108-111: Remove redundant hash (#) symbols from footer links.
For consistency with the navbar changes, update the footer links to point to actual pages instead of using hash symbols.
- <li><a href="#">Our Story</a></li>
- <li><a href="#">Team</a></li>
- <li><a href="#">Careers</a></li>
+ <li><a href="about.html#our-story">Our Story</a></li>
+ <li><a href="about.html#team">Team</a></li>
+ <li><a href="careers.html">Careers</a></li>📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- login.html (1 hunks)
🔇 Additional comments (1)
login.html (1)
19-27: LGTM! Navigation structure has been properly updated.
The navbar links now correctly point to their respective HTML files, addressing the core issue from #935.
| <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> | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Several navigation inconsistencies need attention
- Multiple pages still contain placeholder
href="#"links that need proper destinations - The
login-linkimplementation 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
| <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> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve login link visibility handling.
The current implementation has several potential issues:
- Might not work with different URL structures (e.g., query parameters, different paths)
- Could cause layout shift if executed after page load
- 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.
|
could you please review my PR? 😊 |
Hi GSSOC 2024 extd team this is the pr i have raised and it's not been merged. its Level 1 PR could yo review and merge it. GarimaSingh0109/Resum-Resume#968 vishanurag/Canvas-Editor#737 GarimaSingh0109/Resum-Resume#944
|
This PR cannot be merged as you were not assigned to work on it. |
Pull Request for Resum-Resume 💡
Issue Title : Navbar links are not working on the Login page .
Closes: #935
Info about the related issue (Aim of the project) :
This PR fixes the issue where the navbar links were not functioning correctly on the login page by ensuring they redirect properly to other sections/pages.
Name : Srujana H B
GitHub ID : hbSrujana
Email ID : hbsrujana@gmail.com
Screenshots :
![Uploading Screenshot 2024-10-30 142313.png…]()
![Uploading Screenshot 2024-10-30 142404.png…]()
Describe the add-ons or changes you've made 📃 :
Type of change ☑️ :
Bug fix (non-breaking change which fixes an issue)
What sort of change have you made:
How Has This Been Tested? ⚙️ :
Checklist: ☑️
My code follows the guidelines of this project.
I have performed a self-review of my own code.
I have commented my code, particularly wherever it was hard to understand.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have added things that prove my fix is effective or that my feature works.
Summary by CodeRabbit