refactored html with bootstrap + navbar#3
Open
ivan-lyf wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the gallery.html and year.html pages to use Bootstrap styling, replacing custom CSS files. The changes make the photo gallery pages consistent with the rest of the website by adding the standard navbar and footer, updating the background colors to dark theme, and improving the overall user experience.
Changes:
- Integrated Bootstrap framework and removed custom CSS files (gallery.css and year.css)
- Added consistent navbar and footer components to both gallery pages
- Replaced custom lightbox implementation with Bootstrap modal in year.html
- Added "Photo gallery" link to the main navigation dropdown in index.html
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| index.html | Added "Photo gallery" link to the "More" dropdown menu |
| gallery.html | Complete refactor with Bootstrap navbar, footer, and updated styling to match site theme |
| year.html | Complete refactor with Bootstrap navbar, footer, and modal-based photo viewer |
| css/gallery.css | Removed - styling now handled by Bootstrap |
| css/year.css | Removed - styling now handled by Bootstrap |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <footer class="bg-dark py-4 mt-auto"> | ||
| <div class="container px-5"> | ||
| <div class="row align-items-center justify-content-between flex-column flex-sm-row"> | ||
| <div class="col-auto"><div class="small m-0 text-white">© UBC Rocket 2024</div></div> |
There was a problem hiding this comment.
The copyright year is set to 2024, but it should be 2025 to match the current year and be consistent with other parts of the codebase.
Suggested change
| <div class="col-auto"><div class="small m-0 text-white">© UBC Rocket 2024</div></div> | |
| <div class="col-auto"><div class="small m-0 text-white">© UBC Rocket 2025</div></div> |
| <footer class="bg-dark py-4 mt-auto"> | ||
| <div class="container px-5"> | ||
| <div class="row align-items-center justify-content-between flex-column flex-sm-row"> | ||
| <div class="col-auto"><div class="small m-0 text-white">© UBC Rocket 2024</div></div> |
There was a problem hiding this comment.
The copyright year is set to 2024, but it should be 2025 to match the current year and be consistent with other parts of the codebase.
Suggested change
| <div class="col-auto"><div class="small m-0 text-white">© UBC Rocket 2024</div></div> | |
| <div class="col-auto"><div class="small m-0 text-white">© UBC Rocket 2025</div></div> |
Comment on lines
+110
to
+111
| <div class = "dropdown dropstart"> | ||
| <a class = "dropdown-item" href="gallery.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Photo gallery</a> |
There was a problem hiding this comment.
Inconsistent spacing in class attribute. Should be 'class="dropdown dropstart"' without extra spaces to match the formatting used elsewhere in the navbar.
Suggested change
| <div class = "dropdown dropstart"> | |
| <a class = "dropdown-item" href="gallery.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Photo gallery</a> | |
| <div class="dropdown dropstart"> | |
| <a class="dropdown-item" href="gallery.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Photo gallery</a> |
Comment on lines
+110
to
+111
| <div class = "dropdown dropstart"> | ||
| <a class = "dropdown-item" href="gallery.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Photo gallery</a> |
There was a problem hiding this comment.
Inconsistent spacing in class attribute. Should be 'class="dropdown-item"' without extra spaces to match the formatting used elsewhere in the navbar.
Suggested change
| <div class = "dropdown dropstart"> | |
| <a class = "dropdown-item" href="gallery.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Photo gallery</a> | |
| <div class="dropdown dropstart"> | |
| <a class="dropdown-item" href="gallery.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Photo gallery</a> |
| ] | ||
| } | ||
|
|
||
| // create section for each year |
There was a problem hiding this comment.
This comment line is incomplete or incorrectly placed. The comment "create section for each year" appears to be a todo or implementation note that should either be completed or removed. If this is meant to describe additional data structure, it should be properly formatted as a complete comment.
| const modalEl = document.getElementById('photoModal'); | ||
| const lightboxImg = document.getElementById('lightboxImg'); | ||
| const lightboxCaption = document.getElementById('lightboxCaption'); | ||
| const modalInstance = modalEl ? new bootstrap.Modal(modalEl) : null; |
There was a problem hiding this comment.
The modal implementation references Bootstrap's Modal class but doesn't check if Bootstrap is properly loaded before instantiation. If Bootstrap fails to load, this will throw an error. Consider adding a check for 'window.bootstrap' before creating the modal instance.
Suggested change
| const modalInstance = modalEl ? new bootstrap.Modal(modalEl) : null; | |
| const modalInstance = (modalEl && window.bootstrap && typeof window.bootstrap.Modal === 'function') | |
| ? new window.bootstrap.Modal(modalEl) | |
| : null; |
Comment on lines
+18
to
+115
| <main class="flex-shrink-0"> | ||
| <nav class="navbar navbar-expand-lg navbar-dark bg-dark fixed-top" id="mainNav" style="background-color: #212529 !important;"> | ||
| <div class="container"> | ||
| <a class="navbar-brand" href="index.html"> | ||
| <img src="./assets/ubcrocket_logo_white_long.png" alt="UBC Rocket logo" class="img-fluid img-logo"> | ||
| </a> | ||
| <div class="ms-auto d-block d-md-none"> | ||
| <a class="btn-social mx-2 my-2" href="https://facebook.com/ubcrocket" target="_blank"> | ||
| <i class="fab fa-facebook-f"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.instagram.com/ubcrocket/" target="_blank"> | ||
| <i class="fa-brands fa-instagram"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.linkedin.com/company/ubc-rocket/" target="_blank"> | ||
| <i class="fa-brands fa-linkedin-in"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.youtube.com/channel/UCxxgdake-xHWblLRLDJUpNw" target="_blank"> | ||
| <i class="fa-brands fa-youtube"></i> | ||
| </a> | ||
| </div> | ||
| <button class="navbar-toggler navbar-toggler-right" type="button" data-bs-toggle="collapse" data-bs-target="#navbarResponsive" aria-controls="navbarResponsive" aria-expanded="false" aria-label="Toggle navigation"> | ||
| Menu | ||
| <i class="fa fa-bars"></i> | ||
| </button> | ||
| <div class="collapse navbar-collapse" id="navbarResponsive"> | ||
| <div class="d-flex w-100"> | ||
| <ul class="navbar-nav me-auto"> | ||
| <li class="nav-item"><a class="nav-link" href="index.html">Home</a></li> | ||
| <li class="nav-item"><a class="nav-link" href="about.html">About</a></li> | ||
| <li class="nav-item dropdown dropdown-hover dropdown-hover-all"> | ||
| <a class="nav-link dropdown-toggle" href="#" id="projectsDropdown" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Current projects</a> | ||
| <div class="dropdown-menu" aria-labelledby="projectsDropdown"> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item dropdown-toggle" href="project-COTS.html" id="dropdown-A" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">COTS</a> | ||
| <div class="dropdown-menu" aria-labelledby="dropdown-A"> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-A">Payloads</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-B">Internals</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-C">Aerostructures</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-D">Avionics Hardware</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-E">Avionics Software</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-F">Recovery</a> | ||
| </div> | ||
| </div> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="project-LRE.html" id="dropdown-B" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Liquid Rocket Engine</a> | ||
| </div> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="project-TVR.html" id="dropdown-C" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Thrust Vectoring Rocket</a> | ||
| </div> | ||
| </div> | ||
| </li> | ||
| <li class="nav-item"><a class="nav-link" href="sponsors.html">Sponsors</a></li> | ||
| <li class="nav-item dropdown dropdown-hover dropdown-hover-all"> | ||
| <a class="nav-link dropdown-toggle" href="#" id="moreDropdown" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">More</a> | ||
| <div class="dropdown-menu" aria-labelledby="moreDropdown"> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="faq.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">FAQ</a> | ||
| </div> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="past-projects.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Past Projects</a> | ||
| </div> | ||
| </div> | ||
| </li> | ||
| <li class="nav-item"><a class="nav-link nav-pill nav-sponsor d-block d-md-none" href="index.html#sponsors">Sponsor Us</a></li> | ||
| <li class="nav-item"><a id="joinUsMobile" class="nav-link nav-pill d-block d-md-none" href="index.html#join-us">Join Us</a></li> | ||
| </ul> | ||
| <ul class="navbar-nav ms-auto d-none d-md-flex"> | ||
| <a class="btn-social mx-2 my-2" href="https://facebook.com/ubcrocket" target="_blank"> | ||
| <i class="fab fa-facebook-f"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.instagram.com/ubcrocket/" target="_blank"> | ||
| <i class="fa-brands fa-instagram"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.linkedin.com/company/ubc-rocket/" target="_blank"> | ||
| <i class="fa-brands fa-linkedin-in"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.youtube.com/channel/UCxxgdake-xHWblLRLDJUpNw" target="_blank"> | ||
| <i class="fa-brands fa-youtube"></i> | ||
| </a> | ||
| <li class="nav-item"><a class="nav-link mx-2 nav-pill nav-sponsor" href="index.html#sponsors">Sponsor Us</a></li> | ||
| <li class="nav-item"><a id="joinUsDesktop" class="nav-link mx-2 nav-pill" href="index.html#join-us">Join Us</a></li> | ||
| </ul> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </nav> | ||
| <header class="py-5 bg-dark border-bottom mt-5 pt-5"> | ||
| <div class="container d-flex flex-column gap-3 align-items-start align-items-md-center text-md-center text-light"> | ||
| <p class="text-uppercase fw-semibold mb-1 small text-light">Photos</p> | ||
| <h1 id="yearTitle" class="display-5 mb-0 w-100 text-center text-md-center fw-bold">UBC Rocket Photos</h1> | ||
| <a href="gallery.html" class="btn btn-outline-light btn-sm align-self-start align-self-md-center"> | ||
| <span aria-hidden="true">←</span> | ||
| <span class="ms-2">Back to Yearly Gallery</span> | ||
| </a> | ||
| </div> | ||
| </header> | ||
|
|
||
| <main class="container py-5 text-light" aria-live="polite"> |
There was a problem hiding this comment.
The main element is nested inside another main element, which violates HTML semantics. The outer main at line 18 should be changed to a div, as there should only be one main landmark per page. This affects accessibility and semantic HTML structure.
Comment on lines
+18
to
+113
| <main class="flex-shrink-0"> | ||
| <nav class="navbar navbar-expand-lg navbar-dark bg-dark fixed-top" id="mainNav" style="background-color: #212529 !important;"> | ||
| <div class="container"> | ||
| <a class="navbar-brand" href="index.html"> | ||
| <img src="./assets/ubcrocket_logo_white_long.png" alt="UBC Rocket logo" class="img-fluid img-logo"> | ||
| </a> | ||
| <div class="ms-auto d-block d-md-none"> | ||
| <a class="btn-social mx-2 my-2" href="https://facebook.com/ubcrocket" target="_blank"> | ||
| <i class="fab fa-facebook-f"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.instagram.com/ubcrocket/" target="_blank"> | ||
| <i class="fa-brands fa-instagram"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.linkedin.com/company/ubc-rocket/" target="_blank"> | ||
| <i class="fa-brands fa-linkedin-in"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.youtube.com/channel/UCxxgdake-xHWblLRLDJUpNw" target="_blank"> | ||
| <i class="fa-brands fa-youtube"></i> | ||
| </a> | ||
| </div> | ||
| <button class="navbar-toggler navbar-toggler-right" type="button" data-bs-toggle="collapse" data-bs-target="#navbarResponsive" aria-controls="navbarResponsive" aria-expanded="false" aria-label="Toggle navigation"> | ||
| Menu | ||
| <i class="fa fa-bars"></i> | ||
| </button> | ||
| <div class="collapse navbar-collapse" id="navbarResponsive"> | ||
| <div class="d-flex w-100"> | ||
| <ul class="navbar-nav me-auto"> | ||
| <li class="nav-item"><a class="nav-link" href="index.html">Home</a></li> | ||
| <li class="nav-item"><a class="nav-link" href="about.html">About</a></li> | ||
| <li class="nav-item dropdown dropdown-hover dropdown-hover-all"> | ||
| <a class="nav-link dropdown-toggle" href="#" id="projectsDropdown" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Current projects</a> | ||
| <div class="dropdown-menu" aria-labelledby="projectsDropdown"> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item dropdown-toggle" href="project-COTS.html" id="dropdown-A" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">COTS</a> | ||
| <div class="dropdown-menu" aria-labelledby="dropdown-A"> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-A">Payloads</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-B">Internals</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-C">Aerostructures</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-D">Avionics Hardware</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-E">Avionics Software</a> | ||
| <a class="dropdown-item" href="project-COTS.html#subteam-F">Recovery</a> | ||
| </div> | ||
| </div> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="project-LRE.html" id="dropdown-B" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Liquid Rocket Engine</a> | ||
| </div> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="project-TVR.html" id="dropdown-C" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Thrust Vectoring Rocket</a> | ||
| </div> | ||
| </div> | ||
| </li> | ||
| <li class="nav-item"><a class="nav-link" href="sponsors.html">Sponsors</a></li> | ||
| <li class="nav-item dropdown dropdown-hover dropdown-hover-all"> | ||
| <a class="nav-link dropdown-toggle" href="#" id="moreDropdown" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">More</a> | ||
| <div class="dropdown-menu" aria-labelledby="moreDropdown"> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="faq.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">FAQ</a> | ||
| </div> | ||
| <div class="dropdown dropstart"> | ||
| <a class="dropdown-item" href="past-projects.html" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Past Projects</a> | ||
| </div> | ||
| </div> | ||
| </li> | ||
| <li class="nav-item"><a class="nav-link nav-pill nav-sponsor d-block d-md-none" href="index.html#sponsors">Sponsor Us</a></li> | ||
| <li class="nav-item"><a id="joinUsMobile" class="nav-link nav-pill d-block d-md-none" href="index.html#join-us">Join Us</a></li> | ||
| </ul> | ||
| <ul class="navbar-nav ms-auto d-none d-md-flex"> | ||
| <a class="btn-social mx-2 my-2" href="https://facebook.com/ubcrocket" target="_blank"> | ||
| <i class="fab fa-facebook-f"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.instagram.com/ubcrocket/" target="_blank"> | ||
| <i class="fa-brands fa-instagram"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.linkedin.com/company/ubc-rocket/" target="_blank"> | ||
| <i class="fa-brands fa-linkedin-in"></i> | ||
| </a> | ||
| <a class="btn-social mx-2 my-2" href="https://www.youtube.com/channel/UCxxgdake-xHWblLRLDJUpNw" target="_blank"> | ||
| <i class="fa-brands fa-youtube"></i> | ||
| </a> | ||
| <li class="nav-item"><a class="nav-link mx-2 nav-pill nav-sponsor" href="index.html#sponsors">Sponsor Us</a></li> | ||
| <li class="nav-item"><a id="joinUsDesktop" class="nav-link mx-2 nav-pill" href="index.html#join-us">Join Us</a></li> | ||
| </ul> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </nav> | ||
| <a href="#gallery" class="visually-hidden-focusable position-absolute top-0 start-0 btn btn-secondary m-3">Skip to gallery</a> | ||
| <header class="py-5 bg-dark border-bottom mt-5 pt-5"> | ||
| <div class="container text-center text-light"> | ||
| <p class="text-uppercase fw-semibold mb-1 small text-light">Photos</p> | ||
| <h1 class="display-5 mb-2 fw-bold">UBC Rocket Photo Gallery</h1> | ||
| <p class="lead mb-0 text-light opacity-75">Explore our journey through the years</p> | ||
| </div> | ||
| </header> | ||
|
|
||
| <main id="gallery" class="container py-5 text-light" role="main" aria-label="Photo gallery organized by year"> |
There was a problem hiding this comment.
The main element is nested inside another main element, which violates HTML semantics. The outer main at line 18 should be changed to a div, as there should only be one main landmark per page. This affects accessibility and semantic HTML structure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactored gallery.html and year.html with bootstrap (imported from css/style.css).
Changed background colour to be consistent with other pages.
Added navbar and footer.
Added destination to gallery.html to navbar.
NOTE: currently no real photos, change place holders to actual images before merging (turning to public).
To create a 'year' card in gallery.html:
To add an image in year.html
