Repository navigation
fix(l10n): localize the dialog cancel button, and sweep the zh layouts - #317
Merged
ultraelectronica merged 2 commits intoOct 16, 2026
Merged
Conversation
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
approved these changes
Oct 16, 2026
ultraelectronica
added a commit
that referenced
this pull request
Oct 16, 2026
…and sweep the zh layouts (reviewed + verified)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.confirmandFlickDialogs.inputdeclared their cancel label as a plainStringdefaulting tothe literal
'Cancel'. Call sites pass their own confirm label but not a cancelone, so the English default is what shipped:
lib/cancelLabelFlickDialogs.confirmFlickDialogs.inputIn Chinese the confirm dialog read
[Cancel] [放弃]— an English button beside aChinese 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 callsite that passes a label (
logs_screen.dart,l10n.no) is unaffected.confirmLabelkeeps its'Confirm'/'Save'defaults on purpose: all 25 callsites 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 appwas written in English — and
test/widgets/mini_player_bar_test.dartalreadysweeps 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, at320/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 analyzepassesflutter testpassescargo test --manifest-path rust/Cargo.tomlpassesflutter run --flavor full)CHANGELOG.mdupdated for user-facing changesNotes on the unchecked boxes:
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.
reads existing keys.
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 asmain.flutter test:test/l10n/,test/widgets/flick_dialog_test.dart,test/widgets/glass_bottom_sheet_test.dartandtest/features/songs/metadata_editor_ime_test.dartpass 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 overflowederror. The navigation bar's overflow needs a specific combination to appear
(
miniPlayer+separated+ notcollapsed); the sweep covers all fourcollapsed/separated combinations, so that case is included.
Why not a golden file, since this is a layout test: Flutter's
LocalFileComparatorhas no per-platform variant, so an image written on Windowsdoes 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
_snapshotwrites PNGs only under--dart-define=REPLAY_SCREENSHOTS=true, as areview artefact rather than a gate. This follows that precedent:
--dart-define=ZH_SCREENSHOTS=truewritesbuild/zh_shots/*.pngfor 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:
MaterialApp.homereceives tight constraints, so aSizedBox(width: 100)there is ignored and nothing overflows. The host wraps the child in
Center.different metrics from the real typeface.
GoogleSansFlex,NotoSansSCandboth icon fonts are registered in
setUpAll, and the host usesAppTheme.darkThemeso the theme'sfontFamilyFallbackapplies.RepaintBoundaryaround thepage body does not capture it; it wraps the
MaterialAppinstead.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 calledout for
ProductSansin #307) without a CJK fallback, so those specific labelsrender as tofu in the widget-test environment. On device they fall back through
the platform font.