[Accessibility]: Fixed poor visibility of Navbar links and logo - #48
Merged
udaycodespace merged 2 commits intoAug 7, 2026
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses accessibility/readability issues in the main navbar by introducing a fixed foreground color token for navbar content on an always-dark navbar surface, and by aligning navbar link styling/ordering (including Saved and Log Out) for consistent visibility across themes.
Changes:
- Added a new CSS variable
--navbar-fgand updated navbar link/brand styling to use it for consistent contrast. - Introduced/standardized navbar link styling via
.premium-btn, adjusted Settings/brand presentation, and updated Log Out typography via.logout-btn. - Updated Saved courses navbar link styling and hover color behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/components/common/NavBar.jsx | Updates brand styling, Settings button presentation, and reorders/moves Saved + Log Out controls within the navbar layout. |
| frontend/src/components/bookmarks/Bookmarks.css | Adds fixed navbar-foreground coloring and hover styling for the Saved courses nav link. |
| frontend/src/App.css | Introduces --navbar-fg and adds global navbar link/brand/logout styling rules to ensure consistent contrast and hover behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1370
to
+1394
| .settings-btn { | ||
| background: transparent; | ||
| } | ||
|
|
||
| .settings-btn::after { | ||
| content: ''; | ||
| position: absolute; | ||
| left: 14px; | ||
| right: 14px; | ||
| bottom: 2px; | ||
| height: 2px; | ||
| background: var(--acid); | ||
| transform: scaleX(0); | ||
| transform-origin: left; | ||
| transition: transform 0.25s ease; | ||
| } | ||
|
|
||
| .settings-btn:hover { | ||
| color: var(--acid); | ||
| background: transparent; | ||
| } | ||
|
|
||
| .settings-btn:hover::after { | ||
| transform: scaleX(1); | ||
| } |
Comment on lines
+78
to
+82
| aria-haspopup="true" | ||
| aria-expanded={settingsOpen} | ||
| tabIndex={0} | ||
| > | ||
| <span style={{fontSize: '1.3rem'}}>⚙️</span> <span className="d-none d-md-inline">Settings</span> | ||
| <span className="d-none d-md-inline">Settings</span> |
Comment on lines
345
to
+355
| .saved-courses-nav-link { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| gap: 7px; | ||
| text-decoration: none; | ||
| color: var(--navbar-fg); | ||
| transition: color 0.2s ease; | ||
| font-family: "DM Sans", sans-serif; | ||
| font-size: 1rem; | ||
| font-weight: 600; | ||
| } |
udaycodespace
self-requested a review
August 7, 2026 04:47
udaycodespace
added
ECSoC26
Owner
|
@sujalv28 Nice work overall! However, there are a few changes that need to be addressed before this PR can be merged:
Once these are addressed, this PR should be good to go. |
Contributor
Author
|
Hello @udaycodespace I have checked and resolved the review comments. Can you check and merge it if it solves the issue. Thank You ! |
Owner
LGTM! |
ecsoc-sentinel
Bot
added
the
ECSoC26-L1
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.
Summary
This PR solves the issue #46 [Poor visibility of navbar links and logo]. The navbar background is fixed dark in both themes, but several elements inside it inherited theme-flipping colors instead of a fixed one, making the brand name, Home, and Enrolled Courses invisible in light mode and only readable in dark mode. Fixed with a dedicated fixed navbar-foreground constant, then addressed follow-on layout/consistency issues on Saved/Log Out found during verification.
Related Issue
Issue #46 [Poor visibility of navbar links and logo]
Closes #46 [Poor visibility of navbar links and logo]
What changed?
Visibility fixes (root cause: fixed-dark surface paired with theme-flipping or hardcoded-wrong foreground):
Hover consistency:
Layout / Re-ordering (Saved + Log Out):
Files touched: NavBar.jsx, App.css, Bookmarks.css, AdminHome.jsx, TeacherHome.jsx.
Type
Areas touched
Testing
Test steps
Screenshots & Screen Recordings
Before
After
Hover.Effect.mp4
Hover.effect.2.mp4
Edge cases checked
Other edge case details
Checklist
CONTRIBUTING.md