feat: add web ui i18n#401
Conversation
📝 WalkthroughWalkthroughThis pull request adds lightweight client-side internationalization (i18n) support to the Web UI without external dependencies. It introduces an ADR documenting the approach, a new i18n module handling locale resolution and DOM translation application, updates to HTML pages integrating the i18n system, CSS styling for locale selectors, and comprehensive tests validating translation key consistency and locale resolution logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant JavaScript
participant LocalStorage
participant DOM
rect rgba(220, 150, 150, 0.5)
Note over Browser,DOM: Locale Resolution Phase (DOMContentLoaded)
JavaScript->>Browser: Check URL query parameter (?lang=)
JavaScript->>LocalStorage: Query stored locale preference
JavaScript->>Browser: Access navigator.language
JavaScript->>JavaScript: Resolve to supported locale (ja/en/zh-Hans)
JavaScript->>LocalStorage: Persist selected locale
end
rect rgba(150, 200, 150, 0.5)
Note over JavaScript,DOM: DOM Translation Phase
JavaScript->>DOM: Query elements with data-i18n
JavaScript->>JavaScript: Fetch translation for (locale, key)
JavaScript->>DOM: Set element.textContent from message catalog
end
rect rgba(150, 150, 200, 0.5)
Note over JavaScript,DOM: Attribute Translation Phase
JavaScript->>DOM: Query elements with data-i18n-placeholder
JavaScript->>DOM: Set placeholder attribute
JavaScript->>DOM: Query elements with data-i18n-title
JavaScript->>DOM: Set title attribute
JavaScript->>DOM: Query elements with data-i18n-aria-label
JavaScript->>DOM: Set aria-label attribute
end
rect rgba(200, 200, 100, 0.5)
Note over Browser,DOM: Locale Selector Change
Browser->>JavaScript: User selects locale from dropdown
JavaScript->>LocalStorage: Update stored preference
JavaScript->>DOM: Re-apply translations for new locale
JavaScript->>DOM: Update document.documentElement.lang
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 550c679db0
ℹ️ 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".
|
|
||
| export function initI18n({ | ||
| root = document, | ||
| storage = globalThis.localStorage, |
There was a problem hiding this comment.
initI18n() eagerly reads globalThis.localStorage in its default parameters, and that property access can throw SecurityError in sandboxed/blocked storage contexts (for example strict privacy settings or restricted iframes). When this happens the function throws before locale resolution, and callers like web/preference/index.html stop initializing the rest of their UI logic after initI18n() is invoked. Please guard storage acquisition with a try/catch (or defer access) so i18n can fall back gracefully.
Useful? React with 👍 / 👎.
CI noteLocal verification for this PR passed:
The failing GitHub Actions checks are firmware workflows:
Both fail with the existing firmware linker error: This appears unrelated to the Web UI / ADR changes in this PR. The |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/flash/index.html`:
- Line 10: The script import is pinned to a floating major version
("esp-web-tools@9") — update the src attribute for the install-button module to
the exact stable release "[email protected]" (i.e., change the URL in the
<script> tag that loads install-button.js to use `@10.2.1`) so the dependency is
precisely pinned for reproducibility and security updates.
In `@web/global.css`:
- Around line 40-45: The grouped selectors in global.css are leaking styles
because only the first selector is scoped (e.g., ".card h1, h2, h3..."), so
change the grouped selectors to fully-qualified scoped selectors: replace
occurrences like ".card h1, h2, h3, h4, h5, h6" with ".card h1, .card h2, .card
h3, .card h4, .card h5, .card h6", and similarly scope form elements by
replacing "select, textarea" and " .card select, textarea" patterns with ".card
select, .card textarea" (and any other repeated groups) so all rules apply only
within the intended container selectors.
In `@web/i18n.mjs`:
- Line 205: The navigatorLanguages assignment can drop navigator.language when
navigator.languages exists but is an empty array; change the expression for
navigatorLanguages to explicitly check navigator.languages.length (e.g., use a
conditional that uses navigator.languages when it has length > 0, otherwise use
[navigator.language]) so that navigator.language is included when languages is
empty; update the navigatorLanguages property in web/i18n.mjs accordingly.
In `@web/package.json`:
- Line 7: Add an explicit "engines" entry to web/package.json to lock the Node
version so locals and CI match: update package.json (the same file containing
the "test": "node --test" script) to include an "engines" object, e.g.
"engines": {"node": "22.x"} (or another exact range your CI uses), and ensure
package.json's "engines" value reflects the CI Node 22 configuration so running
"node --test" behaves consistently across environments.
In `@web/preference/index.html`:
- Around line 142-148: The disconnect control currently implemented as a
clickable <div> with id "ble-disconnect-button" and class
"ble-disconnect-button" must be replaced with a real <button> element
(type="button") to restore native keyboard semantics; update the element to
preserve attributes like id, class, title and data-i18n-title, add an accessible
name via aria-label if title text is not sufficient, and ensure any event
handlers bound to "ble-disconnect-button" continue to work; make the same
replacement for the other instance using the same id/class so both controls are
keyboard and screen-reader accessible.
- Around line 96-99: The current lookup uses
document.querySelector(`[name="${prop}"]`) which can throw on unsafe characters
in prop; replace this with a safer lookup using document.getElementsByName(prop)
(e.g., const elems = document.getElementsByName(prop); if (elems &&
elems.length) elems[0].value = value) so that the code around prop/elem
assignment uses getElementsByName instead of querySelector to avoid CSS selector
parsing errors.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47f726c0-261f-464e-8efe-dc9e482c7737
📒 Files selected for processing (8)
docs/adr/0001-web-ui-i18n.mddocs/adr/README.mdweb/flash/index.htmlweb/global.cssweb/i18n.mjsweb/package.jsonweb/preference/index.htmlweb/test/i18n.test.mjs
| <title data-i18n-document-title="flash.documentTitle">Flash firmware</title> | ||
| <link rel="stylesheet" href="../global.css" /> | ||
| <link rel="stylesheet" href="flash.css" /> | ||
| <script type="module" src="https://unpkg.com/esp-web-tools@9/dist/web/install-button.js?module"></script> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the recommended production integration for esp-web-tools install-button.js (exact version pinning, SRI/integrity usage, and whether self-hosting is recommended)?
💡 Result:
The recommended production integration for esp-web-tools install-button.js uses exact version pinning to the latest stable release (10.2.1 as of Jan 2026), loading from unpkg CDN without SRI/integrity attribute specified in official docs. Self-hosting is recommended as an option ("If you prefer to locally host the JavaScript, download it here" from official docs). Recommended script tag: <script type="module" src="https://unpkg.com/[email protected]/dist/web/install-button.js?module"></script> Official docs example uses @10 (older), but current latest is 10.2.1 per npm/GitHub. No SRI usage in docs or examples. Self-hosting via npm install or download for reliability/control.
Citations:
- 1: https://esphome.github.io/esp-web-tools/
- 2: https://www.npmjs.com/package/esp-web-tools
- 3: https://registry.npmjs.org/esp-web-tools
- 4: https://github.com/esphome/esp-web-tools
- 5: https://github.com/esphome/esp-web-tools/
- 6: https://unpkg.com/browse/[email protected]/dist/
- 7: https://unpkg.com/browse/esp-web-tools/dist/web/
- 8: https://app.unpkg.com/[email protected]/files/dist/web
Update esp-web-tools to the latest stable version with exact pinning.
Line 10 uses esp-web-tools@9, but the latest stable release is 10.2.1 (as of Jan 2026). Update to exact version pinning (e.g., @10.2.1) to ensure reproducibility and receive security updates. While floating major versions are common, pinning to a specific release is the production best practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/flash/index.html` at line 10, The script import is pinned to a floating
major version ("esp-web-tools@9") — update the src attribute for the
install-button module to the exact stable release "[email protected]" (i.e.,
change the URL in the <script> tag that loads install-button.js to use `@10.2.1`)
so the dependency is precisely pinned for reproducibility and security updates.
| .card h1, | ||
| h2, | ||
| h3, | ||
| h4, | ||
| h5, | ||
| h6 { |
There was a problem hiding this comment.
Fix unintended global style leakage from grouped selectors.
At Line 40 and Lines 79/86, only the first selector is scoped; the rest (h2...h6, select, textarea) apply globally. This can override styles outside the target containers.
🎯 Proposed scoped selector fix
-.card h1,
-h2,
-h3,
-h4,
-h5,
-h6 {
+.card h1,
+.card h2,
+.card h3,
+.card h4,
+.card h5,
+.card h6 {
margin: 0.3em 0;
}
-.form-group input,
-select,
-textarea {
+.form-group input,
+.form-group select,
+.form-group textarea {
flex: 2;
padding: 0.5em;
}
-.card input,
-select,
-textarea {
+.card input,
+.card select,
+.card textarea {
border: 1px solid `#bdbdbd`;
border-radius: 4px;
}Also applies to: 79-81, 86-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/global.css` around lines 40 - 45, The grouped selectors in global.css are
leaking styles because only the first selector is scoped (e.g., ".card h1, h2,
h3..."), so change the grouped selectors to fully-qualified scoped selectors:
replace occurrences like ".card h1, h2, h3, h4, h5, h6" with ".card h1, .card
h2, .card h3, .card h4, .card h5, .card h6", and similarly scope form elements
by replacing "select, textarea" and " .card select, textarea" patterns with
".card select, .card textarea" (and any other repeated groups) so all rules
apply only within the intended container selectors.
| let currentLocale = resolveLocale({ | ||
| search: location?.search ?? '', | ||
| storedLocale: storage?.getItem?.(LOCALE_STORAGE_KEY), | ||
| navigatorLanguages: navigator?.languages ?? [navigator?.language], |
There was a problem hiding this comment.
Include navigator.language even when navigator.languages is present but empty.
Line 205 can drop navigator.language when navigator.languages is [], which can incorrectly force fallback to en.
✅ Suggested fallback-safe candidate list
- navigatorLanguages: navigator?.languages ?? [navigator?.language],
+ navigatorLanguages: [...(navigator?.languages ?? []), navigator?.language],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| navigatorLanguages: navigator?.languages ?? [navigator?.language], | |
| navigatorLanguages: [...(navigator?.languages ?? []), navigator?.language], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/i18n.mjs` at line 205, The navigatorLanguages assignment can drop
navigator.language when navigator.languages exists but is an empty array; change
the expression for navigatorLanguages to explicitly check
navigator.languages.length (e.g., use a conditional that uses
navigator.languages when it has length > 0, otherwise use [navigator.language])
so that navigator.language is included when languages is empty; update the
navigatorLanguages property in web/i18n.mjs accordingly.
| "scripts": { | ||
| "dev": "live-server --verbose" | ||
| "dev": "live-server --verbose", | ||
| "test": "node --test" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Node version constraints in repo =="
rg -n --iglob '**/package.json' '"engines"\s*:\s*\{|"node"\s*:\s*"[^\"]+"'
echo
echo "== CI node-version settings =="
rg -n --iglob '*.yml' --iglob '*.yaml' 'setup-node|node-version|matrix:|NODE_VERSION'
echo
echo "== Version hint files =="
fd -HI '(^\.nvmrc$|^\.node-version$)' .Repository: stack-chan/stack-chan
Length of output: 264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Node version constraints in repo (package.json files) =="
find . -name "package.json" -type f | while read file; do
if grep -q '"engines"' "$file"; then
echo "Found engines in: $file"
grep -A 3 '"engines"' "$file" | head -5
fi
done
echo
echo "== Specific check: web/package.json =="
if [ -f "web/package.json" ]; then
cat -n web/package.json | head -20
else
echo "web/package.json not found"
fi
echo
echo "== CI node-version settings in yml/yaml files =="
find . -type f \( -name "*.yml" -o -name "*.yaml" \) | while read file; do
if grep -iE 'node.version|setup.node|NODE_VERSION' "$file" > /dev/null 2>&1; then
echo "=== $file ==="
grep -iEn 'node.version|setup.node|NODE_VERSION' "$file"
fi
done
echo
echo "== Version hint files (.nvmrc, .node-version) =="
find . -maxdepth 2 -type f \( -name ".nvmrc" -o -name ".node-version" \) 2>/dev/null && echo "Found" || echo "None found"Repository: stack-chan/stack-chan
Length of output: 884
Add engines constraint to web/package.json to enforce Node version.
node --test requires Node 18+, but your CI pipeline is configured for Node 22. Add an explicit engines field to ensure local and CI environments are aligned and prevent inconsistencies.
🔧 Suggested package.json guard
{
"name": "stackchan-web",
"version": "0.1.0",
"description": "Stack-chan development server",
+ "engines": {
+ "node": ">=22.0.0"
+ },
"scripts": {
"dev": "live-server --verbose",
"test": "node --test"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "test": "node --test" | |
| { | |
| "name": "stackchan-web", | |
| "version": "0.1.0", | |
| "description": "Stack-chan development server", | |
| "engines": { | |
| "node": ">=22.0.0" | |
| }, | |
| "scripts": { | |
| "dev": "live-server --verbose", | |
| "test": "node --test" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/package.json` at line 7, Add an explicit "engines" entry to
web/package.json to lock the Node version so locals and CI match: update
package.json (the same file containing the "test": "node --test" script) to
include an "engines" object, e.g. "engines": {"node": "22.x"} (or another exact
range your CI uses), and ensure package.json's "engines" value reflects the CI
Node 22 configuration so running "node --test" behaves consistently across
environments.
| const elem = document.querySelector(`[name="${prop}"]`) | ||
| if (elem != null) { | ||
| elem.value = value | ||
| } |
There was a problem hiding this comment.
Avoid dynamic CSS selector parsing for BLE property names.
At Line 96, interpolating prop into querySelector can throw on unexpected characters and break update handling. Use getElementsByName instead.
🛡️ Safer element lookup
- const elem = document.querySelector(`[name="${prop}"]`)
- if (elem != null) {
- elem.value = value
- }
+ const elem = document.getElementsByName(prop).item(0)
+ if (elem != null && 'value' in elem) {
+ elem.value = value
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const elem = document.querySelector(`[name="${prop}"]`) | |
| if (elem != null) { | |
| elem.value = value | |
| } | |
| const elem = document.getElementsByName(prop).item(0) | |
| if (elem != null && 'value' in elem) { | |
| elem.value = value | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/preference/index.html` around lines 96 - 99, The current lookup uses
document.querySelector(`[name="${prop}"]`) which can throw on unsafe characters
in prop; replace this with a safer lookup using document.getElementsByName(prop)
(e.g., const elems = document.getElementsByName(prop); if (elems &&
elems.length) elems[0].value = value) so that the code around prop/elem
assignment uses getElementsByName instead of querySelector to avoid CSS selector
parsing errors.
| class="ble-disconnect-button" | ||
| id="ble-disconnect-button" | ||
| title="Disconnect" | ||
| data-i18n-title="preference.disconnect.title" | ||
| > | ||
| × | ||
| </div> |
There was a problem hiding this comment.
Use a real button for disconnect action (keyboard accessibility).
Line 142 uses a clickable <div> for an action control. This blocks native keyboard semantics and can prevent non-pointer users from disconnecting.
♿ Suggested semantic control fix
- <div
+ <button
+ type="button"
class="ble-disconnect-button"
id="ble-disconnect-button"
title="Disconnect"
data-i18n-title="preference.disconnect.title"
+ aria-label="Disconnect"
+ data-i18n-aria-label="preference.disconnect.title"
>
×
- </div>
+ </button>Also applies to: 117-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/preference/index.html` around lines 142 - 148, The disconnect control
currently implemented as a clickable <div> with id "ble-disconnect-button" and
class "ble-disconnect-button" must be replaced with a real <button> element
(type="button") to restore native keyboard semantics; update the element to
preserve attributes like id, class, title and data-i18n-title, add an accessible
name via aria-label if title text is not sufficient, and ensure any event
handlers bound to "ble-disconnect-button" continue to work; make the same
replacement for the other instance using the same id/class so both controls are
keyboard and screen-reader accessible.
1 participant
Summary
ja,en, andzh-Hanswith fallback behaviordata-i18nplus placeholder/title/aria-label attributesScope notes
tts.*, voice, and conversation language settingsTest plan
cd web && npm testgit diff --checkCloses #400
Summary by CodeRabbit
Release Notes
New Features
Documentation