fix(ui): localize the Astryx chrome adopted since #1795 - #2202
Merged
Conversation
apache#1795 moved the chat surfaces onto Astryx ChatLayout and deleted the `label={copy.jumpLatest}` that fed Maka's own scroll-to-bottom button, but never registered the replacement `@astryx.*` keys. Astryx ships no zh catalog, so the pill has been reading "New messages" in a Chinese UI ever since — and the same gap covers every other Astryx surface adopted without its copy. Audited all 219 shipped Astryx keys against the components Maka actually renders (JSX usage, not imports — the barrel re-export makes every component look reachable). 79 rendered keys had no override; this adds them, reusing existing entries where the string is identical rather than duplicating (form.clear, primitives.close/loading, selectPlaceholder). Excluded as dead config per this file's own rule: dateInput.dialogLabel and dateInput.placeholder — DateTimeInput references neither, and no standalone DateInput is rendered. The guard test listed `chat` as a surface Maka does not render, which is what let the regression through; it now guards lightbox only, and a new test pins the chat chrome. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
Astryx expands the pill only when new messages arrived, so the label reads as a notice rather than an action — but the copy this replaced (apache#1194's `jumpLatest`, dropped by apache#1795) said 跳到最新消息, and keeping the wording users already know beats a semantically tidier rename. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
`messages[key] ?? ''` coalesced a deleted override to the empty string, which holds no Latin letters and satisfied the translation check on its own — so the loop caught an English value but not a removed key. Verified both ways: deleting the send override now fails with 'missing override', and reverting its copy to English fails with 'untranslated'. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
The dead-config guard's surviving entry was as stale as the one apache#1795 invalidated: chat-turn.tsx opens image previews through useLightbox, which renders Astryx's Lightbox — with four English strings — while a JSX-tag scan for <Lightbox> reports the component unrendered. Add the four overrides (close reuses primitives.close) and retire the ban-list guard entirely: a list keyed to "what we render today" rots silently, and both of its entries had. Live surfaces are pinned by presence tests instead. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
The token remove button read 'Remove {label}' only because Astryx's
Token had no zh override; now that @astryx.token.remove is localized the
accessible name is 移除…, and the /^Remove / locator times out. The spec
already addresses every other control by its Chinese name.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
…fallback Review findings on apache#2202, all verified before fixing: - The astryx catalogue expanded SharedUiCopy, which `export *`s through the package barrel — violating the README's off-barrel convention for symbols with no cross-package consumer. It now lives in astryx-copy.ts, deliberately unexported. - The en mirror existed only as a diffable reference and was never applied (astryxMessageOverrides returns undefined for en); dropped rather than left to drift against upstream. - `a` renamed to `astryx`; duplicated presence/Latin-letter assertions extracted into assertChineseAstryxOverrides. - P1: the apache#642 streaming fallback rendered a bare ChatMessage, which resolves Astryx's shipped 'Message from {sender}' — an English aria-label in a Chinese tree. It now reuses LocalizedChatMessage with the same assistantAriaLabel as the TurnView path. The regression test pins the exact trigger: wait indicators with zero turns — passing a liveTurn instead projects a real turn and takes the localized path, which is how the first draft of the test passed against the bug. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
Contributor
|
Thorough fix — the zh catalog reads naturally, the E2E matcher change is the test-side half of the fix (suite runs zh;
Only the P1 blocks merge (and it's a few lines); everything else can be follow-ups. Nice work otherwise! 🚢 |
Maintainer review on apache#2202, each claim verified before acting: - chatSendButton.send/stop: ChatComposerInput never renders ChatSendButton — composer.tsx supplies its own send button. Pruned; the pinned test now anchors on chatToolCalls.error instead. - appShell.mobileNavigation: gated on shouldShowAutoToggle, and Maka passes breakpoint 'none' + hasToggle false. Pruned. - table.noData / table.filter.* / tableFiltering.filterByColumn: the usage table mounts only the rowHeader plugin and renders EmptyState before Table when rows are empty. Pruned. - New whole-map sweep: every override must target a key Astryx ships, hold no Latin outside {…}, and carry the same top-level ICU arguments as the en default — depth-aware extraction, since plural branch text ({result}) reads like an argument to a naive regex and would flag every zh string that drops an inapplicable plural. - Presence-helper comment reworded: the check reads the override map directly, so a deleted entry yields '' here; at runtime the same miss falls back to Astryx's shipped en catalog. Sweep red-validated both ways: a misspelled key fails membership, an English value fails the Latin check. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
Contributor
Author
|
Thanks for the pass — all five verified and addressed.
Suite at 326/326, lint clean. |
Contributor
|
Verified all the fixes on the current head — P1 (LocalizedChatMessage on the #642 fallback, regression test passes), all 8 dead overrides pruned, and the whole-map sweep (membership + no-Latin + ICU-arg parity) is a nice upgrade over my suggestion. |
6 tasks
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.
Problem
The scroll-to-bottom pill in the transcript reads "New messages" in a Chinese UI.
#1795moved the chat surfaces onto AstryxChatLayoutand deleted thelabel={copy.jumpLatest}that fed Maka's own scroll-to-bottom button.ChatLayoutsupplies its own label fromt('@astryx.chatLayout.newMessages'), but no matching entry was added toastryxMessageOverrides— and Astryx ships onlyen,fr-FRandpseudocatalogs. That file's own rule is that a slice adopting a new Astryx surface registers its@astryx.*keys in the same PR; this one did not, so the copy silently fell back to English.The same gap covers every other Astryx surface adopted without its copy, not just the pill.
Audit
Compared all 219 shipped Astryx keys against the components Maka actually renders. Reachability by import is misleading — the top-level barrel re-export makes all 219 keys look live — so the filter is JSX usage:
ChatLayout,Banner(25 call sites),Selector(15),TextInput(21),SideNav,TabList,Table,CommandPalette,DateTimeInput,Thumbnail,Token,MultiSelector,MoreMenu,DropdownMenu,Breadcrumbs,AppShell— plusLightbox, reached throughuseLightboxinchat-turn.tsx(image preview), which a JSX-tag scan misses.82 rendered keys had no override (78 from the JSX sweep + 4 Lightbox keys found after the guard's premise fell). This adds them, reusing existing catalogue entries where the string is identical rather than duplicating (
form.clear,form.selectPlaceholder,primitives.close,primitives.loading).Deliberately excluded as dead config:
@astryx.dateInput.dialogLabeland@astryx.dateInput.placeholder—DateTimeInputreferences neither, and no standaloneDateInputis rendered.The guard that let it through
markdown-body.test.tsasserted that no override key matches a ban list of surfaces Maka supposedly never renders. Both entries had rotted:chatstopped being true at #1795 (and the guard then actively blocked this fix), andlightboxwas never verified —useLightboxrenders it. A ban list keyed to "what we render today" goes stale silently, so it is removed entirely; presence tests pin the live surfaces (chat chrome + Lightbox) instead, asserting each key exists and holds no Latin letters.Also fixed while here (review findings)
ChatMessage, resolving Astryx's shippedMessage from {sender}. It now reusesLocalizedChatMessagewith the sameassistantAriaLabelas the normal path; a regression test pins the exact trigger (wait indicators with zero turns — aliveTurnprojects a real turn and takes the localized path instead).astryx-copy.tsinstead of expandingSharedUiCopy(whichexport *s through the barrel). The never-appliedenmirror is dropped rather than left to drift./^Remove /now uses its localized name/^移除/.Copy notes
chatLayout.newMessages→ 跳到最新消息 (kept from the pre-refactor(ui): migrate chat surfaces to Astryx layout #1795 wording users know, over a semantically tidier rename).commandPalette.resultCount: the zh string keeps{count, number}formatting and drops the English plural branch — Chinese does not inflect for number, so a plural clause would be dead config. ICU formatting applies to overrides (resolve.tsruns the formatter), so all{placeholder}s are preserved.token.remove→ 移除{label},composerDrawer.label→ 附加内容 (purpose-named; the English "Items" is too generic to transliterate).Validation
@maka/uisuite — 325/325 pass; typecheck andbiome lintcleanquote-companione2e passes locally; full monorepo build cleanCo-Authored-By: Claude noreply@anthropic.com
https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16