feat: app header - #55
Conversation
* feat: install next+ts+linter+prettier+ husky+redux * feat: configs + perfectionist + lint fix * feat: install scss * refactor: add some folders * feat: install i18n + basic configs * feat: add internatiolisation * feat: language switcher * feat: dependencies update * feat: add sticky header * feat: add mui Button-and-Link-like component * fix: lint * style: add style for header * refactor: remove css from language selector * refactor: add const for flex styles in the header boxes * refactor: history style as link * fix: add some fixes
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds next-intl locale routing, request handling, and middleware; locale-aware app layout and page rendering; MUI theme/provider setup; and shared header, footer, and language-switching components. It also adds English and Russian message catalogs and removes the prior non-localized app shell files. Changesi18n and UI integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LanguageSwitcher
participant Router as src/i18n/navigation.ts
participant Header
User->>LanguageSwitcher: select locale
LanguageSwitcher->>Router: router.replace(pathname, { locale: nextLocale })
Router->>Header: navigate with new locale
Header-->>User: render translated navigation
sequenceDiagram
participant Browser
participant proxy as src/proxy.ts
participant routing as src/i18n/routing.ts
participant request as src/i18n/request.ts
Browser->>proxy: request path
proxy->>routing: apply matcher and locales
Browser->>request: getRequestConfig
request->>routing: validate requestLocale
request->>request: import locale messages
request-->>Browser: { locale, messages }
Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/theme/theme.ts (1)
10-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded design tokens.
Colors (
#faf5ff,#7c3aed, etc.) and dimensions (borderRadius: 50,controlHeight) are hardcoded inline rather than centralized as named tokens/constants. Fine for now, but as the theme grows this will get harder to keep consistent (e.g.,primary.main#7c3aedduplicated conceptually across gradient stops).Also applies to: 93-101
🤖 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/theme/theme.ts` around lines 10 - 14, The theme styling in theme.ts still uses hardcoded color values and sizing directly in the theme object, which should be centralized. Refactor the inline values used in the theme configuration (including the gradient, border, shadow, text color, and the repeated primary purple concept) into named design tokens/constants and reuse them via the theme setup, especially in the theme creation logic and the related style block noted in the review. Keep the existing look while ensuring values like the primary color, radii, and control sizing are defined once and referenced consistently.src/app/[locale]/layout.tsx (1)
19-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing locale validation before rendering/metadata generation.
Both
generateMetadataandRootLayoutuse the rawlocalefromparamswithout validating it againstrouting.locales. The official next-intl setup guide validates this and callsnotFound()for unsupported locales before proceeding. Without it, an invalid locale segment (e.g./fr/...) won't 404 —<html lang={locale}>renders the invalid value while message loading silently falls back todefaultLocaleviai18n/request.ts, producing content/lang-attribute mismatch.🔧 Proposed fix
import { NextIntlClientProvider } from 'next-intl'; import { getTranslations } from 'next-intl/server'; +import { hasLocale } from 'next-intl'; +import { notFound } from 'next/navigation'; import { MuiProvider } from '`@/providers/mui-provider`'; +import { routing } from '`@/i18n/routing`'; export async function generateMetadata({ params }: MetadataProps): Promise<Metadata> { const { locale } = await params; + if (!hasLocale(routing.locales, locale)) { + notFound(); + } const t = await getTranslations({ locale, namespace: 'Metadata', }); ... } export default async function RootLayout({ children, params }: Props) { const { locale } = await params; + if (!hasLocale(routing.locales, locale)) { + notFound(); + } return (🤖 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/app/`[locale]/layout.tsx around lines 19 - 44, Add locale guarding in both generateMetadata and RootLayout by validating the params.locale against routing.locales before using it. If the locale is not supported, call notFound() immediately so invalid locale routes 404 instead of rendering with a mismatched lang value. Keep the validation close to the locale extraction in these functions, and continue using getTranslations and NextIntlClientProvider only after the locale has been confirmed valid.src/i18n/messages/en.json (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove leftover debug/scaffold translation key.
"check": "check english"looks like leftover bootstrap/test content (also consumed twice inpage.tsxviat('check')) rather than real product copy. Consider removing it and the corresponding usage before merge, alongside the same key inru.json.🤖 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/i18n/messages/en.json` at line 18, Remove the leftover scaffold translation key and its usage: delete the `check` entry from the i18n message files (including the matching key in `ru.json`) and update `page.tsx` to stop calling `t('check')` in both places. Use the translation lookup in `page.tsx` and the `messages/en.json` / `messages/ru.json` entries to locate and clean up the debug-only copy.src/components/auth-navigation/auth-navigation.tsx (2)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent import path — bypasses the barrel.
header.tsximports link components via@/components, but this file importsAppLinkButtondirectly from'../app-link/app-link'. Givensrc/components/index.tsalso exports the same component underAppLink, this inconsistency (and the redundant aliasing) is worth consolidating — see the related comment insrc/components/index.ts.🤖 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/auth-navigation/auth-navigation.tsx` at line 3, The auth-navigation import is bypassing the shared components barrel and using a direct path for AppLinkButton, which is inconsistent with the rest of the codebase. Update auth-navigation.tsx to import the component through the components barrel used elsewhere, and align the naming with the existing AppLink export from src/components/index.ts so the aliasing is no longer redundant.Source: Path instructions
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove commented-out
useAuth()call.Line 10 leaves
//useAuth();as dead code alongside the hardcoded stub object. As per path instructions, "Flag commented-out code blocks (dead code left in comments) - penalized per block."Suggested cleanup
- const { isAuthenticated, isAuthLoading, signOut } = { - isAuthenticated: true, - isAuthLoading: false, - signOut: () => {}, - }; //useAuth(); + // TODO(Feature 2): replace with real useAuth() + const { isAuthenticated, isAuthLoading, signOut } = { + isAuthenticated: true, + isAuthLoading: false, + signOut: () => {}, + };🤖 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/auth-navigation/auth-navigation.tsx` around lines 6 - 10, Remove the commented-out useAuth() call from auth-navigation.tsx and keep the AuthNavigation logic using either the stubbed object or the real hook, but not both. Update the destructuring in AuthNavigation so the dead commented code is deleted entirely and the remaining symbols isAuthenticated, isAuthLoading, and signOut stay clear and intentional.Source: Path instructions
src/components/index.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
AppLinkBasebarrel export.
src/components/index.tsre-exportsAppLinkButtonas bothAppLinkBaseandAppLink, butAppLinkBasehas no consumers in the repo. Keeping a single public name avoids a redundant API surface and confusion.🤖 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/index.ts` around lines 1 - 2, Remove the redundant AppLinkBase barrel export from the components index so only the public AppLink alias remains. Update src/components/index.ts to keep the AppLinkButton export as AppLink and delete the AppLinkBase re-export, since AppLinkBase has no consumers and should not be part of the public API.Source: Path instructions
🤖 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/app/`[locale]/page.tsx:
- Around line 8-87: Remove the leftover placeholder/debug JSX from the home page
and replace it with the real `HomePage` content. In the `[locale]/page.tsx`
component, eliminate the repeated `t('check')` `<div>` blocks and render the
intended translation-driven sections instead, using the existing `title` and
`description` message keys from the page/catalog setup so the page no longer
ships with duplicate scaffold markup.
In `@src/components/header/header.tsx`:
- Around line 41-56: The header and auth navigation labels are hardcoded English
strings and are not using the existing next-intl messages. Update the Header and
auth navigation components to call useTranslations for the Header namespace, and
replace the literal text in AppLink/AuthNavigation with translation keys for
“Swagger Editor”, “About”, “History”, “Sign Out”, “Sign In”, and “Sign Up”. Keep
the existing component structure intact and ensure the labels render from the
message catalog so the language switcher updates the visible navigation text.
---
Nitpick comments:
In `@src/app/`[locale]/layout.tsx:
- Around line 19-44: Add locale guarding in both generateMetadata and RootLayout
by validating the params.locale against routing.locales before using it. If the
locale is not supported, call notFound() immediately so invalid locale routes
404 instead of rendering with a mismatched lang value. Keep the validation close
to the locale extraction in these functions, and continue using getTranslations
and NextIntlClientProvider only after the locale has been confirmed valid.
In `@src/components/auth-navigation/auth-navigation.tsx`:
- Line 3: The auth-navigation import is bypassing the shared components barrel
and using a direct path for AppLinkButton, which is inconsistent with the rest
of the codebase. Update auth-navigation.tsx to import the component through the
components barrel used elsewhere, and align the naming with the existing AppLink
export from src/components/index.ts so the aliasing is no longer redundant.
- Around line 6-10: Remove the commented-out useAuth() call from
auth-navigation.tsx and keep the AuthNavigation logic using either the stubbed
object or the real hook, but not both. Update the destructuring in
AuthNavigation so the dead commented code is deleted entirely and the remaining
symbols isAuthenticated, isAuthLoading, and signOut stay clear and intentional.
In `@src/components/index.ts`:
- Around line 1-2: Remove the redundant AppLinkBase barrel export from the
components index so only the public AppLink alias remains. Update
src/components/index.ts to keep the AppLinkButton export as AppLink and delete
the AppLinkBase re-export, since AppLinkBase has no consumers and should not be
part of the public API.
In `@src/i18n/messages/en.json`:
- Line 18: Remove the leftover scaffold translation key and its usage: delete
the `check` entry from the i18n message files (including the matching key in
`ru.json`) and update `page.tsx` to stop calling `t('check')` in both places.
Use the translation lookup in `page.tsx` and the `messages/en.json` /
`messages/ru.json` entries to locate and clean up the debug-only copy.
In `@src/theme/theme.ts`:
- Around line 10-14: The theme styling in theme.ts still uses hardcoded color
values and sizing directly in the theme object, which should be centralized.
Refactor the inline values used in the theme configuration (including the
gradient, border, shadow, text color, and the repeated primary purple concept)
into named design tokens/constants and reuse them via the theme setup,
especially in the theme creation logic and the related style block noted in the
review. Keep the existing look while ensuring values like the primary color,
radii, and control sizing are defined once and referenced consistently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a9104a81-8e59-4d48-bd6f-9cb8be184e17
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
next.config.tspackage.jsonsrc/app/[locale]/layout.tsxsrc/app/[locale]/page.tsxsrc/app/globals.csssrc/app/layout.tsxsrc/app/page.module.csssrc/app/page.tsxsrc/components/app-link/app-link.tsxsrc/components/auth-navigation/auth-navigation.tsxsrc/components/header/header.tsxsrc/components/index.tssrc/components/language-switcher/language-switcher.tsxsrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonsrc/i18n/navigation.tssrc/i18n/request.tssrc/i18n/routing.tssrc/providers/mui-provider.tsxsrc/proxy.tssrc/theme/theme.ts
💤 Files with no reviewable changes (3)
- src/app/page.tsx
- src/app/page.module.css
- src/app/layout.tsx
* feat: add Inter and JetBrains Mono fonts via next/font * feat: reorganize theme into palette, components, typography, theme modules * feat: add i18n translations for header navigation links * fix: prevent header layout shift on language switcher toggle click * feat: add footer with translated links and styling * test: add tests for footer * fix: correct invalid box-shadow offset * fix: fix header height * fix: fix language switcher * refactor: extract routes and external links into constants
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/theme/theme.ts`:
- Around line 7-21: The theme setup enables color schemes and CSS variables, so
the root layout must initialize the correct scheme before rendering the app.
Update the `src/app/[locale]/layout.tsx` root layout to include
`InitColorSchemeScript` as the first child inside `<body>`, alongside the
existing theme usage from `theme`, so the initial SSR color scheme matches the
client and avoids flash/hydration mismatch.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 56fadd97-cefb-4228-a368-3bff6b9ad91b
📒 Files selected for processing (19)
src/app/[locale]/layout.tsxsrc/app/[locale]/page.tsxsrc/app/globals.csssrc/components/auth-navigation/auth-navigation.tsxsrc/components/footer/footer.test.tsxsrc/components/footer/footer.tsxsrc/components/header/header.tsxsrc/components/index.tssrc/components/language-switcher/language-switcher.tsxsrc/constants/brand.tssrc/constants/routes.tssrc/fonts.tssrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonsrc/theme/components.tssrc/theme/index.tssrc/theme/palette.tssrc/theme/theme.tssrc/theme/typography.ts
💤 Files with no reviewable changes (1)
- src/app/globals.css
✅ Files skipped from review due to trivial changes (4)
- src/constants/routes.ts
- src/i18n/messages/ru.json
- src/theme/components.ts
- src/theme/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/i18n/messages/en.json
- src/components/auth-navigation/auth-navigation.tsx
- src/components/language-switcher/language-switcher.tsx
- src/components/header/header.tsx
- src/app/[locale]/layout.tsx
- src/app/[locale]/page.tsx
|
|
||
| import { Footer } from '@/components/footer/footer'; | ||
|
|
||
| export default async function Home() { |
Summary
Implements Feature 1 and partially Feature 7 from the RS School requirements - header with navigation, i18n switcher, sticky behavior, and auth-state-dependent buttons. Two acceptance criteria depend on auth (not yet implemented) and are tracked separately.
Changes Made
LanguageSwitchercomponent (RU/EN via next-intl)AuthNavigationcomponent rendering Sign In/Sign Up (non-authenticated) or History/Sign Out (authenticated); currently driven by auseAuthstub, to be wired to real Supabase auth in Feature 2Headerwith scroll-triggered compact animation (useScrollTrigger)palette.ts,components.ts,typography.ts,theme.ts) to support header color scheme changessrc/constants/routes.ts,src/constants/brand.ts)Not implemented
These require the real auth flow (
story/2-*branches) and will be closed out with Feature 2.Related issue
Closes #8, #9, #17, #18 , #19, #49, #54
Feature
Feature 1: App Header
Feature 7: General Requirements
Acceptance criteria
Feature 1: App Header
Feature 2: Sign In / Sign Up
Feature 3: Swagger Editor
Feature 4: Swagger Viewer
Feature 5: History and Analytics
Feature 6: About Page
Feature 7: General Requirements
Feature 8: YouTube Video
Type of change
Checklist
any/@ts-ignoreSummary by CodeRabbit
New Features
Bug Fixes