feat: add header language toggle - #71
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe locale toggle is centralized in ChangesLocale toggle
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatHeader
participant ChatPageContent
participant localStorage
User->>ChatHeader: Click language toggle
ChatHeader->>ChatPageContent: Invoke toggleLocale
ChatPageContent->>ChatPageContent: Switch locale state
ChatPageContent->>localStorage: Persist quorum_locale
ChatPageContent-->>ChatHeader: Updated locale
ChatHeader-->>User: Display updated language label
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3180891981
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/page.tsx`:
- Around line 289-295: Update toggleLocale so the functional setLocale updater
only computes and returns the next locale; move the localStorage.setItem side
effect outside that updater while preserving the existing locale toggle behavior
and persistence.
- Around line 289-295: Update toggleLocale and the existing thread update flow
so changing language persists the new locale on the current thread, ensuring
reloads retain the selected language when loading thread.locale. Alternatively,
stop the thread-loading logic from overriding the stored locale, while
preserving locale persistence for both new and existing threads.
In `@src/components/Header.tsx`:
- Around line 366-374: Update the visible text in the locale toggle
motion.button using the same locale condition as aria-label, so it displays the
target language rather than the current language: show Korean when locale is
English and English when locale is Korean. Keep the existing onToggleLocale
behavior and accessibility label unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f0ce45a-adff-448a-93b6-6d0e380fd585
📒 Files selected for processing (2)
src/app/page.tsxsrc/components/Header.tsx
📜 Review details
🧰 Additional context used
🪛 React Doctor (0.9.1)
src/app/page.tsx
[error] 290-290: This state updater performs setItem(). React may run updater functions more than once, so side effects here can repeat or observe inconsistent external state.
Keep state updater callbacks pure and return only the next state. Move notifications, storage, timers, ref writes, and other external work into the event or effect that queues the update.
(no-impure-state-updater)
[error] 292-292: This side-effecting call runs inside a state updater, which React may invoke more than once. Move it outside the setter after computing the next state.
React may replay a state updater, so callbacks, analytics, and persistence inside it can run more than once. Compute state purely, then perform the side effect outside the setter.
(no-side-effect-in-state-updater-function)
🔇 Additional comments (2)
src/app/page.tsx (1)
552-552: LGTM!Also applies to: 567-567
src/components/Header.tsx (1)
87-87: LGTM!Also applies to: 105-105
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/app/page.tsx`:
- Line 503: Update the mount logic around setLocale and quorum_locale so a
stored locale is treated as authoritative only when it is valid ("en" or "ko").
Reuse the existing locale validation used elsewhere, allowing thread.locale
hydration when the stored value is missing or invalid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f2ba403b-ac66-4f0a-bbf5-d97dcaf49fa8
📒 Files selected for processing (2)
src/app/page.tsxsrc/components/Header.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: ci
🔇 Additional comments (3)
src/components/Header.tsx (2)
365-374: The visible label still shows the current language.This is the same unresolved issue previously reported: when the locale is
"en", the button displays"EN"even though it switches to Korean, and vice versa. Display the target language instead.
87-87: LGTM!Also applies to: 105-105, 185-185
src/app/page.tsx (1)
289-294: LGTM!Also applies to: 550-550, 565-565
Adds a persisted EN/한글 header toggle for desktop and mobile.
Review fixes applied
292bb55preserves locale preference and mobile fit.