feat(navbar): add mobile navigation menu and translations#230
Conversation
Dasa122
added
feature
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds localized navigation strings (en, hu), imports menu icons, introduces mobileMenuOpen and canSeeAdminUi, renders a mobile-only hamburger toggle with i18n aria labels, shows a collapsible mobile nav panel with mapped route buttons, and refactors desktop links to use NAV_ITEMS. ChangesMobile Navigation Feature
sequenceDiagram
participant User
participant Navbar
participant Router
User->>Navbar: tap hamburger toggle (Menu/X)
Navbar->>Navbar: set mobileMenuOpen true/false
User->>Navbar: tap navigation button (Schedule/Substitutions/Admin)
Navbar->>Router: navigate to target route
Router->>Navbar: navigation complete (Navbar closes mobile menu)
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: 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: 2
🤖 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 `@apps/iris/src/components/navbar.tsx`:
- Around line 215-264: The mobile menu buttons remain focusable when collapsed
because only the container height is toggled; update the JSX so the interactive
contents are only rendered or fully inert when closed: wrap the inner menu block
(the div with className "overflow-hidden" and its child Buttons) in a
conditional that requires mobileMenuOpen (e.g., render that inner div only if
mobileMenuOpen is true) or alternatively add attributes to the container when
closed (aria-hidden="true", inert or tabIndex={-1} on each Button and disabled
on Buttons) to prevent focus; target the mobileMenuOpen flag, the container div
around the Buttons, and the Button components (and keep navigate and
setMobileMenuOpen behavior unchanged).
- Around line 66-80: The mobile menu Button always uses a single aria-label;
change it to be state-aware by using the mobileMenuOpen boolean to pick between
two translated labels (e.g., t('navigation.mobileMenuOpen') vs
t('navigation.mobileMenuClose')) in the Navbar component (the Button that
toggles mobileMenuOpen), and add corresponding keys and translations to both
locale trees under apps/iris/public/locales/en and /hu so all user-facing text
remains in t(...).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 803a1241-9589-4b06-9af6-540e34b63112
📒 Files selected for processing (3)
apps/iris/public/locales/en/translation.jsonapps/iris/public/locales/hu/translation.jsonapps/iris/src/components/navbar.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/navbar.tsx
🔇 Additional comments (2)
apps/iris/public/locales/en/translation.json (1)
189-192: LGTM!apps/iris/public/locales/hu/translation.json (1)
189-192: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/iris/src/components/navbar.tsx (1)
66-84:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
aria-expanded(andaria-controls) to the disclosure toggle.The state-aware
aria-labelis good. This button controls a collapsible region, so it should also exposearia-expandedso assistive tech announces open/closed state. Optionally wirearia-controlsto anidon the panel at Line 220.Suggested fix
<Button aria-label={ mobileMenuOpen ? t('navigation.mobileMenuClose') : t('navigation.mobileMenuOpen') } + aria-controls="mobile-nav-panel" + aria-expanded={mobileMenuOpen} className="flex md:hidden" onClick={() => setMobileMenuOpen((prev) => !prev)} size="icon" variant="ghost" >And add the matching id on the panel container:
- {data && showLinks && ( - <div + {data && showLinks && ( + <div + id="mobile-nav-panel" className={[🤖 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 `@apps/iris/src/components/navbar.tsx` around lines 66 - 84, Add ARIA attributes to the mobile menu toggle Button: set aria-expanded to the boolean mobileMenuOpen and add aria-controls pointing to the id of the collapsible panel; update the panel container (the element rendered for the mobile menu) to include that same id so the Button (using setMobileMenuOpen and mobileMenuOpen state) correctly references the controlled region for assistive tech.
🤖 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 `@apps/iris/src/components/navbar.tsx`:
- Around line 229-267: The mobile menu duplicates the same nav items rendered by
NavLinks; extract a single source of truth (e.g., PRIMARY_NAV_ITEMS) describing
{to, icon, labelKey, adminOnly} and use it in both NavLinks and the
mobileMenuOpen block, mapping over that list and filtering items by
canSeeAdminUi for adminOnly entries; remove the duplicated Button definitions in
the mobile panel and vary only presentation (wrapper className/size/variant)
when rendering the shared items so future route/icon/label changes happen in one
place.
---
Outside diff comments:
In `@apps/iris/src/components/navbar.tsx`:
- Around line 66-84: Add ARIA attributes to the mobile menu toggle Button: set
aria-expanded to the boolean mobileMenuOpen and add aria-controls pointing to
the id of the collapsible panel; update the panel container (the element
rendered for the mobile menu) to include that same id so the Button (using
setMobileMenuOpen and mobileMenuOpen state) correctly references the controlled
region for assistive tech.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c6477cb7-34ae-4225-b025-fe6992f6be59
📒 Files selected for processing (1)
apps/iris/src/components/navbar.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/navbar.tsx
🔇 Additional comments (1)
apps/iris/src/components/navbar.tsx (1)
1-58: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/iris/src/components/navbar.tsx`:
- Line 45: NavItem.to is typed as string but should be narrowed to the router's
route-path type to improve safety; update the NavItem interface (symbol:
NavItem.to) to use the TanStack Router/Link prop type (e.g., LinkProps['to'] or
the library's ValidRoutePath) and ensure usages like navigate({ to: item.to })
and any <Link ... to={item.to}> remain type-compatible with
useNavigate()/navigate; replace the plain string type with the appropriate typed
route-path alias so invalid route strings are caught by the compiler.
- Around line 44-49: The NavItem type is incorrect: replace the
React.ElementType reference with the imported ElementType (or import React
namespace) and narrow NavItem.to from string to the router's specific 'to' union
(use the generated type like keyof FileRoutesByTo or the router's To type) so
usages like navigate({ to: item.to }) type-check; update the NavItem declaration
(and any imports) accordingly, referencing NavItem, ElementType (or React
import), FileRoutesByTo, and the navigate({ to: item.to }) call to locate the
changes.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b0097273-7bb1-4a39-b413-6c19c9cba68a
📒 Files selected for processing (1)
apps/iris/src/components/navbar.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/navbar.tsx
🔇 Additional comments (2)
apps/iris/src/components/navbar.tsx (2)
81-99: LGTM!
234-267: LGTM!Also applies to: 281-294
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/iris/src/components/navbar.tsx (1)
46-46:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix
icontype:React.ElementTyperequires React namespace import.
React.ElementTypeis referenced, but the React namespace is not imported (line 14 only importsReactNode). ImportElementTypedirectly from 'react' or use a different type reference.🔧 Proposed fix
Update the import on line 14:
-import type { ReactNode } from 'react'; +import type { ElementType, ReactNode } from 'react';Then update line 46:
type NavItem = { to: LinkProps['to']; - icon: React.ElementType; + icon: ElementType; labelKey: string; adminOnly?: boolean; };🤖 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 `@apps/iris/src/components/navbar.tsx` at line 46, The prop type for icon uses React.ElementType but the React namespace isn't imported; update the import that currently brings in ReactNode to also import ElementType from 'react' (e.g., import { ReactNode, ElementType } from 'react'), then keep the prop declaration as icon: ElementType in the Navbar component so the type resolves correctly.
🤖 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.
Duplicate comments:
In `@apps/iris/src/components/navbar.tsx`:
- Line 46: The prop type for icon uses React.ElementType but the React namespace
isn't imported; update the import that currently brings in ReactNode to also
import ElementType from 'react' (e.g., import { ReactNode, ElementType } from
'react'), then keep the prop declaration as icon: ElementType in the Navbar
component so the type resolves correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e3169d04-4568-418d-8734-340eed4ea61a
📒 Files selected for processing (1)
apps/iris/src/components/navbar.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (3)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/navbar.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/navbar.tsx
🔇 Additional comments (6)
apps/iris/src/components/navbar.tsx (6)
1-1: LGTM!Also applies to: 10-12
51-55: LGTM!
66-66: LGTM!Also applies to: 68-71
79-99: LGTM!
234-267: LGTM!
273-297: LGTM!
2 participants
This pull request adds a mobile navigation menu to the
Navbarcomponent, improving usability on smaller screens. It introduces a toggle button for opening and closing the menu, ensures accessibility with localized labels, and displays navigation links conditionally based on user permissions and authentication status. Additionally, translations for the navigation menu are added in both English and Hungarian.Mobile navigation improvements:
Navbarthat displays either a menu or close icon, with an accessible label using the newnavigation.mobileMenutranslation key. The button is only shown for authenticated users and when navigation links are visible. (apps/iris/src/components/navbar.tsx,apps/iris/public/locales/en/translation.json,apps/iris/public/locales/hu/translation.json) [1] [2] [3] [4]apps/iris/src/components/navbar.tsx)Localization:
apps/iris/public/locales/en/translation.json,apps/iris/public/locales/hu/translation.json) [1] [2]Closes #229
Summary by CodeRabbit
New Features
Localization