fix(login): address review feedback on language dropdown PR

- 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)
This commit is contained in:
jubnl
2026-04-15 03:04:25 +02:00
parent f35c503658
commit a07e76c740
8 changed files with 116 additions and 6 deletions
+4 -2
View File
@@ -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()
+10 -2
View File
@@ -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',
+5
View File
@@ -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: '',
+51
View File
@@ -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', () => {
@@ -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(
+3 -2
View File
@@ -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);
+2
View File
@@ -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)) {
+10
View File
@@ -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;