From a07e76c7401b4a0735110a301e7e395d95d9600d Mon Sep 17 00:00:00 2001
From: jubnl <jgunther021@gmail.com>
Date: Wed, 15 Apr 2026 03:04:25 +0200
Subject: [PATCH] fix(login): address review feedback on language dropdown PR
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix import path: use i18n barrel instead of TranslationContext directly
- Encapsulate localStorage key behind hasStoredLanguage() helper in settingsStore
- Fix pt-BR detection: only map pt-BR to br, pt-PT now returns null correctly
- Add comment linking server SUPPORTED_LANG_CODES to canonical client source
- Extract /api/config inline handler to routes/publicConfig.ts
- Add aria-haspopup, aria-expanded, role=listbox/option, aria-selected to dropdown
- Add 8 tests for detectBrowserLanguage (FE-COMP-I18N-016–023)
- Add 3 tests for setLanguageTransient (FE-STORE-SETTINGS-015–017)
---
 client/src/i18n/TranslationContext.tsx        |  6 ++-
 client/src/pages/LoginPage.tsx                | 12 ++++-
 client/src/store/settingsStore.ts             |  5 ++
 client/tests/unit/i18n/index.test.ts          | 51 +++++++++++++++++++
 .../tests/unit/stores/settingsStore.test.ts   | 31 +++++++++++
 server/src/app.ts                             |  5 +-
 server/src/config.ts                          |  2 +
 server/src/routes/publicConfig.ts             | 10 ++++
 8 files changed, 116 insertions(+), 6 deletions(-)
 create mode 100644 server/src/routes/publicConfig.ts

diff --git a/client/src/i18n/TranslationContext.tsx b/client/src/i18n/TranslationContext.tsx
index f18dcf20..964a9af7 100644
--- a/client/src/i18n/TranslationContext.tsx
+++ b/client/src/i18n/TranslationContext.tsx
@@ -59,8 +59,10 @@ export function detectBrowserLanguage(): string | null {
     const exactMatch = supported.find(s => s.toLowerCase() === lang.toLowerCase())
     if (exactMatch) return exactMatch
 
-    // Portuguese variants → our code is 'br' (pt-BR)
-    if (lang.toLowerCase().startsWith('pt')) return 'br'
+    // pt-BR has no exact match (our code is 'br', not 'pt-BR'), so map it explicitly.
+    // pt-PT and bare 'pt' are NOT mapped — they fall through to null and let the
+    // server default or 'en' fallback apply instead.
+    if (lang.toLowerCase() === 'pt-br') return 'br'
 
     // Prefix match (e.g. 'de-AT' → 'de', 'zh-CN' → 'zh') — case-insensitive
     const prefix = lang.split('-')[0].toLowerCase()
diff --git a/client/src/pages/LoginPage.tsx b/client/src/pages/LoginPage.tsx
index 8a309b62..4750531e 100644
--- a/client/src/pages/LoginPage.tsx
+++ b/client/src/pages/LoginPage.tsx
@@ -2,8 +2,9 @@ import React, { useState, useEffect, useMemo, useRef } from 'react'
 import { useNavigate, useLocation } from 'react-router-dom'
 import { useAuthStore } from '../store/authStore'
 import { useSettingsStore } from '../store/settingsStore'
-import { SUPPORTED_LANGUAGES, useTranslation, detectBrowserLanguage } from '../i18n/TranslationContext'
+import { SUPPORTED_LANGUAGES, useTranslation, detectBrowserLanguage } from '../i18n'
 import { authApi, configApi } from '../api/client'
+import { hasStoredLanguage } from '../store/settingsStore'
 import { getApiErrorMessage } from '../types'
 import { Plane, Eye, EyeOff, Mail, Lock, MapPin, Calendar, Package, User, Globe, Zap, Users, Wallet, Map, CheckSquare, BookMarked, FolderOpen, Route, Shield, KeyRound, ChevronDown } from 'lucide-react'
 
@@ -124,7 +125,7 @@ export default function LoginPage(): React.ReactElement {
   // 3. Server default (DEFAULT_LANGUAGE env var)
   // 4. 'en' → hardcoded fallback already in store
   useEffect(() => {
-    if (localStorage.getItem('app_language')) return
+    if (hasStoredLanguage()) return
 
     const detected = detectBrowserLanguage()
     if (detected) {
@@ -396,6 +397,9 @@ export default function LoginPage(): React.ReactElement {
       <div style={{ position: 'absolute', top: 16, right: 16, zIndex: 10 }}>
         <button
           onClick={(e) => { e.stopPropagation(); setLangDropdownOpen(o => !o) }}
+          aria-haspopup="listbox"
+          aria-expanded={langDropdownOpen}
+          aria-label="Change language"
           style={{
             display: 'flex', alignItems: 'center', gap: 6,
             padding: '6px 12px', borderRadius: 99,
@@ -414,6 +418,8 @@ export default function LoginPage(): React.ReactElement {
 
         {langDropdownOpen && (
           <div
+            role="listbox"
+            aria-label="Select language"
             onClick={(e) => e.stopPropagation()}
             style={{
               position: 'absolute', top: '100%', right: 0, marginTop: 4,
@@ -426,6 +432,8 @@ export default function LoginPage(): React.ReactElement {
             {SUPPORTED_LANGUAGES.map(({ value, label }) => (
               <button
                 key={value}
+                role="option"
+                aria-selected={value === language}
                 onClick={() => { setLanguageLocal(value); setLangDropdownOpen(false) }}
                 style={{
                   display: 'block', width: '100%', textAlign: 'left',
diff --git a/client/src/store/settingsStore.ts b/client/src/store/settingsStore.ts
index f10ff7d8..2c0e62ce 100644
--- a/client/src/store/settingsStore.ts
+++ b/client/src/store/settingsStore.ts
@@ -15,6 +15,11 @@ interface SettingsState {
   updateSettings: (settingsObj: Partial<Settings>) => Promise<void>
 }
 
+// Returns true when the user has explicitly chosen a language (persisted in localStorage).
+// Use this instead of reading localStorage directly so the key stays encapsulated here.
+export const hasStoredLanguage = (): boolean =>
+  typeof localStorage !== 'undefined' && !!localStorage.getItem('app_language')
+
 export const useSettingsStore = create<SettingsState>((set, get) => ({
   settings: {
     map_tile_url: '',
diff --git a/client/tests/unit/i18n/index.test.ts b/client/tests/unit/i18n/index.test.ts
index ae1f7f4b..3b15721b 100644
--- a/client/tests/unit/i18n/index.test.ts
+++ b/client/tests/unit/i18n/index.test.ts
@@ -8,6 +8,7 @@ import {
   getIntlLanguage,
   isRtlLanguage,
   SUPPORTED_LANGUAGES,
+  detectBrowserLanguage,
 } from '../../../src/i18n'
 import { resetAllStores, seedStore } from '../../helpers/store'
 import { useSettingsStore } from '../../../src/store/settingsStore'
@@ -96,6 +97,56 @@ describe('SUPPORTED_LANGUAGES', () => {
   })
 })
 
+// ── FE-COMP-I18N-016 to 023: detectBrowserLanguage ───────────────────────────
+
+describe('detectBrowserLanguage', () => {
+  afterEach(() => {
+    Object.defineProperty(navigator, 'languages', { value: [], configurable: true })
+    Object.defineProperty(navigator, 'language', { value: '', configurable: true })
+  })
+
+  it('FE-COMP-I18N-016: exact match returns the matched code', () => {
+    Object.defineProperty(navigator, 'languages', { value: ['de'], configurable: true })
+    expect(detectBrowserLanguage()).toBe('de')
+  })
+
+  it('FE-COMP-I18N-017: region-tagged exact match (zh-TW) returns zh-TW', () => {
+    Object.defineProperty(navigator, 'languages', { value: ['zh-TW'], configurable: true })
+    expect(detectBrowserLanguage()).toBe('zh-TW')
+  })
+
+  it('FE-COMP-I18N-018: prefix match (de-AT → de)', () => {
+    Object.defineProperty(navigator, 'languages', { value: ['de-AT'], configurable: true })
+    expect(detectBrowserLanguage()).toBe('de')
+  })
+
+  it('FE-COMP-I18N-019: pt-PT returns null (European Portuguese is a distinct language)', () => {
+    Object.defineProperty(navigator, 'languages', { value: ['pt-PT'], configurable: true })
+    expect(detectBrowserLanguage()).toBeNull()
+  })
+
+  it('FE-COMP-I18N-020: pt-BR maps to br', () => {
+    Object.defineProperty(navigator, 'languages', { value: ['pt-BR'], configurable: true })
+    expect(detectBrowserLanguage()).toBe('br')
+  })
+
+  it('FE-COMP-I18N-021: first-match-wins across multiple entries', () => {
+    Object.defineProperty(navigator, 'languages', { value: ['xx-XX', 'fr'], configurable: true })
+    expect(detectBrowserLanguage()).toBe('fr')
+  })
+
+  it('FE-COMP-I18N-022: unknown language returns null', () => {
+    Object.defineProperty(navigator, 'languages', { value: ['xx'], configurable: true })
+    expect(detectBrowserLanguage()).toBeNull()
+  })
+
+  it('FE-COMP-I18N-023: falls back to navigator.language when navigator.languages is empty', () => {
+    Object.defineProperty(navigator, 'languages', { value: [], configurable: true })
+    Object.defineProperty(navigator, 'language', { value: 'es', configurable: true })
+    expect(detectBrowserLanguage()).toBe('es')
+  })
+})
+
 // ── FE-COMP-I18N-010 to 015: TranslationProvider + useTranslation ─────────────
 
 describe('TranslationProvider + useTranslation integration', () => {
diff --git a/client/tests/unit/stores/settingsStore.test.ts b/client/tests/unit/stores/settingsStore.test.ts
index 508abea2..cc020f5f 100644
--- a/client/tests/unit/stores/settingsStore.test.ts
+++ b/client/tests/unit/stores/settingsStore.test.ts
@@ -170,6 +170,37 @@ describe('settingsStore', () => {
     });
   });
 
+  describe('FE-STORE-SETTINGS-015: setLanguageTransient updates state without touching localStorage', () => {
+    it('sets language in state but does not write to localStorage', () => {
+      localStorage.clear();
+
+      useSettingsStore.getState().setLanguageTransient('fr');
+
+      expect(useSettingsStore.getState().settings.language).toBe('fr');
+      expect(localStorage.getItem('app_language')).toBeNull();
+    });
+  });
+
+  describe('FE-STORE-SETTINGS-016: setLanguageTransient rejects unsupported language code', () => {
+    it('leaves state unchanged for an unknown code', () => {
+      const before = useSettingsStore.getState().settings.language;
+
+      useSettingsStore.getState().setLanguageTransient('xx');
+
+      expect(useSettingsStore.getState().settings.language).toBe(before);
+    });
+  });
+
+  describe('FE-STORE-SETTINGS-017: setLanguageTransient does not overwrite an explicit localStorage choice', () => {
+    it('localStorage remains unchanged after a transient set', () => {
+      localStorage.setItem('app_language', 'de');
+
+      useSettingsStore.getState().setLanguageTransient('es');
+
+      expect(localStorage.getItem('app_language')).toBe('de');
+    });
+  });
+
   describe('FE-STORE-SETTINGS-014: updateSetting API failure leaves optimistic state', () => {
     it('throws on API failure but keeps the optimistic state', async () => {
       server.use(
diff --git a/server/src/app.ts b/server/src/app.ts
index 86a1c275..f4c91791 100644
--- a/server/src/app.ts
+++ b/server/src/app.ts
@@ -6,7 +6,7 @@ import path from 'node:path';
 import fs from 'node:fs';
 
 import jwt from 'jsonwebtoken';
-import { JWT_SECRET, DEFAULT_LANGUAGE } from './config';
+import { JWT_SECRET } from './config';
 import { logDebug, logWarn, logError } from './services/auditLog';
 import { enforceGlobalMfaPolicy } from './middleware/mfaPolicy';
 import { authenticate } from './middleware/auth';
@@ -41,6 +41,7 @@ import notificationRoutes from './routes/notifications';
 import shareRoutes from './routes/share';
 import journeyRoutes from './routes/journey';
 import journeyPublicRoutes from './routes/journeyPublic';
+import publicConfigRoutes from './routes/publicConfig';
 import { mcpHandler } from './mcp';
 import { Addon } from './types';
 import { getPhotoProviderConfig } from './services/memories/helpersService';
@@ -194,7 +195,7 @@ export function createApp(): express.Application {
   app.use('/api/trips/:tripId/reservations', reservationsRoutes);
   app.use('/api/trips/:tripId/days/:dayId/notes', dayNotesRoutes);
   app.get('/api/health', (_req: Request, res: Response) => res.json({ status: 'ok' }));
-  app.get('/api/config', (_req: Request, res: Response) => res.json({ defaultLanguage: DEFAULT_LANGUAGE }));
+  app.use('/api/config', publicConfigRoutes);
   app.use('/api', assignmentsRoutes);
   app.use('/api/tags', tagsRoutes);
   app.use('/api/categories', categoriesRoutes);
diff --git a/server/src/config.ts b/server/src/config.ts
index 50a6dc97..2941a87f 100644
--- a/server/src/config.ts
+++ b/server/src/config.ts
@@ -102,6 +102,8 @@ export const ENCRYPTION_KEY = _encryptionKey;
 // DEFAULT_LANGUAGE sets the language shown on the login page before the user
 // selects one. Only applies when the user has no saved language preference.
 // Supported values: de, en, es, fr, hu, nl, br, cs, pl, ru, zh, zh-TW, it, ar
+// Must stay in sync with client/src/i18n/supportedLanguages.ts (canonical source).
+// Kept duplicated here because server and client are separate npm packages.
 const SUPPORTED_LANG_CODES = ['de', 'en', 'es', 'fr', 'hu', 'nl', 'br', 'cs', 'pl', 'ru', 'zh', 'zh-TW', 'it', 'ar'];
 const rawDefaultLang = process.env.DEFAULT_LANGUAGE || 'en';
 if (!SUPPORTED_LANG_CODES.includes(rawDefaultLang)) {
diff --git a/server/src/routes/publicConfig.ts b/server/src/routes/publicConfig.ts
new file mode 100644
index 00000000..f39dbb28
--- /dev/null
+++ b/server/src/routes/publicConfig.ts
@@ -0,0 +1,10 @@
+import express, { Request, Response } from 'express';
+import { DEFAULT_LANGUAGE } from '../config';
+
+const router = express.Router();
+
+router.get('/', (_req: Request, res: Response) => {
+  res.json({ defaultLanguage: DEFAULT_LANGUAGE });
+});
+
+export default router;