Rework the translation browser, and catch placeholder drift in validate - #95
Merged
Merged
Conversation
…root The sidebar tree rendered fully expanded and could never be closed, because `loaded` doubled as the expansion flag and the API indexes at depth Infinity — so every folder arrived loaded on the first payload. Expansion now has its own state and the tree starts collapsed. - Chevron toggles a folder; the row selects and opens but never closes, so drilling down stays one click while the chevron gives precise control. - ArrowRight opens the focused row, ArrowLeft closes it. The twisty is not a tab stop, per the ARIA tree pattern. - A root row stands for the collection itself: it lists every resource across every folder, owns the expand/collapse-all toggle, and accepts folder drops so a nested folder can be moved back out to the top level. - Filtering opens the branches holding matches and restores the previous expansion when cleared. - Expansion follows its folders: pruned on delete, rebased on move, and the ancestors of the selection are always revealed. - Drops the redundant per-click subtree refetch, which the first payload had already delivered in full. The API's empty-path branch returned early before honouring `includeNested`, so the root could not list resources recursively. Both branches now share one mapping path, which also restores `inheritedTags` on nested resources. An empty `currentFolderPath` used to mean "no folder selected", which the root row makes a real selection. The content area's "select a folder" card covered even search results, and the add-translation button stayed disabled; both gates are gone, along with the now-unused `browser.selectFolderFirst` resource. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The key and locale count chips rendered their raw keys in every non-English locale. `keysText` and `localesText` called `transloco.translate()` inside a computed; `LocaleService` sets the active language synchronously in its constructor, so the computed first ran while the locale file was still in flight, returned the key, and had no remaining dependency to invalidate it. Resolve both through `TranslocoPipe` in the template instead, which re-renders on load, and translate the two ICU strings into the five non-base locales with the plural categories each language actually uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The translation editor now renders the status lifecycle through the localized resources rather than the raw enum, which exposed that `stale` was translated in the wrong sense in four locales: de "Abgestanden" (of beer going flat), ru "Залежалый" (of unsold goods), es "Duro" (of bread) and fr-ca "Vicié" (of foul air). None is the software sense of an out-of-date translation. Also corrects, in the same pass: - `saving` in es and fr-ca, which used the money sense on the submit button - `error.createFailed` in fr-ca, which said the translation had failed - `conflict.chooseDifferentKey` in de, which used *Taste*, a keyboard key - `keyHint` and `keyPatternError` in es and fr-ca, which named a tab label that does not exist - `similarTranslations.showMoreX` in de, fr-ca, ja and ru, one untranslated and three with the placeholder stranded by English word order - `localeTranslationX` in es and fr-ca, which used English adjective order Drops `otherLocalesToggleX`, referenced by nothing, across all six locales and its stub in the testing module. Clears the 20 stale and 2 new entries in the dialog scope, whose values still matched their base. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the gaps a design critique found between the dialog and the promises the product makes elsewhere. Editing safety: - Take ownership of `disableClose` in the component rather than trusting each call site, and route Escape through `dialogRef.keydownEvents()` instead of a window listener that fired through any dialog stacked above it. Confirm before discarding, comparing against captured baselines for the form, the selected folder and the tag list, two of which live outside the form. - Make the key readonly while editing, so a rename is impossible rather than rejected after a round trip, and drop the now unreachable rejection. - Leave Save enabled and diagnose on click: mark the fields touched, jump to the offending tab, focus the control, and mark the tab with an error dot. Keys: - Accept the dot-delimited syntax the product defines. Pasting or typing `apps.common.buttons.ok` now splits at the last dot, sets the folder and keeps the leaf, with the location pill flashing and a live region announcing where the prefix went. The pattern validator stays as the backstop. Auto-translate moves out of the dialog. It persisted to disk on click from inside an unsaved form, so the discard prompt then offered to lose changes that were already committed. It remains available on the translation list item. Layout: - Give the dialog a full-screen treatment below 640px, replacing a 312px column, and size the tab content by viewport instead of a fixed 570px that left 350px empty under the folder tree. - Size form-field subscripts dynamically. German hint text overflowed its 22px box by up to 60px and painted over the base locale card. - Keep the tab strip from paginating at narrow widths, where it scrolled the active tab out of view with nothing to say which was selected. Also renames the read-only banner's colour to the token that exists, adds `--color-warning-text` for the contrast the banner needs on parchment, resolves locale names through `Intl.DisplayNames` while keeping the raw code visible, and extends the reduced-motion guards to the folder tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The toolbar's six controls were assembled from two different control families: the locale and status filters kept Material's own 32px height, greyscale outline and rgba ink, while the icon buttons beside them used the token set. A uniform 12px gap ran between all six, so search, filters, sort, the primary action and the density toggle read as one flat row with no grouping. Both filter triggers now share a %toolbar-filter-trigger placeholder on the same 40px box, warm border and hover/focus treatment as the icon buttons. They are plain buttons rather than mat-stroked-button, whose host styles beat the token styles on height and outline. Zones are separated by a hairline rather than by more of the same gap, and the two filters sit closer to each other than to anything else. Also: - The placeholder in the shared search input cleared only 4.2:1; add --color-text-placeholder, dark enough to read in both themes. - Add --color-text-on-primary for ink on the primary fill. The fill is the same light coral in both themes, so tying its label to --color-text-primary would drop it to 2:1 under dark. - Give the locale filter a `translate` icon; it and the status filter both carried `filter_list`, which told the user nothing about which menu they were opening. Fixes a horizontal overflow that took the Add Translation button off screen on narrow windows. The cause was the app shell, not the toolbar: .app-container's implicit `auto` grid column sized itself to the widest row's min-content width and stretched the whole page past the viewport. With minmax(0, 1fr) and min-width: 0 on the content pane, the toolbar reflows against its real width — search takes its own line under 760px, and the primary action goes icon-only under 640px. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The browser's translation item reported state it could not back up: locales
were hidden with no way to know they existed, the compact row showed a value
without naming its locale, and the status spine was hardcoded English on a
second, unthemed palette.
Data honesty
- Collapsed items render the first four locale rows and the control names the
remainder ("7 more locales"), replacing a fade gradient over a nested
scroller that hid rows silently. Removes the observer, sentinel and the extra
per-item tab stop the scroller created.
- The compact row carries a locale badge (status icon + code), plus a "source"
tag when it falls back to the base locale, so untranslated and finished rows
can no longer look alike.
Readability
- The key chip tooltips the untruncated key and names it in its accessible
name; the old aria-label overrode the chip contents, so screen readers got
neither the full key nor the truncated one.
- Keys wrap at their own word boundaries via <wbr> at separators and camelCase
humps, instead of being sliced mid-token by word-break: break-all.
- Narrow items stack each locale's label above its value (container query), so
the value gets the full card width instead of a 115px column.
Status spine
- Adds --color-status-* tokens with per-surface light/dark values, replacing
five Tailwind hexes that disagreed with the watercolor palette used by the
rows beneath them. `new` and `stale` no longer share a color.
- Rollup labels and accessible name go through Transloco; the breakdown is
built from pluralized resources and joined with Intl.ListFormat.
- The rollup trigger is a real button rather than div[role="button"].
Contrast (light theme, measured against the card)
- base/compact value 2.68:1 -> 5.05:1 via a new --color-primary-text
- status icons 1.84-3.15:1 -> 3.24-3.78:1
- "no translation" placeholder and source tag 2.14:1 -> 6.21:1 / 4.66:1
Also
- statusBreakdown() called translate() inside a computed with no language
dependency, froze on a language switch, and emitted raw status keys; it now
shares the reactive helper in shared/i18n.
- Virtual scroll itemSize under-estimated full items by 41px; retuned from
measurement. It remains an estimate: CDK's fixed-size strategy cannot model
the conditional comment row or the stacked narrow layout.
- Aligns the tracker's tsconfig lib with its es2022 target for Intl.ListFormat.
- Drops the orphaned browser.toggleExpansion resource and the unused
TruncateKeyPipe, moving truncateKey to shared/utils.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cache held a single collection, so a request naming a different one evicted whatever was there. Two Tracker tabs on different collections therefore evicted each other on every 2s `/cache/status` poll: neither ever reached READY, so neither poll ever terminated and the server re-indexed continuously. Collections now live in a Map capped at LINGO_TRACKER_MAX_CACHED_ COLLECTIONS (default 4) with least-recently-used eviction, which keeps the memory ceiling explicit. An entry that is still INDEXING is never evicted — discarding in-flight work would strand the request that started it — so the map may overflow briefly when every entry is busy. LRU order uses a monotonic counter rather than a clock, because several collections can be touched inside the same millisecond. Cache invalidation is now scoped to the collection that was written. Previously `clearCache()` wiped whatever was cached, so a write against one collection forced a re-index of an unrelated one somebody else was viewing. Cross-collection moves clear the source and every destination they touched; a cross-collection folder move skips the incremental update, which only knows how to relocate within a single tree. Fingerprint, revalidation throttle stamp and the deferred refresh timer move onto each entry — a single global throttle let a read of one collection postpone another's staleness check. Indexing results are now matched by entry identity, so a clear during a load cannot write stale results onto the replacement entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Compact density dropped the base value and led with a translation, which handed a developer scanning an eleven-locale collection a script they could not read. The base value is the one string on the row they already know — they wrote it, and it is what they arrived looking for — so it now identifies the row and the selected locale's value sits beside it, source and translation judged against each other in one horizontal glance. Where there is no base locale at all, the shown locale becomes the identity and the row says the pick was automatic. The row is one line again: 38px measured in the browser, 54px where a coarse pointer needs the 44px control floor. Full density gains the source as the first row of the locale grid rather than a heading above it. A translation can only be judged against the source when the two share a baseline, a measure and a type size. A value byte-identical to the base is now called out on the row. Status metadata cannot catch this — copying the source into a locale produces a perfectly valid `translated` status — so the chip was reporting finished work on a string nobody had touched. Read-only collections refuse deletion at every entry point: the store method, the keyboard shortcut, and the leading glyph, which shows a lock in place of the grab handle rather than an affordance that lies. The delete shortcut previously opened a confirmation the API would reject only after the user had committed to it. Also guards the icon flip behind prefers-reduced-motion and routes the inherited-tag tooltip through transloco. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ring's centre reported `priority_high` whenever either `new` or `stale` was above zero. Those are the two states a translator triages differently, and the only thing left separating them was two arcs about 20 degrees apart at a similar brightness — one colour to a reader with deuteranopia. The centre now names one of five states. `new` and `stale` each take the glyph the tooltip already uses for them, and the disc border takes the matching hue, so the distinction is carried twice and neither time by colour alone. `mixed` keeps `priority_high`, which is the one case where merging them is honest. The accessible name is unchanged; it already read the full breakdown. Two further defects from the same critique: - A locale column whose rows all share a status collapses to one chip carrying the count. Eleven identical `translated` chips are a pattern with nothing to find in it. A card with mixed statuses keeps its per-row chips, which is the case where the column is worth reading. - The comment band reserved 49px for a 32px control, because the expand button carried a 44px touch floor and sets the row height alone when the item has no comment. The floor now applies under a coarse pointer only. Measured 37px. Drops `hasIssues`, `isAllTranslated` and `translatedLike`, all now unused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Inverse text on `--color-primary` measured 3.1:1. The floor for body text is 4.5:1, and this link is body text — the fact that it appears only on focus makes it harder to notice, not exempt. `--color-primary-active` is the darkest step of the same ramp and is tuned in both themes: 5.08:1 light, 5.39:1 dark. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment is the only sentence on a full-density card that a person wrote — everything above it is derived from the files — and it was set as the least important thing on it: 14px italic in secondary ink, running unbroken past 130 characters on a wide card, behind a literal "Comment:" prefix repeated on every row. It is its own block now. The text is roman, capped at 72ch, and led by a `sticky_note_2` glyph on the locale-code column, so the note hangs off the same left edge as the rows above it. No fill and no rule: a fill quiet enough for this surface is a shade off the surface itself, and a rule adds a second vertical line to a card whose only other one is the source divider. The prefix is now `.sr-only` rather than deleted — the glyph names the block for sighted users, and the word still names it for everyone else, as a translated token rather than a hardcoded aria-label. The icon's box is the height of the text's first line with the glyph centred inside it. Sized to the glyph instead, it top-aligned to the grid row while the text sat lower in its taller line box, and read as floating above the sentence. The expand control takes its own row, and the band's top rule goes with the old layout that needed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The status filter reported its state as four 16px glyphs behind a Material menu, so the one control answering "what still needs work?" was the quietest thing in the row — and reading it cost a click. Replace the dropdown with an always-visible rail: one toggle per status plus a leading "Needs work" shortcut selecting new + stale together, which was previously two clicks inside the menu. Each toggle carries the number of resources it would leave on screen, which is the question a user is asking before they click it. Colour comes from the existing status chip tokens, so the filter and the rows it filters are painted from one spine and the change costs no new visual language. Counts come from a new statusCounts / needsWorkCount pair in the store, routed through the same matchesAnyStatus predicate that sortedTranslations filters with, so a count can never promise a row the list then declines to show. They count resources rather than status cells, and needsWorkCount is a union: a resource that is stale in German and new in Japanese is one row, not two. Also: - Rebalance the toolbar wrap points around the rail, now the widest zone and the one that grows most in German and Russian; it must be able to shrink and wrap internally or it pushes the primary action off the edge of the pane. - Drop statusFilterText and isShowingAllStatuses, whose only consumer was the removed trigger and which held hardcoded English in a localized UI. - Unselected toggles use --color-text-placeholder rather than --color-text-tertiary, a 2.5:1 decorative tint that is not safe for a label with a number in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Auto-translation had translated the interpolation placeholder itself along with
the surrounding prose, so four aria-labels silently never interpolated in any
non-English locale:
folderNode.folderAriaLabelX {{ Name }} {{ nombre }} {{ nom }} {{ 名前 }} {{ имя }}
folderNode.deleteFolderAriaLabelX {{ Name }} {{ nombre }} {{ nom }} {{ 名前 }} {{ имя }}
folderPicker.selectorAriaLabelX {{ Pfad }} {{ ruta }} {{ chemin }} {{ パス }} {{ путь }}
localeFilter.ariaLabelX {{ texto }} {{ texte }} {{ テキスト }} {{ текст }}
Nothing raises on this: the placeholder is simply dropped from the rendered
string, so a screen reader announces "Ordner" with no folder name. German was
broken in the same way as Japanese — argument names are case-sensitive, so
{{ Name }} fails exactly as {{ 名前 }} does while looking entirely correct,
which is presumably how it survived review.
Replace only the placeholder token in each value; every translator's wording is
left untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…aceholders A translation can be approved by a reviewer and compile cleanly while interpolating an argument no caller ever passes, because a machine translator renamed it along with the prose. ICU renders the missing argument as an empty string rather than raising, so neither the status gate nor the ICU compile pass sees it and the defect reaches production looking like ordinary text. Add it as a third pass alongside the two existing ones, which answer genuinely different questions: status asks whether a human approved the wording, ICU asks whether the value compiles, and this asks whether it interpolates what the caller supplies. Arguments are collected with @messageformat/parser rather than a regex, which matters for correctness: the pass counts arguments nested inside plural branches and the argument a plural switches on, while excluding branch selectors and the octothorpe. A Russian one/few/other against an English =1/other is therefore not flagged — categories are ICU vocabulary and are meant to differ between locales. The comparison distinguishes an unparseable value from one that genuinely has no arguments; without that, every malformed translation would look as though it had dropped all its placeholders and would name the wrong repair, when the ICU pass already reports the real defect. On by default, with --skip-placeholders to opt out. It is independent of --skip-icu, and a test asserts that skipping one does not quietly skip the other. Pure comparison lives in domain, so it stays free of Node dependencies; the file-walking pass lives in core; the flag lives in the CLI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ixtures
ICU treats a single quote as an escape character, so the base value
File '{filename}' was overwritten
declares no argument at all — it renders the literal text "File {filename} was
overwritten". The English source was the broken side here; translations using
their own quotation marks, as German's „{filename}“ did, were correct all
along. Escape the quotes as '' so the visible text is unchanged and the
placeholder comes back.
Found by the placeholder check added in the previous commit.
Note that editing a base value re-translates every locale, so the translations
for these two entries were regenerated rather than preserved, and the CLI's
writer reformatted the rest of the file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Turns auto-translation off and drops the apiKeyEnv entry, so resource edits no longer call Google Translate. This also removes the surprise behind the previous commit: editing a base value with translation enabled regenerates every locale, which silently replaced existing translations in the playground fixtures rather than leaving them alone. With this off, a base-value edit changes only the base value and leaves the translations to be updated deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🎉 This PR is included in version 0.18.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
2 participants
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.
Sixteen commits reworking the Tracker's browsing surface, plus a new validation check and the i18n bugs it uncovered. Each commit is self-contained and carries its own reasoning; this is a summary of the themes.
Navigation and layout
Reading a translation at a glance
newandstaleare no longer conflated — one is work not started, the other is work gone out of date.Status as a first-class control
The status filter reported its state as four 16px glyphs behind a Material menu, so the one control answering "what still needs work?" was the quietest thing in the toolbar, and reading it cost a click.
It is now an always-visible rail: one toggle per status plus a leading Needs work shortcut selecting
new + staletogether (previously two clicks inside the menu). Each toggle carries the number of resources it would leave on screen.Counts come from a new
statusCounts/needsWorkCountpair routed through the same predicatesortedTranslationsfilters with, so a count can never promise a row the list then declines to show — there's a test asserting exactly that. They count resources rather than status cells, andneedsWorkCountis a union: a resource stale in German and new in Japanese is one row, not two.A new validation pass, and the bugs it found
lingo-tracker validatenow checks that every translation interpolates the same placeholders as its base value.This defect is silent by construction. ICU renders a missing argument as an empty string rather than raising, so a translation can be reviewer-approved and compile cleanly while interpolating an argument no caller passes. Machine translation produces it routinely, translating the argument name along with the prose.
It found two real classes of bug:
Renamed placeholders in the Tracker's own UI. Four aria-labels never interpolated in any non-English locale —
{{ nombre }},{{ 名前 }},{{ имя }}. German was broken in the same way ({{ Name }}), since argument names are case-sensitive, which is presumably how it survived review. Fixed by replacing only the placeholder token; translator wording untouched.ICU quote escaping in the playground fixtures.
File '{filename}' was overwrittendeclares no argument at all — ICU treats'…'as an escape, so it renders the literal text. Here the English source was the broken side; translations using their own quotation marks were correct all along.Arguments are collected with
@messageformat/parserrather than a regex, which matters: the pass counts arguments nested in plural branches and the argument a plural switches on, while excluding branch selectors and#. A Russianone/few/otheragainst an English=1/otheris not flagged — categories are ICU vocabulary and are meant to differ.On by default;
--skip-placeholdersopts out, independently of--skip-icu.API
Collections are cached independently rather than one at a time.
Review notes
chore: disable auto-translationturns off Google Translate in.lingo-tracker.json. Worth a deliberate look:add-resourceandedit-resource --base-valuewill no longer populate other locales. It's also what would have prevented the fixture churn in the commit before it, where a base-value edit silently regenerated every locale.validateonly checks the globalconfig.locales.mockDesignSystemdeclares 11 locales includingar,ko,nl,pt-br,sv— none of those are validated by any pass. Pre-existing gap, not introduced here.Verification
lint,test,buildandtypecheckgreen across all 8 projects.validatereports zero placeholder failures and zero ICU failures across all four collections.🤖 Generated with Claude Code