Skip to content

fix(l10n): localize the dialog cancel button, and sweep the zh layouts - #317

Merged
ultraelectronica merged 2 commits into
moss-apps:mainfrom
gc233402-blip:fix/l10n-dialog-cancel
Oct 16, 2026
Merged

ultraelectronica merged 2 commits into
moss-apps:mainfrom
gc233402-blip:fix/l10n-dialog-cancel

Conversation

@gc233402-blip

Copy link
Copy Markdown
Contributor

Summary

Fixes an untranslated button that none of the string checks could see, and adds a
Chinese layout sweep that would have caught it.

The cancel button was never translated. FlickDialogs.confirm and
FlickDialogs.input declared their cancel label as a plain String defaulting to
the literal 'Cancel'. Call sites pass their own confirm label but not a cancel
one, so the English default is what shipped:

call sites in lib/ pass no cancelLabel
FlickDialogs.confirm 18 17
FlickDialogs.input 7 7

In Chinese the confirm dialog read [Cancel] [放弃] — an English button beside a
Chinese one — and every rename / name-your-playlist input dialog did the same.
Both parameters are now nullable and fall back to l10n.cancel. The one call
site that passes a label (logs_screen.dart, l10n.no) is unaffected.

confirmLabel keeps its 'Confirm' / 'Save' defaults on purpose: all 25 call
sites pass it explicitly, so making it required would break callers and an
existing test for no behavioural gain.

New: test/l10n/zh_layout_test.dart. Chinese is the risky locale — the app
was written in English — and test/widgets/mini_player_bar_test.dart already
sweeps width and text scale, but only in English. This reruns that sweep in zh
over the navigation bar (all nine buttons, not the three the default config
enables), the metadata editor, and FlickDialog's two-button row, at
320/375/600 by 1x/2x/3x, asserting nothing overflows and nothing throws. It also
pins the cancel fix above, and asserts the labels really are Chinese so the
sweeps cannot pass by testing English.

Related issue

none

Checklist

  • flutter analyze passes
  • flutter test passes
  • Rust changes: cargo test --manifest-path rust/Cargo.toml passes
  • Built and ran the app if the change touches a playback path (flutter run --flavor full)
  • Generated code regenerated and committed if bridge/drift/freezed models changed
  • CHANGELOG.md updated for user-facing changes
  • No secrets, keys, tokens, or machine-local files committed
  • One change per PR; no unrelated reformatting in the same diff
  • Hardware tested if the Rust audio engine or USB/DAC output changed

Notes on the unchecked boxes:

  • “One change per PR” is not ticked because this deliberately carries a fix
    and the test that pins it, as two commits. They are one subject — the cancel
    label — and the test file is also where the overflow sweep lives. Happy to
    split them if you would rather have the sweep separately.
  • Rust / hardware testing: no Rust, audio, or playback path is touched.
  • Generated code: no bridge/drift/freezed or ARB output changed; the test
    reads existing keys.
  • CHANGELOG.md: I was unsure whether an untranslated button counts as
    user-facing for the changelog. Say the word and I will add a line; I left it
    out rather than guess at the format.

Verification

flutter analyze: 29 issues, the same baseline as main.

flutter test: test/l10n/, test/widgets/flick_dialog_test.dart,
test/widgets/glass_bottom_sheet_test.dart and
test/features/songs/metadata_editor_ime_test.dart pass 29/29.

For the fix: reverting either default and re-running turns the new assertions
red, and prints the actual labels — [Cancel, 放弃] before, [取消, 放弃] after.

For the overflow gate: the sweeps pass on today's code, which is only worth
something if they can fail. Shrinking the swept width to 100 turns both the
navigation-bar and metadata-editor cases red with a RenderFlex overflowed
error. The navigation bar's overflow needs a specific combination to appear
(miniPlayer + separated + not collapsed); the sweep covers all four
collapsed/separated combinations, so that case is included.

Why not a golden file, since this is a layout test: Flutter's
LocalFileComparator has no per-platform variant, so an image written on Windows
does not match the one Ubuntu CI produces and a committed PNG would fail on every
run. Upstream already reached the same conclusion — the recap screen's
_snapshot writes PNGs only under --dart-define=REPLAY_SCREENSHOTS=true, as a
review artefact rather than a gate. This follows that precedent:
--dart-define=ZH_SCREENSHOTS=true writes build/zh_shots/*.png for a human,
and CI asserts instead.

Three things that made the first drafts green but meaningless, and are now fixed
in the file, in case they are useful for future widget tests here:

  1. MaterialApp.home receives tight constraints, so a SizedBox(width: 100)
    there is ignored and nothing overflows. The host wraps the child in Center.
  2. A widget test has no system fonts, so every glyph is a tofu box — with
    different metrics from the real typeface. GoogleSansFlex, NotoSansSC and
    both icon fonts are registered in setUpAll, and the host uses
    AppTheme.darkTheme so the theme's fontFamilyFallback applies.
  3. A dialog renders into the app's overlay, so a RepaintBoundary around the
    page body does not capture it; it wraps the MaterialApp instead.

One thing I found but did not change, because it is a separate decision and not
needed for this fix: the bundled subset still leaves the metadata editor's
inline-styled labels (fontFamily: 'GoogleSansFlex' with no fallback, as called
out for ProductSans in #307) without a CJK fallback, so those specific labels
render as tofu in the widget-test environment. On device they fall back through
the platform font.

FlickDialogs.confirm and FlickDialogs.input both declared their cancel label as
a plain String defaulting to the literal 'Cancel'. Call sites pass their own
confirm label but not a cancel one, so the English default was what every
translated app actually showed:

    confirm(): 18 call sites in lib/, 17 pass no cancelLabel
    input():    7 call sites in lib/,  7 pass no cancelLabel

In Chinese the confirm dialog therefore read [Cancel] [放弃] -- an English button
beside a Chinese one -- and the same held for every rename/name-your-playlist
input dialog.

Both parameters become nullable and fall back to l10n.cancel, which is the
active locale. Callers that pass a label keep working; logs_screen.dart is the
one that does.

confirmLabel keeps its 'Confirm' / 'Save' defaults: all 25 call sites pass it
explicitly, so making it required would break callers (and one existing test)
for no behavioural gain.

This is not something the ARB checks or the source guard can see, because it is
a Dart default rather than a translated string.
Adds test/l10n/zh_layout_test.dart. Two things it checks.

Overflow gate. Chinese is the risky locale, and the app was written in English.
test/widgets/mini_player_bar_test.dart already sweeps width and text scale, but
only in English, so this reruns the sweep in zh: the navigation bar with all
nine buttons, the metadata editor, and FlickDialog's two-button row, each at
320/375/600 logical pixels by 1x/2x/3x text scale, asserting nothing overflows
and nothing throws.

Why not a pixel golden: Flutter's LocalFileComparator has no per-platform
variant, so an image written on Windows does not match the one Ubuntu CI
produces. Upstream already reached the same conclusion -- the recap screen's
_snapshot writes PNGs only under --dart-define=REPLAY_SCREENSHOTS=true, as a
review artefact rather than a gate. This file follows that precedent:
--dart-define=ZH_SCREENSHOTS=true writes build/zh_shots/*.png for a human, and
CI asserts instead.

Cancel button. Pins the fix in the preceding commit: in zh the confirm dialog
and the input dialog must both show l10n.cancel, not 'Cancel'.

Notes from writing it, all three of which produced a green-but-meaningless test
first:

  * MaterialApp.home receives tight constraints, so a SizedBox(width: 100) there
    is ignored and nothing overflows. The host wraps the child in Center.
  * A widget test has no system fonts, so every glyph is a tofu box with
    different metrics than the real typeface. GoogleSansFlex, NotoSansSC and the
    two icon fonts are registered in setUpAll, and the host uses
    AppTheme.darkTheme so the theme's fontFamilyFallback applies.
  * A dialog renders into the app's overlay, so the RepaintBoundary has to wrap
    the MaterialApp rather than the page body.

test/l10n/ plus the dialog and IME suites: 29 passing. flutter analyze holds at
the 29-issue baseline.
ultraelectronica added a commit that referenced this pull request Oct 16, 2026
…and sweep the zh layouts (reviewed + verified)
@ultraelectronica
ultraelectronica merged commit 9fb6010 into moss-apps:main Oct 16, 2026
6 checks passed
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.

2 participants