Skip to content

fix: increase navbar title font size to improve visibility (closes #114) #121

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.

Sign up for GitHub

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

Open
titojuanc wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: increase navbar title font size to improve visibility (closes #114) #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/Navbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const Navbar = () => {
<div className={styles["navbar-container"]}>
<nav className={styles["navbar"]}>
<Link href="/" className={styles["heading"]}>
<h3 className={styles["title"]}>OLD SCHOOL GAME</h3>
<h2 className={styles["title"]}>OLD SCHOOL GAME</h2>
</Link>
<Link href="https://github.com/ajaynegi45/Old-School-Game" className={styles.link}>Github</Link>
</nav>
Expand Down
4 changes: 2 additions & 2 deletions src/components/navbar.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@


.title {
font-size: 14px;
font-size: 30px;

@coderabbitai coderabbitai Bot Apr 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential layout overlap after increasing title size.

At Line 40, font-size: 30px combined with fixed navbar sizing can make the fixed header taller than the 4rem spacer, so page content may slide under the navbar.

Suggested fix (keep larger title, stabilize layout) 
 .navbar {
-    height: 4rem;
+    min-height: 4rem;
     display: flex;
     flex-direction: row;
     align-items: center;
     justify-content: space-between;
-    padding: 22px 0 20px 0;
+    padding: 12px 0;
     border-bottom: 1px solid `#E1E4E8`;
 }
 .empty-navbar{
     margin-top: 2rem;
-    height: 4rem;
+    height: 5.5rem;
     width: 95%;
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/navbar.module.css` at line 40, The increased title font-size
(font-size: 30px) can make the fixed header taller than the existing 4rem spacer
and cause content to slide under the navbar; update the CSS so the spacer
matches the navbar's computed height instead of a fixed 4rem—either increase the
spacer height to accommodate the larger title or make both the header (.navbar
or equivalent) and spacer use a shared CSS variable (e.g., --navbar-height) and
compute height from content (min-height or padding) so the spacer always matches
the header height; adjust the rules that reference the spacer class (e.g.,
.spacer, .navbar-spacer) and the fixed header class (.navbar) to use the same
variable or responsive sizing.

@codeant-ai codeant-ai Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Setting the title to a fixed 30px creates a responsive layout bug because the navbar has no mobile font-size override, so on narrow screens the title can overflow/collide with the right-side link. Use a responsive font size that caps at 30px instead of a fixed value. [logic error]

Severity Level: Major ⚠️ 
- ⚠️ Navbar title and GitHub link overlap on small screens.
- ⚠️ Root layout header appears cramped on mobile viewports.
Suggested change
font-size: 30px;
font-size: clamp(1.1rem, 5vw, 30px);
Steps of Reproduction ✅ 
1. Run the Next.js app and open the root page `/` in a narrow browser viewport (e.g.,
~360px width). This uses `RootLayout` in `src/app/layout.tsx:42-60`, which renders
`<Navbar/>` at `layout.tsx:58`.

2. Observe that `Navbar` (`src/components/Navbar.tsx:5-19`) renders the title as `<h2
className={styles["title"]}>OLD SCHOOL GAME</h2>` and the GitHub link as `<Link ...
className={styles.link}>Github</Link>` on a single flex row (`<nav
className={styles["navbar"]}>` at `Navbar.tsx:9`).

3. In `src/components/navbar.module.css:10-19`, `.navbar` is defined with `width: 95%;`,
`height: 4rem;`, `display: flex;`, and `justify-content: space-between;`. The mobile media
query at `navbar.module.css:71-74` is empty, so there is no smaller-screen font-size
override.

4. The same CSS file sets `.title` to a fixed `font-size: 30px;` at
`navbar.module.css:39-41`. On a narrow viewport, the long uppercase text "OLD SCHOOL GAME"
at 30px fills most of the 95% width, causing the right-side GitHub link to be pushed to
the edge or forced to wrap/clip inside the fixed 4rem navbar, producing overflow/collision
instead of a responsive scale-down.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/components/navbar.module.css
**Line:** 40:40
**Comment:**
	*Logic Error: Setting the title to a fixed `30px` creates a responsive layout bug because the navbar has no mobile font-size override, so on narrow screens the title can overflow/collide with the right-side link. Use a responsive font size that caps at 30px instead of a fixed value.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

color: rgb(59, 61, 65);
letter-spacing: 0.03em;
font-weight: 650;
Expand All @@ -51,7 +51,7 @@
}

.link {
font-size: 14px;
font-size: 30px;
Comment on lines 40 to +54

@codeant-ai codeant-ai Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Architect Review — HIGH

Increasing both the .title and .link font-size to 30px without any mobile-specific overrides means their combined intrinsic width exceeds the 95% navbar width on common mobile viewports, so the two items cannot reliably fit on a single row and will wrap or overflow.

Suggestion: Preserve the larger desktop typography but add mobile/tablet @media rules that reduce .title and .link font-size (or adjust the layout, e.g., stacking) so the navbar remains readable and non-overlapping on small screens.

@coderabbitai coderabbitai Bot Apr 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Scope drift: nav link size was increased with the title.

At Line 54, making .link also 30px is likely larger than needed for issue #114 (title visibility) and may hurt balance/readability, especially on mobile.

Suggested tweak 
 .link {
-    font-size: 30px;
+    font-size: 16px;
     color: rgb(59, 61, 65);
@@
 `@media` (max-width: 700px) {
+    .title { font-size: 24px; }
+    .link { font-size: 14px; }
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/navbar.module.css` at line 54, The .link rule was bumped to
30px along with the title causing scope drift; change the .link selector (and
any .nav-link variants) to a smaller, readable size (e.g., 14–16px) or remove
the explicit font-size so it inherits the base size, and keep the 30px setting
only on the .title/.logo selector; if larger link text is desired on wide
screens, move the increased size into a desktop media query so mobile remains
balanced.

@codeant-ai codeant-ai Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The GitHub link font size was also increased to 30px, which is unrelated to the stated title-only fix and causes the two nav items to compete for horizontal space, increasing the chance of wrapping/overlap in the fixed-width navbar. [logic error]

Severity Level: Major ⚠️ 
- ⚠️ Navbar GitHub link competes visually with site title.
- ⚠️ Fixed-height header crowded on small screens, reduced clarity.
Suggested change
font-size: 30px;
font-size: 14px;
Steps of Reproduction ✅ 
1. Run the app and load the root page `/` in a browser (any viewport). `RootLayout` in
`src/app/layout.tsx:42-60` renders `<Navbar/>` globally at `layout.tsx:58`.

2. In `Navbar` (`src/components/Navbar.tsx:5-19`), the GitHub anchor is rendered as `<Link
href="https://github.com/ajaynegi45/Old-School-Game"
className={styles.link}>Github</Link>` at `Navbar.tsx:13`, sharing the same horizontal
flex row as the title.

3. `src/components/navbar.module.css:39-47` sets `.title` to `font-size: 30px;`, and
`navbar.module.css:53-61` sets `.link` to `font-size: 30px;` as well, while `.navbar` at
`navbar.module.css:10-19` keeps a fixed `height: 4rem;` and `justify-content:
space-between;` with no mobile adjustments in the empty `@media (max-width: 700px)` block
at `navbar.module.css:71-74`.

4. With both the long title "OLD SCHOOL GAME" and the "Github" link set to 30px, they
compete for horizontal space inside the 95%-width, 4rem-high navbar. On narrower
viewports, this causes aggressive crowding and wrapping risk; even on desktop, it reduces
visual hierarchy and contradicts the PR description that only the title size should be
increased.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/components/navbar.module.css
**Line:** 54:54
**Comment:**
	*Logic Error: The GitHub link font size was also increased to `30px`, which is unrelated to the stated title-only fix and causes the two nav items to compete for horizontal space, increasing the chance of wrapping/overlap in the fixed-width navbar.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

color: rgb(59, 61, 65);
letter-spacing: -0.01em;
font-weight: 650;
Expand Down
Loading