Enhancements: dark mode, books page, added menu #1
Conversation
…holder; improve alt text
…es, canonical, OG/Twitter meta, menu nav, heading hierarchy)
…improve maintainability
There was a problem hiding this comment.
Pull Request Overview
This PR implements a dark mode theme toggle, adds a new books page, and introduces a navigation menu to the website. The changes modernize the UI with better accessibility features and refactor inline styles into a centralized CSS file.
- Dark mode support with theme toggle and localStorage persistence
- New dedicated books page showcasing a reading list with book covers fetched from Open Library
- Navigation menu system with dropdown functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| index.html | Added SEO meta tags, refactored inline CSS to use CSS variables, implemented theme toggle and menu dropdown, improved accessibility with skip links and ARIA attributes |
| css/styles.css | New centralized stylesheet with CSS variable-based theming system supporting light and dark modes, shared component styles for both pages |
| books.html | New page displaying a reading list with book covers, ISBN-based cover fetching with fallback logic, and matching theme/menu implementations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@arjun7965 I've opened a new pull request, #2, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@arjun7965 I've opened a new pull request, #3, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@arjun7965 I've opened a new pull request, #4, to work on those changes. Once the pull request is ready, I'll request review from you. |
The regex pattern [^0-9Xx] will match lowercase 'x' but the .toUpperCase() conversion happens after the replacement. This means a lowercase 'x' in the input will be removed before it can be converted to uppercase 'X'. The regex should be [^0-9X]/gi with case-insensitive flag, or handle case conversion before the regex replacement. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Amazon URL contains tracking parameters (dib, dib_tag, keywords, qid, sr). Consider using a clean URL with only the essential ASIN/ISBN for better privacy and cleaner code. The URL could be shortened to https://www.amazon.com/dp/B0F2NCPQ2K. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: arjun7965 <15646612+arjun7965@users.noreply.github.com>
Co-authored-by: arjun7965 <15646612+arjun7965@users.noreply.github.com>
Co-authored-by: arjun7965 <15646612+arjun7965@users.noreply.github.com>
Refactor: extract duplicated menu dropdown code to shared module
Consolidate duplicate theme toggle code into shared JS module
Refactor: extract shared JavaScript to common.js
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (2)
books.html:151
- Unused variable settled.
let settled = false;
books.html:152
- Unused variable remaining.
let remaining = isbns.length;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@arjun7965 I've opened a new pull request, #13, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@arjun7965 I've opened a new pull request, #14, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: arjun7965 <15646612+arjun7965@users.noreply.github.com>
Co-authored-by: arjun7965 <15646612+arjun7965@users.noreply.github.com>
Add inline documentation to ISBN conversion algorithms
Remove inline event handlers for CSP compliance
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.