feat: move language and theme controls to mobile bottom nav#95
feat: move language and theme controls to mobile bottom nav#95jazzz2502 wants to merge 2 commits into
Conversation
|
Someone is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thank you for your Pull Request! We're thrilled to have your contribution to FreshScan AI. Before we review, please make sure you have:
A maintainer will review your code as soon as possible! |
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Bottom navigation theme and language controls src/components/BottomNav.tsx |
BottomNav imports SunMoon/Languages, useTranslation, and toggleTheme. It initializes i18n, renders a theme toggle button that calls toggleTheme(), and a language select bound to i18n.language that calls i18n.changeLanguage() with EN, HI, BN options. |
Navbar responsive visibility src/components/Navbar.tsx |
Auth and theme controls wrapper class changes from flex to hidden md:flex, hiding these controls on small screens and showing them only on medium-and-up breakpoints. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant BottomNav
participant toggleTheme
participant i18n
User->>BottomNav: Click theme button
BottomNav->>toggleTheme: toggleTheme()
User->>BottomNav: Select language option
BottomNav->>i18n: changeLanguage(lang)
Estimated Code Review Effort
🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
🐰 A mobile nav so fine and bright,
Theme and tongue now take their flight,
To bottom safe on small-screen stage,
While top grows lean on every page!
Responsive, neat — a hopping delight!
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately reflects the main change: moving language and theme controls from the top navbar to the mobile bottom navigation. |
| Linked Issues check | ✅ Passed | The PR fully addresses Issue #92 by hiding language/theme controls on mobile using Tailwind's hidden md:flex and moving them to BottomNav. |
| Out of Scope Changes check | ✅ Passed | All changes are directly related to implementing responsive mobile navbar controls. No unrelated or out-of-scope modifications were made. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Navbar.tsx`:
- Line 96: The right-side controls container in Navbar (the div with className
"hidden md:flex items-center gap-2 sm:gap-4") hides auth actions on mobile;
instead, remove or change that class so auth controls remain visible and only
non-auth controls (theme/language provided by BottomNav) are hidden on mobile.
Concretely, keep the outer container (in the Navbar component) visible on all
breakpoints (e.g., remove "hidden md:flex") and apply "hidden md:flex" (or a
similar responsive class) to a narrower wrapper that only surrounds the
language/theme controls (the BottomNav or its containing element), ensuring
login/profile/logout buttons remain in the always-visible area.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d54d6e37-d070-4a42-bdc0-06a2ef9107f9
📒 Files selected for processing (2)
src/components/BottomNav.tsxsrc/components/Navbar.tsx
|
|
||
| {/* Auth Button & Theme Toggle */} | ||
| <div className="flex items-center gap-2 sm:gap-4"> | ||
| <div className="hidden md:flex items-center gap-2 sm:gap-4"> |
There was a problem hiding this comment.
Auth controls are unintentionally hidden on mobile.
On Line 96, hidden md:flex is applied to the entire right-side controls container, which also contains login/profile/logout actions (not just language/theme). Since BottomNav only adds theme/language controls, mobile users lose auth access.
Proposed fix
- {/* Auth Button & Theme Toggle */}
- <div className="hidden md:flex items-center gap-2 sm:gap-4">
+ {/* Desktop-only language + theme */}
+ <div className="hidden md:flex items-center gap-2 sm:gap-4">
<select
value={i18n.language}
onChange={(e) => i18n.changeLanguage(e.target.value)}
className="bg-surface-low border border-outline-variant/30 px-2 py-1 text-xs"
>
<option value="en">EN</option>
<option value="hi">HI</option>
<option value="bn">BN</option>
</select>
{/* Theme Toggle Button */}
<button
type="button"
onClick={toggleTheme}
className="font-[family-name:var(--font-mono)] text-[9px] sm:text-[10px] tracking-widest text-on-surface-variant hover:text-neon transition-colors duration-200 border border-outline-variant/30 px-2 py-1 sm:px-3"
>
THEME
</button>
+ </div>
+
+ {/* Auth controls remain available on mobile and desktop */}
+ <div className="flex items-center gap-2 sm:gap-4">
{loggedIn ? (
<div className="relative" ref={dropdownRef}>
...
</div>
) : (
<Link
to="/auth"
...
>
...
</Link>
)}
</div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Navbar.tsx` at line 96, The right-side controls container in
Navbar (the div with className "hidden md:flex items-center gap-2 sm:gap-4")
hides auth actions on mobile; instead, remove or change that class so auth
controls remain visible and only non-auth controls (theme/language provided by
BottomNav) are hidden on mobile. Concretely, keep the outer container (in the
Navbar component) visible on all breakpoints (e.g., remove "hidden md:flex") and
apply "hidden md:flex" (or a similar responsive class) to a narrower wrapper
that only surrounds the language/theme controls (the BottomNav or its containing
element), ensuring login/profile/logout buttons remain in the always-visible
area.
|
|
|
@jpdevhub can you please check this pr |
|
You removed the mode part ? |
You're right. I removed the MODE item while making space for the language and theme controls on mobile. I checked the codebase and /mode is still actively used. I'll restore the MODE navigation item and update the PR shortly. |
|
I have restored the MODE item in the mobile bottom navigation and pushed the update. Thanks for catching that. |
2 participants
Description
This PR resolves Issue #92 by improving the mobile navigation experience.
Changes Made
Testing
Closes #92
Summary by CodeRabbit
New Features
UI/UX