Skip to content

refactor(ui): migrate chat surfaces to Astryx layout - #1795

Merged
Astro-Han merged 5 commits into
mainfrom
refactor/astryx-chat-surface-migration
Aug 1, 2026
Merged

refactor(ui): migrate chat surfaces to Astryx layout#1795
Astro-Han merged 5 commits into
mainfrom
refactor/astryx-chat-surface-migration

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace Maka-owned chat scrolling, pinned-bottom tracking, dock, and jump-to-bottom UI with the Astryx ChatLayout product primitive through a single ChatSurfaceLayout seam.
  • Migrate the desktop chat shell, quote companion, and attachment stories onto that shared layout; remove the superseded overlay-scroll and pinned-bottom implementations and dependency.
  • Keep ChatComposerInput unchanged so composer behavior and capabilities are preserved.
  • Keep Graph/Plan execution status in the fixed dock above the composer while only transcript content scrolls.
  • Apply the flex contract from upstream Astryx fix(ui): expose archived conversations in the sidebar #2573 as a temporary compatibility rule because published @astryxdesign/core@0.2.0 still overflows by the dock height.

The migration removes 1,149 lines while adding 781, leaving one chat layout ownership path instead of parallel Maka and Astryx implementations.

Verification

  • npm --workspace @maka/ui test — 258 tests passed
  • npm run typecheck in apps/desktop — passed
  • npx playwright test --config e2e/playwright.config.ts e2e/scroll-geometry.spec.ts e2e/sidebar-geometry.spec.ts --workers=1 — 9 tests passed
  • npm run build:renderer in apps/desktop — passed
  • npm run build-storybook in apps/desktop — passed
  • Empty and short transcripts have zero phantom scroll range
  • Long transcript final turn is constrained to the intended dock gap with both lower and upper bounds
  • Graph status remains fixed above the composer while a 24-turn transcript scrolls; the regression test moved 30,226.5px before the fix and at most 1px after it

Review focus

  • Confirm ChatLayout is the sole owner of chat scrolling and dock behavior and no superseded path remains.
  • Confirm Graph/Plan status belongs to the dock while transcript content alone belongs to the message area.
  • Confirm the Astryx fix(ui): expose archived conversations in the sidebar #2573 compatibility selectors match the published DOM contract and are safe to delete after upgrading to a release containing the upstream fix.
  • Confirm the geometry tests assert user-visible boundaries rather than implementation-specific source shape.

@Astro-Han
Astro-Han marked this pull request as ready for review August 1, 2026 10:39
@Astro-Han
Astro-Han merged commit 3e82140 into main Aug 1, 2026
4 checks passed
@Astro-Han
Astro-Han deleted the refactor/astryx-chat-surface-migration branch August 1, 2026 10:40
GabrielDrapor added a commit to GabrielDrapor/maka-agent that referenced this pull request Aug 5, 2026
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
GabrielDrapor added a commit to GabrielDrapor/maka-agent that referenced this pull request Aug 5, 2026
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
GabrielDrapor added a commit to GabrielDrapor/maka-agent that referenced this pull request Aug 5, 2026
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
Astro-Han pushed a commit that referenced this pull request Aug 5, 2026
* fix(ui): localize the Astryx chrome adopted since #1795

#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

* fix(ui): keep 跳到最新消息 for the scroll-to-bottom pill

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
(#1194's `jumpLatest`, dropped by #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

* test(ui): assert the pinned chat overrides exist before checking them

`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

* fix(ui): localize the Lightbox reached via useLightbox

The dead-config guard's surviving entry was as stale as the one #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

* test(e2e): match the quote token remove button by its localized name

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

* fix(ui): address review — off-barrel Astryx copy, localize #642 fallback

Review findings on #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 #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

* fix(ui): prune dead overrides, sweep the whole zh map in tests

Maintainer review on #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

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

1 participant