Skip to content

fix(pad): show detected language in settings dropdown (#7925)#7928

Merged
JohnMcLear merged 2 commits into
developfrom
fix/7925-language-dropdown-detected
Jun 9, 2026
Merged

fix(pad): show detected language in settings dropdown (#7925)#7928
JohnMcLear merged 2 commits into
developfrom
fix/7925-language-dropdown-detected

Conversation

@JohnMcLear

Copy link
Copy Markdown
Member

Problem

Fixes #7925 (follow-up to #7586).

The pad UI correctly auto-detects the browser language and renders in it (e.g. German), but the Language dropdown in Settings still shows English.

Root cause

Two code paths set the dropdown value and disagree when the language is auto-detected (no language cookie, no pad-wide lang):

  • pad_editor.ts (on html10n's localized event) correctly sets both dropdowns to html10n.getLanguage() (e.g. de).
  • pad.ts refreshMyViewControls() / refreshPadSettingsControls() set them to effectiveOptions.lang || 'en' / padOptions.lang || 'en'. When nothing is explicitly chosen, getMyViewOverrides() deletes overrides.lang, so the value is undefined and the dropdown falls back to hardcoded 'en'. These refreshes run after the localized handler (during handshake), clobbering the correct value back to English.

Fix

Fall back to the detected language before 'en':

$('#languagemenu').val(effectiveOptions.lang || html10n.getLanguage() || 'en');
$('#padsettings-languagemenu').val(padOptions.lang || html10n.getLanguage() || 'en');

getLanguage() returns the matched lowercase locale key, which matches the <option value> keys built from the available locales — so the right option is selected. An explicit user choice (cookie/pad option) still takes precedence.

Testing

Added a regression test that loads a pad under a de-DE browser locale and asserts the dropdown reads "Deutsch" with no manual selection.

RED-green verified against a running server:

🤖 Generated with Claude Code

When the UI language was auto-detected from the browser (no language
cookie and no pad-wide lang set), refreshMyViewControls() and
refreshPadSettingsControls() set the language dropdown to
`<option>.lang || 'en'`. Since the detected language lives only in
html10n (not in padOptions/effectiveOptions), the value was undefined
and the dropdown fell back to hardcoded English — even though the pad
UI itself rendered correctly in the detected language.

Fall back to html10n.getLanguage() before 'en' so the dropdown reflects
the language actually rendered. getLanguage() returns the matched
lowercase locale key, which matches the <option value> keys.

Adds a regression test that loads a pad under a de-DE browser locale and
asserts the dropdown reads "Deutsch" without any manual selection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)

Grey Divider


Remediation recommended

1. Unasserted locale check ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new regression test checks whether the Bold button tooltip is German via locator.evaluate(), but
it never asserts the boolean result, so the test will not fail if the UI is not actually rendered in
German. This weakens the regression coverage and can allow language-detection regressions to slip
through.
Code

src/tests/frontend-new/specs/language.spec.ts[R104-107]

+      // The toolbar should have rendered in German (detection works) ...
+      await page.locator('.buttonicon-bold').evaluate((el) =>
+          el.parentElement!.title === 'Fett (Strg-B)')
+
Evidence
The test currently computes a boolean but never asserts it, so it cannot fail due to a wrong
tooltip. The repo’s other Playwright tests demonstrate the intended pattern: asserting the return
value of evaluate() with expect().

src/tests/frontend-new/specs/language.spec.ts[94-107]
src/tests/frontend-new/admin-spec/adminsettings.spec.ts[309-312]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The regression test `dropdown reflects the browser-detected language` calls `locator.evaluate()` with a boolean expression, but the returned boolean is ignored. This makes the “UI rendered in German” verification non-functional.
### Issue Context
Other frontend tests assert `evaluate()` results via `expect(...)`, so this test should do the same (or use a more direct Playwright assertion like `toHaveAttribute`).
### Fix Focus Areas
- src/tests/frontend-new/specs/language.spec.ts[104-107]
### Suggested change
Replace the unused `await locator.evaluate(...)` call with an assertion, for example:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix language dropdown to reflect auto-detected UI language
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

Walkthroughs

User Description

Problem

Fixes #7925 (follow-up to #7586).

The pad UI correctly auto-detects the browser language and renders in it (e.g. German), but the Language dropdown in Settings still shows English.

Root cause

Two code paths set the dropdown value and disagree when the language is auto-detected (no language cookie, no pad-wide lang):

  • pad_editor.ts (on html10n's localized event) correctly sets both dropdowns to html10n.getLanguage() (e.g. de).
  • pad.ts refreshMyViewControls() / refreshPadSettingsControls() set them to effectiveOptions.lang || 'en' / padOptions.lang || 'en'. When nothing is explicitly chosen, getMyViewOverrides() deletes overrides.lang, so the value is undefined and the dropdown falls back to hardcoded 'en'. These refreshes run after the localized handler (during handshake), clobbering the correct value back to English.

Fix

Fall back to the detected language before 'en':

$('#languagemenu').val(effectiveOptions.lang || html10n.getLanguage() || 'en');
$('#padsettings-languagemenu').val(padOptions.lang || html10n.getLanguage() || 'en');

getLanguage() returns the matched lowercase locale key, which matches the <option value> keys built from the available locales — so the right option is selected. An explicit user choice (cookie/pad option) still takes precedence.

Testing

Added a regression test that loads a pad under a de-DE browser locale and asserts the dropdown reads "Deutsch" with no manual selection.

RED-green verified against a running server:

🤖 Generated with Claude Code

AI Description
• Make language dropdowns fall back to html10n-detected language before defaulting to English.
• Prevent handshake-time refresh logic from clobbering the correct auto-detected language.
• Add frontend regression test covering browser-locale auto-detection with no explicit language set.
Diagram
graph TD
  A{{"Browser locale"}} --> B(["html10n"]) --> C["pad.ts refresh"] --> D["Language dropdowns"]
  E["language.spec.ts"] --> F["Playwright browser context"] --> A

  subgraph Legend
    direction LR
    _ext{{"External input"}} ~~~ _svc(["Client service"]) ~~~ _mod["Module/file"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Persist detected language into effectiveOptions/padOptions
  • ➕ Single source of truth for language across refresh paths
  • ➕ Reduces repeated html10n lookups at UI update time
  • ➖ Riskier behavior change (could affect server/client option semantics)
  • ➖ Needs careful handling to avoid treating detection as an explicit user choice
2. Unify refresh logic to only set dropdowns from one event path
  • ➕ Eliminates clobbering by design (one writer to the dropdown state)
  • ➕ Potentially clearer lifecycle ordering
  • ➖ More refactor/coordination across handshake/localization timing
  • ➖ Higher regression risk for settings refresh behavior

Recommendation: Keep the PR’s lightweight fallback to html10n.getLanguage() as the best near-term fix: it resolves the clobbering issue without changing option persistence semantics, and it’s protected by a regression test. The more invasive alternatives (persisting detection or refactoring refresh ownership) can be considered later if additional language-state inconsistencies emerge.

Grey Divider

File Changes

Bug fix (1)
pad.ts Fallback to html10n detected language for both language selectors +8/-2

Fallback to html10n detected language for both language selectors

• Updates both the pad settings and view settings language dropdowns to prefer an explicitly configured language, otherwise fall back to html10n.getLanguage() before defaulting to 'en'. This prevents post-localization refresh logic from resetting the dropdown to English when the UI was auto-detected into another language.

src/static/js/pad.ts


Tests (1)
language.spec.ts Add regression test for browser-locale detected language in dropdown +23/-0

Add regression test for browser-locale detected language in dropdown

• Adds a Playwright test that sets the browser locale to de-DE, clears cookies, opens a new pad, and verifies the UI is rendered in German and the language dropdown shows 'Deutsch' without manual selection.

src/tests/frontend-new/specs/language.spec.ts


Grey Divider

Qodo Logo

The detection-works precondition computed a boolean via locator.evaluate()
but never asserted it, so it could not fail. Assert the bold button's
parent title is the German "Fett (Strg-B)" with toHaveAttribute instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@JohnMcLear

Copy link
Copy Markdown
Member Author

Thanks @qodo — good catch. The "detection works" check computed a boolean via locator.evaluate() but never asserted it. Fixed in 1890c31 by asserting the bold button's parent title is the German Fett (Strg-B) via toHaveAttribute, so the precondition can now actually fail. Re-ran language.spec.ts (chromium) locally — all 5 pass.

@JohnMcLear JohnMcLear merged commit 5b146f0 into develop Jun 9, 2026
33 checks passed
@JohnMcLear JohnMcLear deleted the fix/7925-language-dropdown-detected branch June 9, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

language dropdown shows English instead of detected language

1 participant