fix(site): scope language picker to lesson pages - #412
Conversation
The picker showed on index.html but window.AIFS_onLangChange is only defined on lesson.html, so selecting a language there updated localStorage/URL but never re-rendered content. docs/i18n.md documents the site switcher as belonging to lesson.html only — gate mount() accordingly. Fixes rohitg00#404
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe language picker now requires both its host element and language-change callback before initialization. Certification lessons still force English and hide the picker. Other pages mount the picker after the guard. ChangesLanguage picker initialization
Estimated code review effort: 2 (Simple) | ~5 minutes Merge Risk: ⚪ Minimal · up to This narrowly scopes the language picker to lesson pages where language switching is supported; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Deliberate, not an oversight — this looks like one of the bot's heuristics comparing the diff against the issue's literal wording, without visibility into the i18n pipeline's actual scope. There's no translated content source for the landing page anywhere in the repo (see docs/i18n.md — the "Site switcher" row scopes it to lesson.html only; landing-page strings were never part of the translation pipeline). So there was nothing for the picker to render into on index.html — it updated localStorage/URL but had no content to swap in, which is the bug. Building a full landing-page i18n system would be a much larger scope change than this bug report calls for. Given the picker was never functional there and docs/i18n.md documents lesson.html as its intended home, removing it from a page where it can't work seemed like the right fix rather than expanding scope to make it work everywhere. Happy to go the other direction if @rohitg00 would rather see landing-page translations added — just flagging that's a separate, bigger piece of work. |
What this PR does
Scopes the language picker to lesson pages only, since
window.AIFS_onLangChange— the hook that actually re-renders content in the selected language — is only ever defined onlesson.html.Kind of change
Checklist
docs/en.mdclaimsPhase / lesson
N/A — this is a site-level fix (
site/lang-picker.js), not a lesson change.Notes for reviewer
Root cause: the picker mounted on
index.htmltoo, but the landing page has noAIFS_onLangChangehook, so selecting a language updatedlocalStorage/the URL but never re-rendered content — matching the reported bug.docs/i18n.md's "Site switcher" row documentslesson.htmlas the only intended location, so I gatedinit()onwindow.AIFS_onLangChangebeing defined rather than trying to add landing-page translations that don't exist in the pipeline.Tested locally on both pages:
index.htmlno longer mounts the picker;lesson.htmlstill mounts it and language switching still works (translated markdown still fetches, falls back to English correctly).Left the empty
<div id="langPicker">inindex.htmlin place rather than removing it — kept the diff to one file/one behavior per the contribution guidelines.Fixes #404