-
Notifications
You must be signed in to change notification settings - Fork 307
fix: fixed the broken website and Refactor styles and structure for improved layout and responsiveness #160
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
Conversation
- Updated profile.html to adjust stylesheet paths and navigation links. - Enhanced styles.css with new layout features, including a challenges filter section and improved card styles. - Introduced challenges.css with a new design for the challenges page, including a hero section and grid layout for challenge cards. - Added about.css for the about page with a cohesive design and responsive styles. - Modified themes.css to include card background variables for better theme management. - Implemented a footer design across pages for consistent navigation and information display.
|
let me know if any issues in the github pages host the linking of the stylesheet should be changed if any style is not applied during hosting |
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.
Greptile Summary
This PR represents a comprehensive refactoring of the CodeClip application's styling and navigation system. The changes transform the application from a basic functional interface to a modern, polished web application with consistent theming and responsive design.
Key Changes Made:
-
Navigation System Overhaul: Updated all navigation links across pages to use absolute paths with
/CodeClipprefixes, addressing deployment issues where the application is served from a subdirectory (likely GitHub Pages). -
Modular CSS Architecture: Introduced page-specific stylesheets (
challenges.css,about.css,profile.css) replacing the monolithicstyles.cssapproach. This creates better separation of concerns and maintainability. -
UI/UX Improvements: Added hero sections with gradient backgrounds, improved card layouts with hover effects, comprehensive filter controls for challenges, and a consistent footer design across all pages.
-
Enhanced Theming: Extended the existing theme system with new CSS custom properties for card backgrounds and better dark/light theme support.
-
Responsive Design: Implemented mobile-first responsive design with hamburger navigation menus, proper breakpoints, and mobile-optimized layouts.
-
Accessibility Enhancements: Added ARIA labels, semantic HTML elements (
<article>,<header>), and proper focus states throughout the application.
The refactoring integrates with the existing codebase by maintaining the router system structure (keeping data-route attributes) while fixing broken paths that prevented proper navigation. The changes align with the application's goal of being an interactive coding challenge platform by providing a more professional and user-friendly interface.
Confidence score: 2/5
• This PR has significant navigation and CSS architecture issues that could break core functionality
• Multiple critical problems including CSS variable conflicts, inconsistent path handling, hardcoded colors bypassing theme system, and potential router system conflicts
• Files needing attention: styles/about.css (CSS conflicts), pages/editor.html (router bypass), pages/profile.html (path inconsistencies), and styles.css (hardcoded values)
8 files reviewed, 14 comments
| display: none; /* hide by default */ | ||
| } | ||
|
|
||
| /* Hamburger CSS */ | ||
| .hamburger { | ||
| display: block; | ||
| width: 25px; | ||
| height: 3px; | ||
| background: var(--text-color); | ||
| margin: 4px 0; | ||
| transition: 0.4s; | ||
| .nav__list.open { | ||
| opacity: 1; | ||
| visibility: visible; | ||
| transform: translateY(0); | ||
| display: flex; /* show when open */ |
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.
style: Using both display:none/flex and opacity/visibility creates redundancy that could cause layout issues
| <a href="/CodeClip" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Home</a> | ||
| <a href="/CodeClip/pages/challenges.html" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Challenges</a> | ||
| <a href="/CodeClip/pages/editor.html" style="color: #4FC3F7; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; background: rgba(79, 195, 247, 0.1);">Editor</a> | ||
| <a href="/CodeClip/pages/profile.html" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Profile</a> |
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.
logic: Navigation links removed data-route attributes that were required for SPA routing. The router in app.js expects these attributes to handle client-side navigation.
| <a href="/CodeClip" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Home</a> | |
| <a href="/CodeClip/pages/challenges.html" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Challenges</a> | |
| <a href="/CodeClip/pages/editor.html" style="color: #4FC3F7; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; background: rgba(79, 195, 247, 0.1);">Editor</a> | |
| <a href="/CodeClip/pages/profile.html" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Profile</a> | |
| <a href="/CodeClip" data-route="/" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Home</a> | |
| <a href="/CodeClip/pages/challenges.html" data-route="/challenges" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Challenges</a> | |
| <a href="/CodeClip/pages/editor.html" data-route="/editor" style="color: #4FC3F7; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; background: rgba(79, 195, 247, 0.1);">Editor</a> | |
| <a href="/CodeClip/pages/profile.html" data-route="/profile" style="color: #f8f9fa; text-decoration: none; padding: 0.5rem 1rem; border-radius: 0.5rem; transition: background 0.2s;">Profile</a> |
| <script type="module" src="app.js"></script> | ||
| <script type="module" src="theme.js"></script> | ||
| <script src="challenges.js"></script> |
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.
logic: Script paths removed '../' prefix but files are in ../scripts/ directory - these will fail to load
| <script type="module" src="app.js"></script> | |
| <script type="module" src="theme.js"></script> | |
| <script src="challenges.js"></script> | |
| <script type="module" src="../scripts/app.js"></script> | |
| <script type="module" src="../scripts/theme.js"></script> | |
| <script src="../scripts/challenges.js"></script> |
| <link rel="stylesheet" href="../styles.css"> | ||
| <link rel="stylesheet" href="/styles/profile.css"> | ||
| <link rel="stylesheet" href="/CodeClip/styles.css"> | ||
| <link rel="stylesheet" href="..\styles\profile.css"> |
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.
syntax: Inconsistent path separator - uses backslashes instead of forward slashes like line 15. Should be ../styles/profile.css
| <link rel="stylesheet" href="..\styles\profile.css"> | |
| <link rel="stylesheet" href="../styles/profile.css"> |
| <li><a href="profile.html" data-route="/profile" class="nav__link">Profile</a></li> | ||
| <li><a href="about.html" class="nav__link">About</a></li> | ||
| <li><a href="index.html" data-route="/" class="nav__link">Home</a></li> | ||
| <li><a href="challenges.html" data-route="/challenges" class="nav__link active">Challenges</a></li> |
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.
logic: Challenges nav link marked as active on about page - should be the About link instead
| <li><a href="challenges.html" data-route="/challenges" class="nav__link active">Challenges</a></li> | |
| <li><a href="challenges.html" data-route="/challenges" class="nav__link">Challenges</a></li> |
| margin-bottom: 80px; | ||
| border-radius: 10px; |
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.
style: Adding 80px margin-bottom to a sticky header could cause unexpected spacing issues, especially on mobile devices.
|
|
||
|
|
||
| .content { | ||
| padding-top: 300px; |
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.
style: 300px top padding seems excessive for mobile. This could push content far below the fold.
| padding-top: 300px; | |
| padding-top: 80px; |
| <li><a href="/CodeClip" class="nav__link" aria-label="Home">Home</a></li> | ||
| <li><a href="/challenges" class="nav__link active" aria-current="page" aria-label="Challenges">Challenges</a></li> | ||
| <li><a href="/CodeClip/pages/editor.html" class="nav__link" aria-label="Editor">Editor</a></li> | ||
| <li><a href="/CodeClip/pages/profile.html" class="nav__link" aria-label="Profile">Profile</a></li> |
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.
logic: navigation links have inconsistent path structures - Home and Profile/Editor use /CodeClip prefix while Challenges uses root-relative /challenges
| <li><a href="/CodeClip" class="nav__link" aria-label="Home">Home</a></li> | |
| <li><a href="/challenges" class="nav__link active" aria-current="page" aria-label="Challenges">Challenges</a></li> | |
| <li><a href="/CodeClip/pages/editor.html" class="nav__link" aria-label="Editor">Editor</a></li> | |
| <li><a href="/CodeClip/pages/profile.html" class="nav__link" aria-label="Profile">Profile</a></li> | |
| <li><a href="/CodeClip" class="nav__link" aria-label="Home">Home</a></li> | |
| <li><a href="/CodeClip/pages/challenges.html" class="nav__link active" aria-current="page" aria-label="Challenges">Challenges</a></li> | |
| <li><a href="/CodeClip/pages/editor.html" class="nav__link" aria-label="Editor">Editor</a></li> | |
| <li><a href="/CodeClip/pages/profile.html" class="nav__link" aria-label="Profile">Profile</a></li> |
| <link href="https://fonts.googleapis.com/css2?family=Bitcount:[email protected]&family=Oooh+Baby&family=Inter:wght@400;500;600;700&display=swap" rel="stylesheet" /> | ||
|
|
||
| <!-- Link your separate CSS file here --> | ||
| <link rel="stylesheet" href="/CodeClip/styles/challenges.css" /> |
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.
style: absolute path /CodeClip/styles/challenges.css may break if deployed elsewhere or in development environments without this path structure
|
@adityai0 try the code by cloning into yourself i think it works fine and also let me know if any changes need! |
|
@Srivarshan-T resolve merge conflicts |
|
@adityai0 completely fixes the broken website changes the ui/ux the current website removed the editor and challenges section i added it also and profile page also here so kindly merge as soon as possible because it shows merge conflicts if pull request merged before my request |
fixes this issue #72