Skip to content

Commit 1a5df9c

Browse files
taearlsclaude
andcommitted
fix(nav): resolve NavigationToggle visibility CSS specificity conflict
- Add .closed CSS class to NavigationBar.module.css to override inline-flex - Update NavigationBar.tsx to use styles.closed instead of Tailwind hidden - Add comprehensive test suite for navigation toggle functionality (8 tests) - Update ROADMAP.md with issue #97 completion and technical lesson Closes #97 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 6fb8da3 commit 1a5df9c

File tree

4 files changed

+325
-1
lines changed

4 files changed

+325
-1
lines changed

ROADMAP.md

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,80 @@ _None - All prerequisites for #43 are complete. Ready to implement._
519519

520520
## Changelog
521521

522+
### 2025-12-23 - Issue #97 Completed: NavigationToggle Visibility Fix
523+
524+
- **Completed**: #97 - NavigationToggle does not show/hide navigation bar
525+
- **Priority**: 🔴 BUG (Navigation toggle button animated but navigation never hid)
526+
- **Status**: Completed Dec 23, 2025
527+
- **Effort**: ~1 hour (including comprehensive test suite)
528+
- **Impact**: Fixed critical navigation UX bug on mobile viewports
529+
530+
**Root Cause Analysis**:
531+
532+
CSS specificity conflict in `NavigationBar.tsx`. The CSS Module class `navigation-list-container` defined `display: inline-flex` with higher specificity than Tailwind's `hidden` utility (`display: none`). The navigation remained visible even when state was `CLOSED`.
533+
534+
**Implementation Details**:
535+
536+
1. **Added CSS Module `.closed` class** (`NavigationBar.module.css:39-41`)
537+
- Created `.navigation-list-container.closed` rule with `display: none`
538+
- Uses CSS Module specificity to properly override `inline-flex`
539+
- Added comment documenting the specificity requirement
540+
541+
2. **Updated NavigationBar component** (`NavigationBar.tsx:89-92`)
542+
- Changed from Tailwind's `"hidden"` to CSS Module's `styles.closed`
543+
- Maintains consistent specificity within CSS Module scope
544+
545+
3. **Added comprehensive test suite** (`tests/component/NavigationBar.spec.tsx`)
546+
- 8 test cases covering toggle functionality
547+
- Tests CSS Module class application (`_closed_` pattern matching)
548+
- Verifies aria-label accessibility updates
549+
- State-agnostic tests that work regardless of initial viewport state
550+
551+
**Changes**:
552+
553+
```css
554+
/* NavigationBar.module.css - Added */
555+
.navigation-list-container.closed {
556+
display: none;
557+
}
558+
```
559+
560+
```tsx
561+
// NavigationBar.tsx - Before
562+
className={mergeClasses(
563+
styles["navigation-list-container"],
564+
isNavigationOpen.value === NAVIGATION_STATE.CLOSED && "hidden",
565+
)}
566+
567+
// NavigationBar.tsx - After
568+
className={mergeClasses(
569+
styles["navigation-list-container"],
570+
isNavigationOpen.value === NAVIGATION_STATE.CLOSED && styles.closed,
571+
)}
572+
```
573+
574+
**Files Modified**:
575+
576+
- `src/components/navigation/NavigationBar/NavigationBar.module.css` - Added `.closed` class
577+
- `src/components/navigation/NavigationBar/NavigationBar.tsx` - Use CSS Module class
578+
579+
**Files Created**:
580+
581+
- `tests/component/NavigationBar.spec.tsx` - Comprehensive test suite (8 tests)
582+
583+
**Testing**:
584+
585+
- ✅ All 190 unit tests passing (8 new NavigationBar tests)
586+
- ✅ Production build successful
587+
- ✅ Navigation toggle now properly shows/hides navigation list
588+
- ✅ CSS Module class verified with pattern matching
589+
590+
**Technical Lesson**:
591+
592+
When using CSS Modules with Tailwind, be aware of specificity conflicts. CSS Module classes have higher specificity than Tailwind utilities. Use CSS Module classes for state-based visibility changes when the base styles are defined in CSS Modules.
593+
594+
---
595+
522596
### 2025-12-23 - Issue #65 Completed: Font Size Readability Enhancement
523597

524598
- **Completed**: #65 - Increase Font Size for Desktop/Tablet Readability

src/components/navigation/NavigationBar/NavigationBar.module.css

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@
3535
@apply sm:h-[76px] sm:w-full sm:flex-row;
3636
}
3737

38+
/* Hide navigation when closed on mobile - uses CSS Module specificity to override inline-flex */
39+
.navigation-list-container.closed {
40+
display: none;
41+
}
42+
3843
/* WCAG 2.5.5 - Ensure navigation links meet 44×44px minimum touch target */
3944
.navigation-list-container a {
4045
min-height: var(--touch-target-min);

src/components/navigation/NavigationBar/NavigationBar.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export default function NavigationBar({ links }: NavigationBarProps) {
8888
role="menu"
8989
className={mergeClasses(
9090
styles["navigation-list-container"],
91-
isNavigationOpen.value === NAVIGATION_STATE.CLOSED && "hidden",
91+
isNavigationOpen.value === NAVIGATION_STATE.CLOSED && styles.closed,
9292
)}
9393
>
9494
<FlexContainer
Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
import "@testing-library/jest-dom/vitest";
2+
3+
import { render, screen, fireEvent } from "@testing-library/react";
4+
import { MemoryRouter } from "react-router";
5+
6+
import NavigationBar from "@/components/navigation/NavigationBar/NavigationBar.tsx";
7+
import ThemeContext from "@/state/contexts/ThemeContext.tsx";
8+
import type { RouteDataItem } from "@/constants/navigationData.tsx";
9+
10+
// Mock navigation links for testing
11+
const mockLinks: Array<RouteDataItem> = [
12+
{
13+
ariaLabel: "Visit Home Page",
14+
href: "/",
15+
name: "Home",
16+
},
17+
{
18+
ariaLabel: "Visit Code Page",
19+
href: "/code",
20+
name: "Code",
21+
},
22+
{
23+
ariaLabel: "Visit Contact Page",
24+
href: "/contact",
25+
name: "Contact",
26+
},
27+
];
28+
29+
const renderNavigationBar = () => {
30+
return render(
31+
<MemoryRouter>
32+
<ThemeContext.Provider>
33+
<NavigationBar links={mockLinks} />
34+
</ThemeContext.Provider>
35+
</MemoryRouter>,
36+
);
37+
};
38+
39+
describe("<NavigationBar />", () => {
40+
describe("Navigation Toggle Functionality", () => {
41+
it("should toggle navigation visibility when toggle button is clicked", () => {
42+
renderNavigationBar();
43+
44+
const navigationList = screen.getByRole("menu");
45+
46+
// Get the initial state of the toggle button
47+
// The button aria-label tells us the current action (Open = nav is closed, Close = nav is open)
48+
const initialButton =
49+
screen.queryByRole("button", { name: /Open Navigation/i }) ||
50+
screen.queryByRole("button", { name: /Close Navigation/i });
51+
52+
expect(initialButton).toBeInTheDocument();
53+
54+
const isInitiallyOpen = initialButton?.getAttribute("aria-label") === "Close Navigation";
55+
56+
if (isInitiallyOpen) {
57+
// Navigation starts open - verify no closed class
58+
expect(navigationList.className).not.toMatch(/closed/);
59+
60+
// Click to close
61+
fireEvent.click(initialButton!);
62+
63+
// Should now have closed class
64+
expect(navigationList.className).toMatch(/closed/);
65+
66+
// Button should now say "Open Navigation"
67+
expect(
68+
screen.getByRole("button", { name: /Open Navigation/i }),
69+
).toBeInTheDocument();
70+
71+
// Click to open again
72+
fireEvent.click(
73+
screen.getByRole("button", { name: /Open Navigation/i }),
74+
);
75+
76+
// Should no longer have closed class
77+
expect(navigationList.className).not.toMatch(/closed/);
78+
} else {
79+
// Navigation starts closed - verify has closed class
80+
expect(navigationList.className).toMatch(/closed/);
81+
82+
// Click to open
83+
fireEvent.click(initialButton!);
84+
85+
// Should no longer have closed class
86+
expect(navigationList.className).not.toMatch(/closed/);
87+
88+
// Button should now say "Close Navigation"
89+
expect(
90+
screen.getByRole("button", { name: /Close Navigation/i }),
91+
).toBeInTheDocument();
92+
93+
// Click to close again
94+
fireEvent.click(
95+
screen.getByRole("button", { name: /Close Navigation/i }),
96+
);
97+
98+
// Should have closed class again
99+
expect(navigationList.className).toMatch(/closed/);
100+
}
101+
});
102+
103+
it("should apply closed CSS class when navigation is closed", () => {
104+
renderNavigationBar();
105+
106+
const navigationList = screen.getByRole("menu");
107+
108+
// Find the toggle button and get current state
109+
const closeButton = screen.queryByRole("button", {
110+
name: /Close Navigation/i,
111+
});
112+
113+
if (closeButton) {
114+
// If we have a close button, navigation is open - click to close
115+
fireEvent.click(closeButton);
116+
}
117+
118+
// Now navigation should be closed
119+
// The closed class from CSS Module should be applied
120+
// CSS Module mangles the class name, so we check for pattern containing "closed"
121+
expect(navigationList.className).toMatch(/closed/);
122+
});
123+
124+
it("should remove closed CSS class when navigation is opened", () => {
125+
renderNavigationBar();
126+
127+
const navigationList = screen.getByRole("menu");
128+
129+
// First ensure navigation is closed
130+
const closeButton = screen.queryByRole("button", {
131+
name: /Close Navigation/i,
132+
});
133+
134+
if (closeButton) {
135+
fireEvent.click(closeButton);
136+
}
137+
138+
// Verify it's closed
139+
expect(navigationList.className).toMatch(/closed/);
140+
141+
// Now open it
142+
const openButton = screen.getByRole("button", {
143+
name: /Open Navigation/i,
144+
});
145+
fireEvent.click(openButton);
146+
147+
// The closed class should be removed
148+
expect(navigationList.className).not.toMatch(/closed/);
149+
});
150+
151+
it("should toggle between open and closed states on multiple clicks", () => {
152+
renderNavigationBar();
153+
154+
const navigationList = screen.getByRole("menu");
155+
156+
// Perform multiple toggles and verify state changes correctly each time
157+
for (let i = 0; i < 4; i++) {
158+
const currentButton =
159+
screen.queryByRole("button", { name: /Open Navigation/i }) ||
160+
screen.queryByRole("button", { name: /Close Navigation/i });
161+
162+
expect(currentButton).toBeInTheDocument();
163+
164+
const wasOpen =
165+
currentButton?.getAttribute("aria-label") === "Close Navigation";
166+
167+
fireEvent.click(currentButton!);
168+
169+
// State should have toggled
170+
if (wasOpen) {
171+
expect(navigationList.className).toMatch(/closed/);
172+
} else {
173+
expect(navigationList.className).not.toMatch(/closed/);
174+
}
175+
}
176+
});
177+
178+
it("should render all visible navigation links", () => {
179+
renderNavigationBar();
180+
181+
// Check that all non-hidden links are rendered
182+
expect(screen.getByText("Home")).toBeInTheDocument();
183+
expect(screen.getByText("Code")).toBeInTheDocument();
184+
expect(screen.getByText("Contact")).toBeInTheDocument();
185+
});
186+
187+
it("should have accessible toggle button with aria-label", () => {
188+
renderNavigationBar();
189+
190+
// Should have a toggle button with appropriate aria-label
191+
const toggleButton =
192+
screen.queryByRole("button", { name: /Open Navigation/i }) ||
193+
screen.queryByRole("button", { name: /Close Navigation/i });
194+
195+
expect(toggleButton).toBeInTheDocument();
196+
expect(toggleButton).toHaveAttribute("aria-label");
197+
});
198+
199+
it("should update aria-label when toggle state changes", () => {
200+
renderNavigationBar();
201+
202+
// Get initial button
203+
const initialButton =
204+
screen.queryByRole("button", { name: /Open Navigation/i }) ||
205+
screen.queryByRole("button", { name: /Close Navigation/i });
206+
207+
const initialLabel = initialButton?.getAttribute("aria-label");
208+
209+
// Click to toggle
210+
fireEvent.click(initialButton!);
211+
212+
// Get new button
213+
const newButton =
214+
screen.queryByRole("button", { name: /Open Navigation/i }) ||
215+
screen.queryByRole("button", { name: /Close Navigation/i });
216+
217+
const newLabel = newButton?.getAttribute("aria-label");
218+
219+
// Labels should be different after toggle
220+
expect(newLabel).not.toBe(initialLabel);
221+
});
222+
});
223+
224+
describe("CSS Module Integration", () => {
225+
it("should use CSS Module closed class for hiding navigation (not Tailwind hidden)", () => {
226+
renderNavigationBar();
227+
228+
const navigationList = screen.getByRole("menu");
229+
230+
// Ensure navigation is closed
231+
const closeButton = screen.queryByRole("button", {
232+
name: /Close Navigation/i,
233+
});
234+
if (closeButton) {
235+
fireEvent.click(closeButton);
236+
}
237+
238+
// The class should contain "closed" (CSS Module pattern)
239+
// It should NOT just be "hidden" (Tailwind utility - which has specificity issues)
240+
expect(navigationList.className).toMatch(/closed/);
241+
// Verify it's a CSS Module class (contains hash suffix like _closed_abc123)
242+
expect(navigationList.className).toMatch(/_closed_[a-z0-9]+/i);
243+
});
244+
});
245+
});

0 commit comments

Comments
 (0)