Added a responsive, functional language switcher button in navbar#327
Added a responsive, functional language switcher button in navbar#327reach2saksham wants to merge 5 commits intoAOSSIE-Org:new-designfrom
Conversation
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughReplaced the previous dynamic language switcher with an in-file LanguageDropdown inside Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navbar
participant Router
participant Document
User->>Navbar: Clicks language button
activate Navbar
Navbar->>Document: Attach outside-click listener
Navbar->>Navbar: Open menu, focus first item
User->>Navbar: ArrowDown / ArrowUp or selects language
Navbar->>Router: router.replace(newLocalePath, { locale })
Router-->>Navbar: Navigation complete
User->>Document: Click outside
Document-->>Navbar: Outside click detected
Navbar->>Navbar: Close menu, remove listener
deactivate Navbar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip Migrating from UI to YAML configuration.Use the |
📝 WalkthroughWalkthroughAdded a LanguageDropdown component to the Navbar enabling users to switch between English and Hindi. The component infers the current language from the URL pathname, handles dropdown state management, and rewrites the URL path when a language is selected. Changes
Sequence DiagramsequenceDiagram
actor User
participant Navbar
participant LanguageDropdown
participant Router
participant Browser
User->>LanguageDropdown: Click language button
LanguageDropdown->>LanguageDropdown: Toggle dropdown open state
LanguageDropdown->>Navbar: Render language options
User->>LanguageDropdown: Select new language (e.g., Hindi)
LanguageDropdown->>Router: useRouter().push(new path with 'hi' locale)
Router->>Browser: Rewrite URL pathname segment
Browser->>Navbar: Reload with new locale
LanguageDropdown->>LanguageDropdown: Infer current language from pathname
LanguageDropdown->>Navbar: Render with checkmark on selected language
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/layout/Navbar.tsx (3)
125-178: Consider extracting icons to a shared module.These icon components are well-implemented. If similar icons are needed elsewhere in the codebase, consider moving them to a shared
components/iconsmodule for reuse. This is optional and can be deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Navbar.tsx` around lines 125 - 178, Extract the three inline icon components (GlobeIcon, ChevronIcon, CheckIcon) into a shared icons module (e.g., components/icons), export them from that module, and replace the definitions in Navbar.tsx with imports of those named exports; ensure the exported components keep the same props and className behaviors (ChevronIcon({ open }: { open: boolean })) so existing usages and typings in Navbar remain unchanged.
69-86: Add proper ARIA roles for listbox semantics.The button declares
aria-haspopup="listbox", but the dropdown container lacksrole="listbox"and the options lackrole="option"witharia-selected. This incomplete pattern may confuse assistive technologies.Proposed fix for mobile dropdown (apply similar changes to desktop)
- <div className="absolute left-0 right-0 mt-2 rounded-2xl border border-default bg-surface shadow-lg z-50 overflow-hidden animate-in fade-in slide-in-from-top-2 duration-150" style={{ backgroundColor: 'var(--background, white)' }}> + <div role="listbox" aria-label="Select language" className="absolute left-0 right-0 mt-2 rounded-2xl border border-default bg-surface shadow-lg z-50 overflow-hidden animate-in fade-in slide-in-from-top-2 duration-150" style={{ backgroundColor: 'var(--background, white)' }}> {LANGUAGES.map((lang) => ( <button key={lang.code} onClick={() => switchLocale(lang.code)} + role="option" + aria-selected={lang.code === currentLocale} className={`flex items-center justify-between w-full px-4 py-3 text-sm font-medium transition-colors hover:bg-(--hover-background) ${🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Navbar.tsx` around lines 69 - 86, The dropdown lacks proper listbox semantics: when rendering the language menu (the block controlled by open that maps LANGUAGES) add role="listbox" and an accessible label (aria-label or aria-labelledby) to the container, and on each language button (the mapped element using key={lang.code}, onClick={() => switchLocale(lang.code)}) replace or augment with role="option" and include aria-selected={lang.code === currentLocale}; keep the visual CheckIcon but ensure the selected option reflects aria-selected, and ensure the trigger button that sets open retains aria-haspopup="listbox" and aria-expanded tied to open for full assistive support.
14-17: ImportLANGUAGESfrom the canonical source instead of duplicating.The codebase already has a
LANGUAGESconfiguration insrc/config/languages.ts. Duplicating this creates a maintenance risk—if locales are added or modified, both definitions must be kept in sync.Consider extending the canonical type if you need the
shortfield, or derive it from the existingcode:Proposed fix
-const LANGUAGES = [ - { code: "en", label: "English", short: "EN" }, - { code: "hi", label: "हिन्दी", short: "HI" }, -]; +import { LANGUAGES } from "@/config/languages";Then derive
shortinline where needed:<span>{current.code.toUpperCase()}</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Navbar.tsx` around lines 14 - 17, Replace the duplicated LANGUAGES array in Navbar.tsx with an import of the canonical LANGUAGES from src/config/languages (keep the constant name LANGUAGES to minimize changes) and remove the local definition; if you need a short label, either extend the canonical language type where it's defined or compute the short value in the component (e.g., derive from language.code by uppercasing when rendering, e.g., using current.code.toUpperCase()) so the single source of truth remains the config LANGUAGES and the Navbar only derives presentation values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/layout/Navbar.tsx`:
- Line 7: Navbar.tsx currently imports useRouter and usePathname from
next/navigation which bypasses the locale-aware wrappers; update the import to
use the locale-aware hooks exported from "@/i18n/navigation" instead (replace
the direct next/navigation import for useRouter and usePathname), ensuring
Navbar uses the same locale-aware routing as LanguageSwitcher.tsx so locale
switching works correctly.
- Around line 10-12: Remove the dead dynamic import for LanguageSwitcher: delete
the const LanguageSwitcher = dynamic(() => import("../ui/LanguageSwitcher"), {
ssr: false }) declaration (and any unused references to LanguageSwitcher) since
LanguageDropdown now handles language switching; ensure LanguageDropdown remains
and no other code references LanguageSwitcher before committing.
- Around line 41-51: The switchLocale function currently mutates the pathname
segments (LANGUAGES, switchLocale, pathname) which is fragile; replace that
logic with the locale-aware router.replace call used in LanguageSwitcher: call
the locale-aware useRouter's router.replace(pathname, { locale: code })
(preserving query/hash) instead of manually splicing segments, then call
setOpen(false); remove the segment manipulation code and ensure the file uses
the same locale-aware useRouter as LanguageSwitcher.
---
Nitpick comments:
In `@src/components/layout/Navbar.tsx`:
- Around line 125-178: Extract the three inline icon components (GlobeIcon,
ChevronIcon, CheckIcon) into a shared icons module (e.g., components/icons),
export them from that module, and replace the definitions in Navbar.tsx with
imports of those named exports; ensure the exported components keep the same
props and className behaviors (ChevronIcon({ open }: { open: boolean })) so
existing usages and typings in Navbar remain unchanged.
- Around line 69-86: The dropdown lacks proper listbox semantics: when rendering
the language menu (the block controlled by open that maps LANGUAGES) add
role="listbox" and an accessible label (aria-label or aria-labelledby) to the
container, and on each language button (the mapped element using
key={lang.code}, onClick={() => switchLocale(lang.code)}) replace or augment
with role="option" and include aria-selected={lang.code === currentLocale}; keep
the visual CheckIcon but ensure the selected option reflects aria-selected, and
ensure the trigger button that sets open retains aria-haspopup="listbox" and
aria-expanded tied to open for full assistive support.
- Around line 14-17: Replace the duplicated LANGUAGES array in Navbar.tsx with
an import of the canonical LANGUAGES from src/config/languages (keep the
constant name LANGUAGES to minimize changes) and remove the local definition; if
you need a short label, either extend the canonical language type where it's
defined or compute the short value in the component (e.g., derive from
language.code by uppercasing when rendering, e.g., using
current.code.toUpperCase()) so the single source of truth remains the config
LANGUAGES and the Navbar only derives presentation values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e14c647-8802-4af9-bc27-a3f5719d6136
📒 Files selected for processing (1)
src/components/layout/Navbar.tsx
src/components/layout/Navbar.tsx
Outdated
| const LanguageSwitcher = dynamic(() => import("../ui/LanguageSwitcher"), { | ||
| ssr: false, | ||
| }); |
There was a problem hiding this comment.
Remove unused LanguageSwitcher dynamic import.
LanguageSwitcher is imported but never used in this file since LanguageDropdown now handles language switching. This is dead code.
Proposed fix
-import dynamic from "next/dynamic";
-
-const LanguageSwitcher = dynamic(() => import("../ui/LanguageSwitcher"), {
- ssr: false,
-});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/layout/Navbar.tsx` around lines 10 - 12, Remove the dead
dynamic import for LanguageSwitcher: delete the const LanguageSwitcher =
dynamic(() => import("../ui/LanguageSwitcher"), { ssr: false }) declaration (and
any unused references to LanguageSwitcher) since LanguageDropdown now handles
language switching; ensure LanguageDropdown remains and no other code references
LanguageSwitcher before committing.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/layout/Navbar.tsx (1)
315-320: Expose the mobile menu state to assistive tech.The hamburger button still has only a static label. Adding
aria-expandedandaria-controlswould make the open/closed state of the mobile panel discoverable.Suggested change
- <button + <button className="p-1.5 text-primary hover:bg-(--hover-background) rounded-lg transition-colors" onClick={() => setIsMobileMenuOpen(!isMobileMenuOpen)} aria-label="Toggle menu" + aria-expanded={isMobileMenuOpen} + aria-controls="navbar-mobile-menu" > @@ - <div className="lg:hidden border-t border-default bg-(--background-secondary) px-4 sm:px-6 py-4 shadow-xl animate-in slide-in-from-top-5 rounded-b-3xl"> + <div id="navbar-mobile-menu" className="lg:hidden border-t border-default bg-(--background-secondary) px-4 sm:px-6 py-4 shadow-xl animate-in slide-in-from-top-5 rounded-b-3xl">Also applies to: 343-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Navbar.tsx` around lines 315 - 320, The mobile menu toggle button in Navbar.tsx should expose its state to assistive tech: update the button (where setIsMobileMenuOpen and isMobileMenuOpen are used) to include aria-expanded={isMobileMenuOpen} and aria-controls referencing the id of the mobile panel, and ensure the mobile panel element (the collapsible div rendered for mobile) has that matching id; apply the same attributes for the secondary button instance noted around lines 343-345 to keep both toggles accessible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/layout/Navbar.tsx`:
- Around line 42-47: The trigger buttons in Navbar use aria-haspopup="listbox"
but the popup panels are plain divs of buttons; change the interaction to a
menu-button pattern: update the triggers (the button using setOpen and
aria-expanded={open}) to use aria-haspopup="menu" and ensure aria-controls
references the popup id, then mark the popup container (the div rendered when
open) with role="menu" and each child button with role="menuitem" (and add
tabindex or aria-disabled as needed); keep using the existing setOpen and open
state logic but ensure keyboard behavior and focus management follow a menu
pattern (focus first item on open or manage Arrow keys) so screen readers
receive the correct menu semantics instead of listbox semantics.
- Around line 17-18: The locale switch currently uses usePathname() and calls
router.replace(pathname, { locale: code }) which drops query string and hash;
update the Locale change logic in Navbar.tsx to read search params via
useSearchParams(), build a query object from searchParams.entries(), capture the
hash via window.location.hash (guarded for SSR), then call router.replace with a
location object ({ pathname, query }) and the locale option, and finally
re-append the hash if present so anchors and filters are preserved; adjust
references around usePathname, useSearchParams, useLocale, pathname,
currentLocale and the router.replace call.
---
Nitpick comments:
In `@src/components/layout/Navbar.tsx`:
- Around line 315-320: The mobile menu toggle button in Navbar.tsx should expose
its state to assistive tech: update the button (where setIsMobileMenuOpen and
isMobileMenuOpen are used) to include aria-expanded={isMobileMenuOpen} and
aria-controls referencing the id of the mobile panel, and ensure the mobile
panel element (the collapsible div rendered for mobile) has that matching id;
apply the same attributes for the secondary button instance noted around lines
343-345 to keep both toggles accessible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93ea0ad4-7cb0-4098-8eec-740be80591d6
📒 Files selected for processing (1)
src/components/layout/Navbar.tsx
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
@M4dhav This PR is ready to be merged from my side. |
1 participant
Description
Adds an English / Hindi language switcher to the navbar.
Fixes #324
Type of change
components/layout/Navbar.tsxNew:
LanguageDropdowncomponentusePathname()and navigates to the equivalent route in the selected language viauseRouter().isMobileprop:EN/HI) with a popover menu anchored to the right.English/हिन्दी) with an inline dropdown.mousedownlistener attached to a wrappingref.backgroundColor: var(--background, white)inline style to prevent the parent navbar'sbackdrop-blurfrom bleeding through and making the panel transparent.New: inline SVG icon helpers
Three tiny, prop-typed SVG components added at the module level to keep JSX readable:
GlobeIcon,ChevronIcon,CheckIcon.Please delete options that are not relevant.
How Has This Been Tested?
/en, click theENglobe button in the navbar, selectहिन्दी. URL should change to/hi/.... Refresh, the dropdown should showHIas active with a checkmark.Notes
Englishis the default locale. If no recognised locale prefix is found in the pathname,switchLocaleprepends the selected code rather than replacing a non-existent segment.<LanguageSwitcher />import and usages have been left in place as comments so they can be removed or re-enabled independently.useRouterandusePathnameare already available vianext/navigation.Please include screenshots below if applicable:
Recording.2026-03-16.172458.mp4
Checklist:
Maintainer Checklist
Summary by CodeRabbit