-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/quicklab navbar #108
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.
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request introduces a new QuickLab feature to the application by adding a dedicated routing structure, navigation components, theming system, and styling. It includes responsive navbar components (desktop/mobile), a dark/light theme manager with localStorage persistence, and routes handling for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📋 PR Auto-Check📝 Files Changed✨ Code QualityPrettier Check: ✅ Passed Auto-generated on 2025-12-15T04:28:36.155Z |
📋 PR Auto-Check📝 Files Changed✨ Code QualityPrettier Check: ✅ Passed Auto-generated on 2025-12-15T04:30:13.901Z |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
client/src/components/quicklab/DesktopNavbar.jsx (1)
5-51: LGTM! Well-structured desktop navigation with a couple of optional improvements.The desktop navbar is cleanly implemented with:
- Proper responsive breakpoint (
hidden lg:block)- Fixed positioning with appropriate z-index
- Controlled search input properly bound to props
- Dark mode support via Tailwind classes
The Login/Register buttons are correctly implemented as placeholders for this scaffolding phase, as noted in the PR objectives.
Optional: Enhance accessibility for the search input.
Consider adding an
aria-labelto the search input for screen readers:<input type="text" value={searchQuery} onChange={(e) => setSearchQuery(e.target.value)} placeholder="Search labs..." + aria-label="Search laboratory tests" className="block w-full pl-10 pr-3 py-2 border border-slate-300 dark:border-slate-700 rounded-lg bg-slate-50 dark:bg-slate-900 text-slate-900 dark:text-slate-50 placeholder-slate-500 dark:placeholder-slate-400 focus:outline-none focus:ring-2 focus:ring-yellow-500 dark:focus:ring-yellow-400 focus:border-transparent transition-all" />client/src/components/quicklab/MobileNavbar.jsx (1)
5-69: LGTM! Well-designed mobile navigation with room for optional improvements.The mobile navbar effectively provides:
- Fixed top search bar with logo
- Fixed bottom navigation with clear iconography
- Proper mobile-only visibility (
lg:hidden)- Spacer to prevent content overlap with fixed elements
- Consistent search behavior with desktop version
The placeholder navigation buttons align with the PR's scaffolding objectives.
Optional: Enhance accessibility.
Consider these improvements for better accessibility:
- Add
aria-labelto the search input (line 23-29):<input type="text" value={searchQuery} onChange={(e) => setSearchQuery(e.target.value)} placeholder="Search labs..." + aria-label="Search laboratory tests" className="..." />
- Add
aria-labelattributes to bottom navigation buttons to clarify their purpose for screen readers:-<button className="flex flex-col items-center justify-center w-full h-full space-y-1 text-slate-500 dark:text-slate-400 hover:text-yellow-600 dark:hover:text-yellow-400 transition-colors active:scale-95"> +<button aria-label="Go to home page" className="flex flex-col items-center justify-center w-full h-full space-y-1 text-slate-500 dark:text-slate-400 hover:text-yellow-600 dark:hover:text-yellow-400 transition-colors active:scale-95">client/src/quicklab.css (1)
52-135: Consider refactoring component classes to use theme tokens.The component classes currently hardcode RGB color values rather than referencing the CSS custom properties defined in the
@themeblock (lines 6-36). This creates maintenance overhead—if theme colors change, you'll need to update values in multiple locations.Consider refactoring to use the defined tokens. For example:
.btn-quicklab-primary { - background-color: rgb(234 179 8); /* yellow-500 */ + background-color: var(--color-lab-yellow-500); - color: rgb(15 23 42); /* black-900 */ + color: var(--color-lab-black-900); padding: 0.75rem 1.5rem; border-radius: 0.5rem; transition: background-color 0.2s; font-weight: 600; }This approach:
- Reduces duplication
- Makes theme updates easier
- Provides a single source of truth for color values
- Aligns with the design system architecture established in the
@themeblockclient/src/quicklab-theme.js (1)
3-29: Consider extracting the theme key constant.The localStorage key
'quicklab-theme'is hardcoded in three functions (lines 65, 72, 78). Consider extracting it as a constant to maintain a single source of truth.+const THEME_STORAGE_KEY = 'quicklab-theme'; + export const colors = { primary: '#eab308', // ... rest of colors }; // Then use it in the functions: export const initTheme = () => { - const theme = localStorage.getItem('quicklab-theme') || 'light'; + const theme = localStorage.getItem(THEME_STORAGE_KEY) || 'light'; // ... };This makes future refactoring easier (e.g., if you need to change the key or add a prefix).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
client/src/App.jsx(2 hunks)client/src/components/quicklab/DesktopNavbar.jsx(1 hunks)client/src/components/quicklab/MobileNavbar.jsx(1 hunks)client/src/components/quicklab/Navbar.jsx(1 hunks)client/src/quicklab-theme.js(1 hunks)client/src/quicklab.css(1 hunks)client/src/routes/QuickLabRoutes.jsx(1 hunks)client/src/shared-base.css(1 hunks)server/Controllers/QuickLab/labReportController.js(0 hunks)
💤 Files with no reviewable changes (1)
- server/Controllers/QuickLab/labReportController.js
🧰 Additional context used
🧬 Code graph analysis (5)
client/src/components/quicklab/DesktopNavbar.jsx (2)
client/src/components/quicklab/Navbar.jsx (1)
searchQuery(7-7)client/src/components/ui/DarkModeToggle.jsx (1)
DarkModeToggle(3-21)
client/src/routes/QuickLabRoutes.jsx (1)
client/src/components/ui/NotFoundPage.jsx (1)
NotFoundPage(5-64)
client/src/components/quicklab/Navbar.jsx (2)
client/src/components/quicklab/DesktopNavbar.jsx (1)
DesktopNavbar(5-51)client/src/components/quicklab/MobileNavbar.jsx (1)
MobileNavbar(5-69)
client/src/components/quicklab/MobileNavbar.jsx (2)
client/src/components/quicklab/Navbar.jsx (1)
searchQuery(7-7)client/src/components/ui/DarkModeToggle.jsx (1)
DarkModeToggle(3-21)
client/src/App.jsx (10)
client/src/components/Patient/PatientMobileNavigation.jsx (1)
location(19-19)client/src/components/Doctors/DoctorMobileBottomBar.jsx (1)
location(5-5)client/src/components/Doctors/DoctorNavbar.jsx (1)
location(9-9)client/src/routes/guards/AdminSetupGuard.jsx (1)
location(12-12)client/src/routes/guards/PatientPreventGuard.jsx (1)
location(13-13)client/src/routes/guards/AdminPreventGuard.jsx (1)
location(12-12)client/src/routes/guards/PatientSetupGuard.jsx (1)
location(13-13)client/src/routes/guards/protectedRoutes.jsx (1)
location(9-9)client/src/routes/QuickmedRoutes.jsx (1)
QuickmedRoutes(6-17)client/src/routes/QuickLabRoutes.jsx (1)
QuickLabRoutes(4-13)
🔇 Additional comments (5)
client/src/shared-base.css (1)
1-21: LGTM! Solid foundation for the design system.The global CSS baseline is well-structured with:
- Consistent custom properties for transitions and border radii
- Standard universal reset for consistent cross-browser rendering
- Comprehensive system font stack
- Smooth theme transitions using the defined variables
This provides a clean foundation for the QuickLab UI components.
client/src/routes/QuickLabRoutes.jsx (1)
4-13: LGTM! Clean routing scaffolding for QuickLab.The component follows the established pattern from QuickmedRoutes (similar structure with Navbar + Routes). The catch-all route correctly handles unmatched paths by showing NotFoundPage, which aligns with the PR objective of providing routing support for ongoing QuickLab development.
client/src/App.jsx (2)
15-15: LGTM! Import added correctly.The QuickLabRoutes import follows the same pattern as QuickmedRoutes and integrates cleanly with the existing routing structure.
47-53: LGTM! QuickLab routing integrated properly.The routing logic correctly:
- Checks for
/quick-labpaths usingstartsWith()for nested route support- Evaluates QuickLab routes after QuickMed but before authentication-based routing
- Follows the established pattern from QuickmedRoutes
The placement ensures public QuickLab pages are accessible without authentication.
client/src/components/quicklab/Navbar.jsx (1)
6-18: LGTM! Excellent composition pattern for responsive navigation.The Navbar wrapper effectively:
- Centralizes search state management
- Shares state consistently between desktop and mobile variants
- Uses fragments to avoid unnecessary DOM wrappers
- Follows React best practices for component composition
This architecture makes it easy to maintain synchronized search behavior across breakpoints.
| export const initTheme = () => { | ||
| const theme = localStorage.getItem('quicklab-theme') || 'light'; | ||
| document.documentElement.classList.toggle('dark', theme === 'dark'); | ||
| return theme; | ||
| }; | ||
|
|
||
| export const toggleTheme = () => { | ||
| const isDark = document.documentElement.classList.toggle('dark'); | ||
| localStorage.setItem('quicklab-theme', isDark ? 'dark' : 'light'); | ||
| return isDark ? 'dark' : 'light'; | ||
| }; | ||
|
|
||
| export const setTheme = (theme) => { | ||
| document.documentElement.classList.toggle('dark', theme === 'dark'); | ||
| localStorage.setItem('quicklab-theme', theme); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add SSR compatibility checks for document access.
The theme functions directly access document.documentElement and localStorage without checking if they exist. This will cause runtime errors in SSR environments (like Next.js or during server-side rendering).
Apply this diff to add environment checks:
export const initTheme = () => {
+ if (typeof window === 'undefined') return 'light';
const theme = localStorage.getItem('quicklab-theme') || 'light';
document.documentElement.classList.toggle('dark', theme === 'dark');
return theme;
};
export const toggleTheme = () => {
+ if (typeof window === 'undefined') return 'light';
const isDark = document.documentElement.classList.toggle('dark');
localStorage.setItem('quicklab-theme', isDark ? 'dark' : 'light');
return isDark ? 'dark' : 'light';
};
export const setTheme = (theme) => {
+ if (typeof window === 'undefined') return;
document.documentElement.classList.toggle('dark', theme === 'dark');
localStorage.setItem('quicklab-theme', theme);
};This ensures the code won't break if the application later adopts SSR or is executed in a Node.js environment during build time.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const initTheme = () => { | |
| const theme = localStorage.getItem('quicklab-theme') || 'light'; | |
| document.documentElement.classList.toggle('dark', theme === 'dark'); | |
| return theme; | |
| }; | |
| export const toggleTheme = () => { | |
| const isDark = document.documentElement.classList.toggle('dark'); | |
| localStorage.setItem('quicklab-theme', isDark ? 'dark' : 'light'); | |
| return isDark ? 'dark' : 'light'; | |
| }; | |
| export const setTheme = (theme) => { | |
| document.documentElement.classList.toggle('dark', theme === 'dark'); | |
| localStorage.setItem('quicklab-theme', theme); | |
| }; | |
| export const initTheme = () => { | |
| if (typeof window === 'undefined') return 'light'; | |
| const theme = localStorage.getItem('quicklab-theme') || 'light'; | |
| document.documentElement.classList.toggle('dark', theme === 'dark'); | |
| return theme; | |
| }; | |
| export const toggleTheme = () => { | |
| if (typeof window === 'undefined') return 'light'; | |
| const isDark = document.documentElement.classList.toggle('dark'); | |
| localStorage.setItem('quicklab-theme', isDark ? 'dark' : 'light'); | |
| return isDark ? 'dark' : 'light'; | |
| }; | |
| export const setTheme = (theme) => { | |
| if (typeof window === 'undefined') return; | |
| document.documentElement.classList.toggle('dark', theme === 'dark'); | |
| localStorage.setItem('quicklab-theme', theme); | |
| }; |
🤖 Prompt for AI Agents
In client/src/quicklab-theme.js around lines 64 to 79, the functions access
document.documentElement and localStorage directly which breaks in SSR/node
environments; update initTheme, toggleTheme, and setTheme to first detect a
browser environment (e.g. if (typeof window === 'undefined' || typeof document
=== 'undefined')) and return sensible defaults or no-ops when absent, and guard
localStorage access similarly (or wrap in try/catch) so they only read/write
storage when available; ensure initTheme returns the chosen theme string even
when running on the server and toggleTheme/setTheme become no-ops or persist to
storage only in the browser.
| @import 'tailwindcss'; | ||
|
|
||
| /* Enable dark mode with class selector */ | ||
| @custom-variant dark (&:where(.dark, .dark *)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Tailwind CSS version in package.json
cat package.json | jq '.dependencies.tailwindcss, .devDependencies.tailwindcss' 2>/dev/null || cat package.json | grep -A 2 -B 2 'tailwindcss'
# Also check for tailwind.config file
echo "=== Tailwind Config Files ==="
fd -t f "tailwind.config" -x cat {} 2>/dev/null || echo "No tailwind.config file found"Repository: genzz-dev/QuickClinic
Length of output: 228
🏁 Script executed:
#!/bin/bash
# Check for lock files
echo "=== Lock Files ==="
if [ -f "package-lock.json" ]; then
echo "Found package-lock.json"
grep -A 2 '"tailwindcss"' package-lock.json | head -20
elif [ -f "yarn.lock" ]; then
echo "Found yarn.lock"
grep -A 3 "tailwindcss" yarn.lock | head -20
elif [ -f "pnpm-lock.yaml" ]; then
echo "Found pnpm-lock.yaml"
grep -A 3 "tailwindcss" pnpm-lock.yaml | head -20
else
echo "No lock file found"
fi
# Check full package.json structure
echo -e "\n=== Full package.json ==="
cat package.jsonRepository: genzz-dev/QuickClinic
Length of output: 734
🏁 Script executed:
#!/bin/bash
# Check if tailwindcss appears in lock file at all
echo "=== Checking for tailwindcss in lock file ==="
if grep -q '"tailwindcss"' package-lock.json; then
echo "Found tailwindcss in package-lock.json:"
grep -A 10 '"tailwindcss"' package-lock.json | head -30
else
echo "tailwindcss NOT found in package-lock.json"
fi
# Check framer-motion dependencies to see if it depends on tailwindcss
echo -e "\n=== Checking framer-motion dependencies ==="
grep -A 30 '"framer-motion"' package-lock.json | grep -E '(version|dependencies)' | head -20Repository: genzz-dev/QuickClinic
Length of output: 276
🌐 Web query:
@custom-variant tailwindcss v4 syntax documentation
💡 Result:
Short answer — Tailwind v4 introduced the @custom-variant/@variant directive to declare custom variants in CSS. Example forms from the official docs:
-
Block form:
@custom-variant theme-midnight {
&:where([data-theme="midnight"] *) {
@slot;
}
}
(uses class like theme-midnight:bg-black). [1] -
Shorthand single-rule form:
@custom-variant theme-midnight (&:where([data-theme="midnight"] *)); [1] -
v4/@variant examples (alternative syntax shown in v4 beta docs):
@variant pointer-coarse (@media (pointer: coarse));
@variant theme-midnight (&:where([data-theme="midnight"] *)); [2]
Notes: several v4 users reported large CSS/performance changes when migrating custom variants (see related GitHub discussion). [3]
Sources:
[1] Tailwind CSS — Adding custom styles / Adding custom variants (official docs).
[2] Tailwind CSS v4.0 Beta docs — Adding custom variants.
[3] tailwindlabs/tailwindcss GitHub issue “(V4) Performance issue (@custom-variant)”.
Tailwind CSS dependency is missing and v4 is required.
The project does not have tailwindcss listed in package.json. The syntax used—@import 'tailwindcss' and @custom-variant—requires Tailwind v4 or higher. Add tailwindcss v4+ to your dependencies:
"dependencies": {
"tailwindcss": "^4.0.0"
}🤖 Prompt for AI Agents
In client/src/quicklab.css around lines 1 to 4, the stylesheet uses Tailwind v4+
syntax (@import 'tailwindcss' and @custom-variant) but the project lacks the
tailwindcss dependency; update package.json to add "tailwindcss": "^4.0.0" (or
later), run your package manager install to fetch it, and ensure the project's
build/postcss configuration references Tailwind (e.g., include tailwind as a
PostCSS plugin or import path) so the @import and custom variant are resolved at
build time.
Project
Change Type
Stack
Page Type
Route/API Endpoint Status
What Changed
Route/API Affected
/quick-lab
Description
added navbar and routes for quicklab
currently only added the navbar in routes
this will help in development of new page
Screenshots (If Applicable)
Mobile View
Desktop View
Code Quality
npx prettier --check .)Related Issues
Closes #NA
Summary by CodeRabbit
New Features
/quick-labpathStyling
✏️ Tip: You can customize this high-level summary in your review settings.