refactor(desktop): rebuild MCP page with Astryx - #2035
Conversation
9fc11c9 to
e9f1b7f
Compare
629da0b to
508dbae
Compare
508dbae to
3fa8908
Compare
3fa8908 to
fbd5c76
Compare
|
@Astro-Han When you have a moment, could you please review this PR? It is rebased on main, CI is green, and the scope remains limited to the MCP page Astryx redesign without Runtime, IPC, credential, catalog, or connection-lifecycle changes. The PR includes real-app before/after evidence, Storybook coverage, and MCP E2E verification. Thank you. |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the careful rebase and verification. The component choices are solid — TabList/Tab matches the skills page, Card+Item is the right shape for the market grid, and the Banner/EmptyState/Dialog usage is consistent. Two issues I'd like addressed before merge, plus a few nits.
1. Workspace width no longer aligns with the page header (regression)
mcp.css sets .maka-mcp-workspace to width: min(100%, 960px) while .maka-module-main-header stays at min(100%, 900px) (module-shell.css). At window widths ≥ ~1270px the workspace/card region extends ~30px beyond the header on each side — reproduced in the built app; the other module pages (e.g. Daily Review's .maka-daily-review-panel) pin content to 900px to match the header. Please revert to 900px — the header is shared across modules, so that's the low-risk fix.
2. Server description gets hard-clipped at compact widths
Around 700–500px window widths the .maka-mcp-server-description row overflows its container (measured 148px @ 700px, 348px @ 500px), and the new overflow: hidden on the server Card turns the previous visible bleed into hard clipping — no ellipsis. Item only auto-truncates string labels/descriptions; this one is a ReactNode carrying a <code> child. The compact E2E asserts main-overflow and centering but nothing about row-internal clipping. Please constrain the Item slot properly and add an assertion for it.
Nits (optional)
- The server list is a Card + hand-rolled
div role="listitem"+ Item + hand-written borders instead of AstryxList/ListItem.ListItemhas no children slot, so the error Banner/Collapsible can't ride it — fine to keep the current structure, but worth a comment explaining why. .maka-mcp-command-barcould carryrole="toolbar"+ aria-label like the Daily Review toolbar.- The market grid collapses to a single full-width column below ~920px window width (page height ~1900px+ with the current catalog) — pre-existing on main (min 320px vs 300px now), so not blocking, but it sits inside the PR body's "responsive MCP layouts" claim.
fbd5c76 to
2f7c0e6
Compare
|
Thanks for the detailed review — I rebased onto
Local evidence on this head:
The PR is Draft again while exact-head CI reruns. The full local |
|
LGTM, Merging! |
Closes #1900
Summary
Banner,Card,Item, andIconButtonprimitivesScope boundary
This is a renderer-only product refactor plus test and visual evidence:
The
window.maka.mcpcall sites are unchanged. The only files outside renderer, Storybook, and E2E assertions are test-fixture comments and the screenshots below.Before / after
Marketplace
Configured servers
All four images use the same real Electron
module-mcpfixture at 1280x900. The after images were recaptured from the exact current head after restoring the shared 900px content boundary.Storybook coverage
These states cover light and dark themes plus the applicable wide, compact, and minimum-width viewports.
Verification
Current head
2f7c0e66dis rebased ontomain@512efea09and is mergeable without conflicts.Local verification on the exact current head:
npm run format:check— 1,491 filesnpm run lint— 2,427 filesnpm run typechecknpm testcompleted all affected production workspaces. Three pre-existing stderr-contract assertions fail under the local Node 22 runtime becausenode:sqliteemits its ExperimentalWarning (Storage: 1, CLI: 2); this PR has no Storage, CLI, Runtime, Core, or dependency diff. The same three assertions reproduce when those workspaces run alone.GitHub CI is green on the exact current head:
changes,typecheck,test_workspaces, both E2E shards,alignment_audit,storybook, aggregatetest, and aggregatee2eall passed; the unaffected Runtime Host and Headless lanes were skipped by the impact plan.The two unrelated smoke-harness false failures discovered during UI regression are tracked separately in #2096 and Draft PR #2101.
Overlap audit
No open PR modifies this MCP renderer scope. The follow-up in #2101 is limited to test infrastructure and has no production overlap with this PR.