refactor(webui): adopt every shared @lablup/ui-common component - #1924
Merged
Merged
Conversation
The gallery tooltip is the only call site. These tests drive that call site rather than the component API, so the same assertions run before and after the props change from label to content. They require the trigger to keep an accessible description equal to the tooltip text, no extra tab stop around an already-focusable control, visibility on hover and keyboard focus, and no body portal inside a native modal. The shared-component assertion comes last in each case, so on the current local Tooltip they fail only there. The Escape case also fails today on the dismissal itself: the local CSS-only tooltip never implemented WCAG 1.4.13 dismissal. Refs #1902
The local Tooltip in primitives.tsx duplicated the package's Tooltip. The adapter in the new common-overlays.tsx seam takes content instead of label and keeps three product guarantees the package does not give on its own: - The package sets aria-describedby on its wrapper div and only while open, so the focused control would lose its description. The adapter clones the child with aria-describedby pointing at an always-present hidden node holding the same text. - The wrapper defaults to tabIndex 0; the adapter passes -1 because the gallery wraps an already-focusable Button. - Inside NativeModalContext it keeps the in-place markup, because the package portals content into body, outside a native showModal() top layer (the common-select.tsx precedent). Escape now dismisses a keyboard-opened tooltip (WCAG 1.4.13), which the CSS-only tooltip never did. expectSafeLayout accepts the tooltip's measured top/left or its pre-measure visibility:hidden and nothing else. The pinned tests pass; each mitigation was reverted once and its test failed on the behavior assertion. Refs #1902
ErrorBanner keeps its name and props, so these tests drive the existing component. They require role="status" for the info tone and role="alert" otherwise, with the banner as the only live region; no heading added to the surrounding outline (the title was a <strong>); anchor and icon-button actions rendered inside the element carrying the test id and reachable by keyboard; and, in the gallery, a compact start-aligned banner that keeps its 2px high-contrast border. The shared ErrorState DOM is asserted last, so the current local banner fails only there. Refs #1902
ErrorBanner stays as the exported name with its props, now an adapter in common-feedback.tsx over ErrorState, so the 23 call sites are untouched and the sibling issues that rely on the name keep working. The local implementation is gone from primitives.tsx.
The package's contract differs in three places, each kept on the product side:
- Role: ErrorState is always role="alert" aria-live="polite". A ref bridge removes both from the inner block, so the section keeps role="status" for info and role="alert" otherwise and stays the only live region.
- Heading: ErrorState titles with an h2, which would land inside sections that already own one. The bridge sets role="none" on it, matching the previous <strong>.
- Action: primaryAction {label, onClick} cannot express the chat anchor or the icon Buttons in AuthGate and SchemaMismatchView, so the adapter still renders action as a ReactNode inside the test-id element.
Tones map error to danger, warning to warning and info to accent. Adapter-scoped CSS turns the centered 200px block into the compact banner; .ds-banner stays the root, so the 2px high-contrast border rule still applies.
Each of the three mitigations was reverted once and failed its test on the behavior assertion.
Refs #1902
BaseCard renders a div, while the gallery cards are articles and the data card is a named, focusable scroll container. The test requires two article landmarks named by their headings on the controls tab, no hover lift on these non-interactive cards, and a data card with the sample-table name and tabindex 0 that Tab reaches with a visible focus outline (BaseCard's own CSS sets outline:none under :focus-within), and an axe-clean page (an aria-label on a role-less div would be an aria-prohibited-attr failure). The BaseCard class is asserted last, so the current markup fails only there. Refs #1902
The three .surface-card articles in the gallery now render through a Card adapter in the new common-layout.tsx seam over BaseCard. .surface-card stays on the root, so styles.css, the high-contrast border rules and the browser assertions that select it keep working. BaseCard renders a role-less div, so the adapter passes role: article for the two sample cards and region for the named, focusable table card (an aria-label on a role-less div fails axe). hoverable is off because BaseCard's hover lift reads as clickable. BaseCard's :focus-within rule sets outline:none, which would hide the keyboard focus on the table card, so adapter-scoped CSS restores a 2px focus outline and keeps the previous block flow. The Activity operation card was left as it is to keep the parallel string rewrite of operations.tsx a clean rebase; BaseCard also reaches the Activity route through StatCard. Each mitigation (role, hoverable, focus outline) was reverted once and failed its test on the behavior assertion. Refs #1902
The compact navigation is a native <dialog> sheet today; the shared Drawer is an aside inside a backdrop div with its own focus trap. These tests pin what the sheet guarantees: - At 390x844: the menu button opens it with focus on the localized close button (the library default "Close" never appears), Tab from the last link wraps to the close button and Shift+Tab wraps back, Escape closes it with focus on the menu button, choosing a route closes it with focus on the menu button, it stays a left sheet at most 320px wide, the page never scrolls horizontally, and axe and expectSafeLayout pass with it open. - The closed drawer's links cannot take focus and Tab never enters it, at 390 and 1440; with the 200 percent test text scale the open drawer's links stay inside the viewport. - The in-app reduce-motion setting turns its transitions off with the OS preference unset. - Unit level: focus stays on a nav link while the shell re-renders with changing props (the SSE snapshot does this constantly), and Cmd/Ctrl+K and ? do nothing while focus is inside the drawer. The shared Drawer DOM is asserted last. On the current sheet every case fails only there, except the new widening case: the native sheet stays open when the viewport grows past 960px, which the swap handles. Refs #1902
The local Sheet (a native <dialog> through ModalDialog) is gone. The compact off-canvas navigation below 960px now uses a Drawer adapter in common-overlays.tsx over the package Drawer, whose own focus trap, Escape handling and opener restoration replace the native dialog's. Dialog stays local and native. Mitigations, each reverted once and shown failing its test on the behavior assertion: - Stable onClose: the package re-runs its open effect when onClose changes identity, so every shell re-render while open (the SSE snapshot re-renders constantly) would pull focus to the menu button and then the close button. The adapter passes one stable callback that reads the latest prop. - First-open focus: on the first open the package focuses its close button one frame before the panel stops being visibility:hidden, so focus stayed on the menu button. The adapter finishes the hand-off once the panel can take focus, unless focus already moved inside. - Shortcut guard: shell.tsx suppressed Cmd/Ctrl+K and ? only inside dialog[open]; it now also matches [role="dialog"][aria-modal="true"]. - Left sheet: adapter-scoped CSS keeps the approved 4185 geometry, a left sheet inset 12px at min(320px, 100vw - 32px). Two !important declarations exist only to outrank the package's width:100vw !important below 768px. - The panel keeps data-testid="mobile-nav-sheet" through a ref bridge, and the localized title and close label are required props, so the package default "Close" cannot appear. - Widening past 960px while open used to leave the sheet open over the desktop layout with a display:none restore target. The shell now closes the drawer and moves focus to the current desktop sidebar link. expectSafeLayout accepts the panel's inline width and max-width:100vw and nothing else. The dead .ds-sheet rule is removed. Refs #1902
PageLayout is a width container (1400px for the wide variant) that also adds its own 0.5rem inline padding below 768px. These tests require the shell's content column to fill the content box of .app-content-grid exactly at 390px, with and without the 200 percent test text scale, and at 1440px (where it is narrower than 1400px), with no horizontal overflow in the column, the route content or the document. The PageLayout class is asserted last, so the current section fails only there. expectTextScalePanelsReflow now also checks .page-layout. Refs #1902
AppShell keeps .app-shell, .app-toolbar and .app-content-grid, and its content column is now the shared PageLayout (wide variant, 1400px cap) through an adapter in common-layout.tsx, keeping the .app-content class. At 1440px the column is 1168px wide, so nothing moves; the cap only centers content on wider screens. Below 768px the package adds 0.5rem inline padding inside a column that .app-content-grid already pads; adapter-scoped CSS drops that second layer. With it reverted the route content sat 8px inside the grid's content box at 390px and the test failed on that edge. Refs #1902
PageHeader forwards only className and renders its own h1 and description without test ids or the focus-fallback attribute. These tests pin, on the Settings, gallery and ProductConnectionSurface headings: the existing title and description test ids on the same elements with the same text, exactly one h1 per route, the Settings title as the only data-dialog-focus-fallback (tabIndex -1), focus landing on it when a dialog closes after its trigger was removed from the DOM, and at 390px with the 200 percent text scale no horizontal overflow and a bold title rather than the package's 300 weight. The PageHeader classes are asserted last, so the current markup fails only there. expectTextScalePanelsReflow and the font diagnostics now also cover .page-header, .page-header__title and .page-header__description, which the gallery will use after the swap. Refs #1902
…h PageHeader The .screen-heading blocks on Settings, the gallery and the three ProductConnectionSurface states now render through a PageHeader adapter in common-layout.tsx. Models, Chat and Activity keep their headings for #1914, together with the .screen-heading and .eyebrow CSS they still use. PageHeader forwards only className and has no eyebrow slot. The eyebrow line is dropped at these sites and the eyebrow prop is removed from ProductConnectionSurface and its three call sites; the catalog keys stay until #1914 removes them. A narrow ref bridge restores the title and description test ids on the package's h1 and description, and with focusFallback puts tabIndex -1 and data-dialog-focus-fallback on the Settings h1. The fallback stays on a named page heading rather than an unnamed layout wrapper, and each route still has exactly one (Models keeps its own until #1914). Adapter-scoped CSS keeps the product's bold page-title typography instead of the package's weight-300 display title. Each mitigation (test ids, focus fallback, typography) was reverted once and failed its test on the behavior assertion; modal-focus.test.tsx passes unchanged. Refs #1902
The runtime summary and measurement details render dl.activity-metric tiles. These tests require every label, value, scope, observed time and unavailable reason to stay in the tiles, the tiles to stay non-interactive, and, on real tiles served through a mocked /ui-api/v1/runtime at 390px and 1440px, no clipped text (StatCard truncates labels and values with nowrap and ellipsis), the authored casing (it uppercases labels), no hover lift, no horizontal page overflow and a clean axe run. The StatCard group role and "label: value" name are asserted last, so the current tiles fail only there. Refs #1902
The summary and measurement-details tiles in activity/runtime.tsx now render through a StatCard adapter in the new common-data.tsx seam, keeping the activity-metric class and the runtime-summary test id. Only the tile JSX changed: every label, value, scope, observed-time and reason expression is byte-identical, with the detail lines in StatCard's hint slot. Each tile is now a group named "label: value". Values stay pre-formatted strings. formatCompactNumber is not adopted: it abbreviates and otherwise falls back to a locale-less toLocaleString(), while format.ts threads the explicit Locale. animate stays off and usePrefersReducedMotion is not adopted: the hook reads only the OS media query and cannot see the in-app reduce-motion setting, so CSS remains the only mechanism. Both decisions are recorded at the seam. Adapter-scoped CSS undoes StatCard's nowrap/ellipsis truncation and label uppercasing and the hover lift of a non-interactive tile. Each was reverted once and failed its test on the behavior assertion (truncation shows at 390px). The .activity-metric dt/dd rules in activity.css no longer match anything; they are left for the Activity rework so this change touches only tile JSX in the activity files. Refs #1902
While the catalog snapshot is pending, the Models table shows a bare status paragraph. These tests require exactly one live region inside the table whose text is the localized models.library.waiting string (English and Korean), visible to sighted users, with no package "Loading" label anywhere, and, in the browser at 390px and 1440px via the slow-catalog mock, no horizontal page overflow, expectSafeLayout, and axe. The decorative Skeleton shapes are asserted last, so the current paragraph fails only there. At 390px axe already reports scrollable-region-focusable on the table on main: the loading and empty table scrolls horizontally with nothing focusable inside, independent of the waiting content. The test tolerates exactly that node so the waiting state cannot add anything else. Refs #1902
The Models DataTable loading slot now renders a LoadingStatus adapter in common-feedback.tsx: one role="status" holding the visible, localized models.library.waiting text, above decorative Skeleton rows that are aria-hidden. SkeletonRow and SkeletonText were not used because each mounts one status per row with a default "Loading" name; SkeletonCard and SkeletonChart have no matching surface. With the shapes made non-decorative the test found four live regions instead of one and failed on that assertion. expectSafeLayout accepts a Skeleton's inline width and height in px or percent and nothing else. The shimmer stops under the in-app reduce-motion setting through the existing global CSS rule. Refs #1902
SmoothHeight's active state pins an inline height with overflow:hidden, which can clip focus rings of controls at the edges. These tests require that a list whose operations have all settled is never inside an active (clipping) wrapper, that while an operation is in flight the list animates and is released one settle window after the last one finishes, and, in the browser at 390px and 1440px with the running mock operation, that no ancestor clips the focus ring of any control in the list, with expectSafeLayout passing. The SmoothHeight wrapper is asserted last, so the current list fails only there. Refs #1902
…ight The Activity operations list (and its empty state) is wrapped in a SmoothHeight adapter in common-layout.tsx; operations.tsx changes only by that wrapper and its import. Placement: SmoothHeight's active state pins an inline height with overflow:hidden, which can clip focus rings at the list edges. The adapter is active only while an operation is in flight and for a 400ms settle window after the last one finishes, so the final change still animates and a settled list is never height-pinned or clipped. While active, the browser test shows no ancestor clips the focus ring of any control in the list at 390px or 1440px, because each operation card already pads its controls. Reverting to always-active failed the settled-list test; dropping the settle window failed the in-flight test. expectSafeLayout accepts the wrapper's measured px height and nothing else. Refs #1902
The library name cell stacks the inspect button, a "Selected" note and a "source · quantization" paragraph, which makes it the tallest cell. These tests require the name, selected state, source and quantization to stay in the cell as separate words, the inspect button to stay its only focusable control, and, in the browser at 390px and 1440px, no clipped text, no horizontal overflow, a clean axe run, and a row no taller than a same-page baseline clone that uses the previous paragraph markup, so the check does not depend on platform font metrics. The Badge DOM is asserted last, so the current paragraph fails only there. Refs #1902
The library name cell now renders "Selected", the source and the quantization as Badges through an adapter in common-data.tsx, on one wrapping line under the inspect button, instead of a small note and a margin-bearing paragraph. Lifecycle state keeps using StatusBadge. The badges are separated by real spaces so assistive technology and copied text keep them as separate words; without them the test read "cache4bit" and failed. Adapter-scoped CSS keeps the authored casing and lets long metadata wrap rather than overflow. Row height at 1440px drops from 100.5px to 79.5px; at 390px another cell sets the height and it stays 124.5px. The browser test compares against a same-page clone with the previous markup. The rest of the table is left to its redesign. Refs #1902
… check The shared Drawer always writes its width through CSSOM and the shared Tooltip positions its body-portalled content with top/left, so the real-server CSP check now covers both before it collects securitypolicyviolation events: the 390px case opens the navigation drawer (focus on its close button, expectSafeLayout and axe, Escape closes it), and both cases focus and hover the gallery tooltip, run expectSafeLayout, and dismiss it with Escape. The spec still runs only against a secured server through MLXCEL_WEBUI_CSP_URL. The new steps were dry-run against the preview build without the CSP assertions. Refs #1902
docs/webui/ui-common.md now states that the WebUI uses all 17 package components with imports confined to the common-*.tsx seam, replaces the old Dialog/Sheet, Tooltip and error-banner exception rows with the adopted Drawer, Tooltip and ErrorBanner adapters and their mitigations, and adds rows for Card, PageLayout, PageHeader, StatCard, LoadingStatus, SmoothHeight and Badge. It records the usePrefersReducedMotion and formatCompactNumber decisions, adds the follow-up to contribute Dialog upstream, lists the new CSSOM geometry the static layout assertions accept, and drops the stale screenshot-baseline wording. docs/webui/design-system.md updates the primitives export list, the shell's Escape, shortcut-suppression and compact-navigation behavior, and the current unit and browser test counts. ModalDialog carries a short comment pointing at the Dialog follow-up. Refs #1902
Rebuilt with scripts/webui/build_bundle.py (node v26.5.1, pnpm 11.18.0); --verify is clean. The initial JavaScript chunk is 152.67 kB gzipped, within the 200 KiB budget. Refs #1902
.ds-button:hover in components.css applies to disabled buttons, so a hovered disabled primary button swaps its blue fill for the 8% selection tint behind white text. This fails on main as well. Dropping the eyebrow above the sign-in form moved the Connect button up, so after sign-in the pointer now rests on the still-disabled Models "Add" button, and the existing 1440 signed-in axe check started failing about one run in ten while the fill transitioned back. The test signs in with the slow-catalog mock, hovers the disabled Add button, and requires its settled background to match the resting one and color-contrast to pass. Refs #1902
.ds-button:hover and .ds-icon-button:hover now apply only to enabled buttons. Before, hovering a disabled primary or danger button replaced its fill with the 8% selection tint behind white text, which fails color contrast. The package's own hover rules already exclude :disabled. The regression test and the 1440 signed-in axe check, which hit this through the moved sign-in form, pass consistently (80 of 80 stressed runs). Refs #1902
Rebuilt with scripts/webui/build_bundle.py (node v26.5.1, pnpm 11.18.0); --verify is clean. The initial JavaScript chunk stays at 152.67 kB gzipped. Refs #1902
inureyes
added
status:review
e6e38ee changed `.ds-button:hover` to `.ds-button:hover:not(:disabled)`, which raises it to the same (0,3,0) specificity as the package's `.button--primary:hover:not(:disabled)`. Product CSS loads after the package, so the 8% selection tint now replaces the primary and danger hover fills behind white text, a 1.16:1 contrast on hover in the light theme. The test hovers the enabled gallery Primary and Danger buttons and requires axe color-contrast to pass. It fails on the current CSS with the hovered Primary button flagged. Refs #1902
Wrap the disabled exclusion in `:where()` so the product hover rule keeps its original `.ds-button:hover` specificity. The package's enabled-variant hover fills win again, as they did on main, and disabled buttons still keep their resting fill. Both hover browser cases pass. Refs #1902
The shared Drawer traps Tab and handles Escape only from inside its panel, and unlike the native modal sheet it does not make the page behind it inert. After a pointer press on the drawer heading, focus sits on <body>: Shift+Tab then reaches controls hidden behind the drawer, and Escape does nothing. The test presses the heading, walks Shift+Tab three times requiring focus to stay on <body> or inside the drawer, then presses Escape and requires the drawer to close with focus back on the menu button. It fails on the current code when Shift+Tab lands on the gallery's Open dialog button. Refs #1902
AppShell marks <main> inert while the navigation drawer is open, as the native modal sheet did, so sequential focus cannot leave the drawer. The Drawer adapter also closes on an Escape that starts outside its panel, because the package listens on the panel only. The new containment case and the rest of ui-common-shell pass, including Escape and route-choice restoration to the menu button and the widening case. Naive reverts fail on their own assertion: without inert, Shift+Tab reaches the gallery's Open dialog button; without the listener, Escape leaves the drawer open. Refs #1902
The ui-common matrix and the design-system shell paragraph now state that the page behind the open navigation drawer is inert and that Escape closes it from outside the panel. The browser test counts cover the two new cases (54 Playwright tests), and the import-count sentence names the base.css import and the DataTable type re-export alongside the 17 component imports. Refs #1902
Rebuilds src/webui/assets from the restored button hover specificity and the inert page behind the open navigation drawer. Refs #1902
Member
Author
Implementation Review SummaryIntent
Findings Addressed
Remaining Items
Verification
|
Member
Author
Security and Performance Review (8141562)No CRITICAL, HIGH or MEDIUM findings, so nothing was changed. Checked
Remaining (LOW, not fixed)
Gates rerun: typecheck, lint, unit (336 Vitest), and on port 4902 |
A false -> true -> false animate sequence inside the 400ms settle window let the timer started at the first false transition end the window early, instead of the window lasting a full 400ms from the latest transition to false. Render SmoothHeight directly with fake timers and replay that sequence, then assert the settle window is still active 300ms after the latest transition to false and released only once a full 400ms has passed. Validation: - pnpm --dir webui exec vitest run src/design-system/ui-common-smooth-height.test.tsx fails on the new case (1 of 3 failed) against the current implementation.
Track a token that increments on every transition to false, even while already settling, and key the settle timer's effect on that token as well as the settling flag. Keying the effect on the settling flag alone missed a false -> true -> false replay inside the window: calling setSettling(true) while it is already true is a no-op value-wise, so the effect never reran and the original timer released the window 400ms after the first transition instead of the latest one. Validation: - pnpm --dir webui exec vitest run src/design-system/ui-common-smooth-height.test.tsx passes (3 of 3).
The drawer's outside-the-panel Escape handler closes the drawer whenever the event target sits outside `.drawer`, with no way for a future popup that portals elsewhere (like the Tooltip content or a nested dialog) to claim the key for itself first. Open the nav drawer, dispatch an Escape from a detached element that already called preventDefault on it, and assert the drawer stays open. Validation: - pnpm --dir webui exec vitest run src/design-system/ui-common-drawer.test.tsx fails on the new case (1 of 3 failed) against the current implementation.
Back off once the Escape event is already defaultPrevented, so a future popup that portals outside `.drawer` and handles its own Escape does not also close the drawer beneath it. Validation: - pnpm --dir webui exec vitest run src/design-system/ui-common-drawer.test.tsx passes (3 of 3).
The `.smooth-height` and `.skeleton` entries in expectSafeLayout matched any element with that bare class, so an unadapted future use of either shared component could pass the check by class name alone, without carrying the exact inline geometry the whitelist is meant to certify. Scope both to the product adapter's own class, matching the existing aside.drawer.ds-drawer entry: `.smooth-height.ds-smooth-height` and `.ds-loading__shapes .skeleton`. Validation: - pnpm --dir webui exec playwright test --config playwright-report/local-1902.config.ts --project=chromium tests/ui-common-components.spec.ts passes (14 of 14), which exercises both selectors.
Note in the ui-common export matrix that the SmoothHeight settle window always restarts from the latest transition to false, and that the drawer's outside-panel Escape handler backs off once the event is already defaultPrevented. Refresh the Vitest count in the verification section to 338 tests after the two new cases.
Rebuild src/webui/assets/ with scripts/webui/build_bundle.py so the embedded bundle reflects the SmoothHeight, Drawer and safe-layout whitelist fixes. Validation: - python3 scripts/webui/build_bundle.py --verify reports the regenerated digest as verified.
7 tasks
Brings in #1919, #1921 and #1922 (the i18n catalog consolidation from #1913). Conflicts were resolved so both changes survive: primitives.tsx keeps #1922's ConfirmDialog and drops the local Sheet and Tooltip; Activity runtime tiles keep #1922's t(locale, 'activity.*') keys inside the StatCard adapter; the operations list keeps #1922's keyed copy wrapped in SmoothHeight. The embedded bundle was rebuilt by scripts/webui/build_bundle.py rather than merged by hand. Merged instead of rebased so the test-first commit hashes cited in the PR evidence stay valid and no published commit is rewritten. Validated after the merge: typecheck, lint, unit (359 Vitest in 47 files, including src/i18n/drift.test.ts), all seven Playwright spec files on a private preview port, build_bundle.py --verify, check_webui_contract.py and check_binary_assets.py.
The merge brought in the catalog drift, confirmation and chat tests from #1922 and one more Activity and Settings browser case each. The verification paragraph now states the counts measured on the merged branch: 359 Vitest tests across 47 files and 56 Playwright tests (57 with font diagnostics).
11 tasks
inureyes
added a commit
that referenced
this pull request
Sep 19, 2026
Brings in #1902's ui-common component adoption. components.css merged cleanly with both sides intact: the .material-glass rule keeps -webkit-backdrop-filter ahead of backdrop-filter, .ds-button-primary and .ds-button-danger keep reading --token-buttonPrimaryBg and --token-buttonDangerBg, and #1902's :where(:not(:disabled)) hover rules are unchanged. app.tsx takes #1902's PageHeader Settings screen with this branch's Theme and Color scheme selects and the system-scheme listener. The design-system and ui-common docs keep both texts, with the test counts recomputed. The bundle was regenerated, not hand-merged. theme.spec.ts (including the glass button fill case) and ui-common-components.spec.ts (including both button hover cases) pass in the full Chromium suite (88), with unit (35 node:test, 470 Vitest), the theme-selector gate and build_bundle.py --verify. Refs #1903
inureyes
added a commit
that referenced
this pull request
Sep 19, 2026
Part of epic #1910. ## Summary - `identity.display_name` is now the model's inference id with no character rewriting. The catalog no longer turns `-` and `_` into spaces. Cache and preset entries keep their full name (`mlx-community/Qwen3-4B-4bit`), models-dir entries keep the directory name, and single-model entries keep the last `/` segment of the served id, as the issue decided. `qwen3-0.6b-4bit` and `qwen3_0.6b_4bit` are two distinct rows again. Every surface that prints `display_name` (library row, inspector heading, toolbar pill, Chat and Activity selectors, operation titles, settings selectors, dialogs) now shows the name passed to `-m` and the API. The Rust change is the `display_name` helper in `src/server/webui/catalog_metadata.rs` and its two call sites. - `ModelIdentity.display_name.maxLength` in `docs/webui/api.yaml` goes from 128 to 256 to match `inference_id`, because a cache `owner/name` can be 193 characters. The field's new description gives the rule for each source. `schema_version` stays `webui.ui-api.v1`, and the generated `ui-api.d.ts` is unchanged. - The delete dialog still accepts only `identity.id` as its token, and the fence test is byte-identical to main. The body now says to type the model ID shown below, not the model name. The field label reads "Opaque model ID shown above (starts with mdl_)" in English and "위에 표시된 모델 ID (mdl_로 시작)" in Korean. The `tests/fixtures/webui/strings.json` copies of both strings, which had drifted, now match `strings.ts`. - Long ids wrap. The dialog body paragraph and the model names listed in the capacity-recovery dialog now carry `models-wrap`, like the id already did. Without it, a verbatim underscore name has no break point: at 390px the capacity dialog measured scrollWidth 523 against clientWidth 356. - Fixtures and tests that used space-separated names now use hyphenated ids. That includes `identity-collision.json`, which still collides on `inference_id`, and the performance fixtures and specs. The `models.long_name` gallery string is now a long hyphenated id, the same in en and ko. `docs/webui/catalog.md` and `docs/webui/catalog.ko.md` describe the new meaning. The committed bundle is regenerated. ## Premises checked - Search already found the real name before this change: `inventory()` and the server `q` filter both match `inference_id`, which already held `qwen3-0.6b-4bit`. What changes is that the matched row now prints that name. - The issue expected a trailing `/` to fall back to the whole name through the existing `unwrap_or(name)`, but it never did. `"x/".rsplit('/').next()` is `Some("")`, so the old helper returned an empty string. The new helper skips the empty segment, and the test covers that case. - The issue did not say how to treat presets; this PR keeps their names whole. A preset's inference id is its section name (`config.model_alias = name`), and an overlay-only preset takes over a cache `owner/name`, which the last-segment rule would shorten. - Single-model mode keeps the last-segment rule, as the issue specified. The id there is either `--alias` or the checkpoint directory's `file_name()`, never a path, so the rule only changes an alias that contains `/` (for example `--alias Qwen/Qwen3-8B` displays as `Qwen3-8B`). The docs and the api.yaml description say so. Showing the alias whole would be a one-line change if that is preferred. ## Changes during review - Implementation review: the capacity-dialog list items got the wrap class. The capacity-recovery unit test now checks the listed names and the class, and fails without them. - Implementation review: the api.yaml description and both catalog docs now give the rule for each source. The earlier wording said "verbatim" everywhere, which is not true for single-model aliases. The Korean confirm label dropped "불투명", a word Settings already uses for the visual Opaque material. - Security and performance review: `performance.spec.ts` searched the 1000-entry catalog for `'0999'`, which only matched the old padded label, so the `WebUI bundle` engine smoke timed out on every engine. It now searches for `perf-model-999`. The review found no injection or path exposure: names render as React text or escaped attributes, and `display_name` is always equal to `inference_id` or a substring of it. The delete fence still requires the exact id and revision. ## Revert-and-fail With the `catalog_metadata.rs` change alone reverse-applied, the new contract test fails: ``` test server::webui::catalog::tests::catalog_contract_tests::display_name_is_the_inference_id_verbatim ... FAILED panicked at src/server/webui/catalog_contract_tests.rs:543:5: assertion `left == right` failed left: [("Meta-Llama-3.1-8B-Instruct_4bit", "Meta Llama 3.1 8B Instruct 4bit"), ("mlx-community/Qwen3-4B-4bit", "Qwen3 4B 4bit"), ("qwen3-0.6b-4bit", "qwen3 0.6b 4bit"), ("qwen3_0.6b_4bit", "qwen3 0.6b 4bit"), ("team/qwen3-preset", "qwen3 preset")] right: [("Meta-Llama-3.1-8B-Instruct_4bit", "Meta-Llama-3.1-8B-Instruct_4bit"), ("mlx-community/Qwen3-4B-4bit", "mlx-community/Qwen3-4B-4bit"), ("qwen3-0.6b-4bit", "qwen3-0.6b-4bit"), ("qwen3_0.6b_4bit", "qwen3_0.6b_4bit"), ("team/qwen3-preset", "team/qwen3-preset")] test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 8596 filtered out ``` With the patch re-applied, all 34 catalog tests pass. The check was run twice, once by the implementer and once independently. The frontend check was done the same way. Reverting `api.yaml`, `strings.ts` and `dialogs.tsx` makes the new 193-character schema-bound test and the delete-dialog wording test fail. Removing the capacity-list wrap class makes the capacity-recovery test fail. ## Test plan This branch is rebased onto origin/main at fd33ff1, which includes #1921 (catalog backend column) and #1922 (i18n catalog consolidation). The committed bundle was rebuilt by `build_bundle.py` at each rebased commit rather than merged by hand. One rebase conflict needed both sides: the performance spec's Chat option now reads `perf-model-0 · Ready`, combining this PR's name with #1922's localized state label. No test or fixture added by #1922 uses a space-separated model name. Results on the rebased head: - [x] `cargo test --profile test-fast --features metal,accelerate -p mlxcel --lib server::webui`: 78 passed, 0 failed, including `display_name_is_the_inference_id_verbatim` - [x] `cargo clippy -p mlxcel --lib --tests --features metal,accelerate -- -D warnings` and `cargo fmt --all -- --check`: clean - [x] `pnpm --dir webui run typecheck` and eslint: clean - [x] `pnpm --dir webui run unit`: node 18/18, vitest 340/340 in 39 files - [x] Playwright chromium default suite: 30/30. Engines config (`engines.spec.ts` and `performance.spec.ts`), chromium: 7/7. All runs used a private port with `--strictPort`, never the preview server shared with other worktrees - [x] `check_webui_contract.py` (49 fixtures), `build_bundle.py --verify`, `check_binary_assets.py`, `check_cross_repo_refs.py`: pass Before the rebase, the same gates also passed `server::router_server` (63 passed, 1 ignored) and the engines config in firefox (7/7). ## Real-binary acceptance The orchestrator built `mlxcel-server` from 3082e1d, the pre-rebase head carrying the same change (`--profile test-fast --features metal,accelerate`), and ran it with `--webui --models-dir models/mlx --no-models-autoload` against the real checkpoint store and the default managed cache. I drove the bundled WebUI in Chromium at 1440px with a read-only Playwright script that aborts any load, unload, download, delete or refresh request. It recorded no mutating request. - [x] Hyphenated models-dir entry: `qwen3-0.6b-4bit` prints as `qwen3-0.6b-4bit` in the library row, the inspector heading and the toolbar pill (`qwen3-0.6b-4bit · Unloaded`). - [x] Underscored models-dir entry: `stablelm-2-1_6b-chat-4bit` prints unchanged in the same three places. - [x] Cache entry: `mlx-community/Qwen3-0.6B-4bit` prints as the full `owner/name` in the row, the inspector heading and the pill. It also appears as `mlx-community/Qwen3-0.6B-4bit · unloaded` in the Chat "Model for next turn" picker and as `mlx-community/Qwen3-0.6B-4bit` in the Activity selector (225 options). - [x] Search: `qwen3-0.6b-4bit` returns the `qwen3-0.6b-4bit` row, plus `mlx-community/Qwen3-0.6B-4bit`, which contains the same text in a case-insensitive match. `qwen3_0.6b_4bit` returns no rows ("No matching local models"). - [x] Delete dialog, opened on the cache entry and cancelled without typing: the body ends "To confirm, type the model ID shown below, not the model name.", the `mdl_` id is printed below it, the field label reads "Opaque model ID shown above (starts with mdl_)", and Confirm stays disabled while the field is empty. The long cache name wraps inside the dialog. ## Merge with main and CI - `origin/main` 5616f86 (#1924, ui-common components) was merged into this branch as 7e40c01 by the epic #1910 orchestrator. Only the generated bundle conflicted; it was regenerated with `scripts/webui/build_bundle.py` (`--verify` digest 711cbf2a) and not hand-merged. On the merged tree: `typecheck`, `eslint`, `unit` (Vitest 362 in 47 files), and Chromium Playwright on a private port (`models`, `ui-common-shell`, `ui-common-components`, `browser`, `chat`, `activity`, `settings`: 56 passed) all pass. The merged Rust tree was checked earlier: `server::webui` 78 passed, `models::registry` 10 passed. - The GB10 `WebUI installed artifact` job's Activity performance gate reported `investigate` on four consecutive attempts at the pre-merge heads while every other step passed: `two-visible` median decode degradation -0.35 (baseline CV 5.63), +0.20 (CV 5.45), +2.74 (CV 3.38), and `one-visible` +3.38 (CV 4.22). The gate flags a median above 2 percent or a baseline CV above 5 percent over five pairs. On the same runner, `main` after #1922 failed the same gate (`two-visible` -6.76, CV 7.45) and passed on rerun, and #1919 failed it once (+3.19) and passed on its next head. This change only alters the catalog label string and a few UI strings, which do not touch decode or the Activity polling path, so these are read as runner noise. Per the maintainer's decision for this epic, the PR merges with this evidence recorded rather than with the gate relaxed; stabilizing the gate is tracked in #1925. Closes #1912
inureyes
added a commit
that referenced
this pull request
Sep 19, 2026
…1923) Closes #1903. Part of #1910. ## What changes - **Theme ids.** `data-theme` always holds `<family>-<scheme>`: `mlxcel-light`, `mlxcel-dark` (the approved baseline, still the default), `glass-light`, `glass-dark`. `webui/src/design-system/theme.ts` resolves it; the stored `themeFamily` and `colorScheme` (`system`/`light`/`dark`) land in `data-theme-family` and `data-color-scheme`. The old flat `theme` field is migrated, and `system` follows host changes live. Settings gets Theme (Standard, Glass) and Color scheme selects. - **ui-common mechanism.** `main.tsx` imports one theme entry (`themes/index.css`) after `base.css`, and no ui-common theme file. The color, shadow and button `--token-*` values moved from `:root` into `[data-theme]` blocks unchanged; `common-tokens.css` keeps the theme-independent half. - **Glass theme.** Translucent chrome with a light reflection, tinted buttons, highlighted primary and danger fills, near-opaque content surfaces, no per-button blur. It reads the existing intensity, material, transparency, backdrop support, contrast and motion switches plus the `prefers-*` queries; any transparency or contrast preference makes it opaque at any intensity. Values derive from lablup/backend.ai-go (derivation line, `NOTICE`); the tokens stay here because they bind to this product's semantic token layer. - **No flash.** `webui/public/theme-bootstrap.js` is a classic render-blocking script in `<head>`, external because the CSP is `script-src 'self'`. `build_bundle.py` rejects a bundle where it is missing, duplicated, deferred in any way, or after the app module, and any inline script. - **Gate.** `check:theme-selectors` (in the `webui-bundle` job and `make verify-webui-frontend`) evaluates every `data-theme`/`data-theme-family` selector against the shipped ids and rejects `data-color-scheme` selectors, `prefers-color-scheme` outside the resolver, stray theme imports and ids with no theme block; exemptions need a written reason. Against `main` it reports `tokens.css:47`, `:48`, `:50`, `:51`, `:75`. ## Found along the way - Chrome blur never rendered in Chromium or Firefox (Lightning CSS drops a standard `backdrop-filter` that precedes its `-webkit-` twin), and the primary and danger fills were hard-coded over ui-common's rules. Both are one-line fixes in `components.css`, and both survive the merge with #1902's rewrite of that file, alongside its `:where(:not(:disabled))` hover rules. - `features/chat/style.test.ts` passed vacuously (Vitest blanks CSS imports); it now reads from disk. `models.css` read two undefined tokens; inspector labels are now muted and the selected row highlighted. - Review fixed rendered glass fills and a bootstrap check that let `type="module"` through; the security pass fixed the gate skipping its check through a symlink and two super-linear patterns. ## Validation Rebased onto `fd33ff15` (#1919, #1921, #1922) with an unchanged source diff, then merged with main at `5616f86b` (#1924, #1902's component adoption); `app.tsx` keeps #1902's PageHeader Settings screen with the theme selects, and the bundle was regenerated. On `cdb80fe7`: typecheck, lint, the gate, unit (35 node:test, 470 Vitest), Chromium suite (88, including the glass button fill case and #1902's button hover cases; axe on all four ids), `build_bundle.py --verify`, contract, binary-asset and cross-repo checks. The engines smoke on Chromium, Firefox and WebKit passed before the rebase. Each new check was reverted to confirm it fails. ## Bundled-binary checks Playwright Chromium against `mlxcel-server` built from `c26e3247` (before the rebase and the #1924 merge) with the embedded bundle and no model loaded. 1. **No flash: pass.** `theme-bootstrap.js` is served 200, `text/javascript`, `no-cache`, under the strict CSP, ahead of the app module. With the app module held, all nine cases (cold profile under light and dark hosts, each id stored under the opposite host, glass `system` under both hosts, legacy `{theme: dark}`) had the right `data-theme` and canvas with nothing mounted, and no CSP violation. 2. **Settings: pass.** Glass then Dark gives `glass-dark`, persists without the legacy field, is restored before mount on reload; `system` follows a host flip live. 3. **390x844: pass** in both families: 0 px overflow on all four routes; the sheet opens, navigates, closes on Escape and returns focus. 4. **200 percent zoom: not native** (headless Chromium cannot zoom; a half-width CSS viewport at deviceScaleFactor 2 stood in). 320 and 640 CSS px: 0 px overflow everywhere. 195 CSS px (390 px window): 125 px on every route from `body { min-width: 320px }`; main's bundle measures the same, so it predates this PR. 5. **Keyboard: pass** for `mlxcel-light` and `glass-dark`: 18 Tab stops in DOM order, each with a visible indicator; Escape restores focus from the command palette (button and Cmd+K), a Select popup and the sheet. 6. **Contrast: pass.** axe WCAG 2.2 AA on all four routes for all four ids at intensity 100: 0 violations normally, under `prefers-contrast: more` and with High contrast on; the chrome drops to `blur(0px)` with no gradient. 7. **Served-CSP spec: pass** (both variants). ## Merge with main `origin/main` 31c0459 (#1920, verbatim display names) was merged into this branch as 83fc058 by the epic #1910 orchestrator. Only the generated bundle conflicted; it was regenerated with `scripts/webui/build_bundle.py` (`--verify` digest 986294f2) and not hand-merged. On the merged tree: `typecheck`, `lint`, `check:theme-selectors` (152 files, 4 theme ids), `unit` (Vitest 473 in 51 files) and the Chromium Playwright suite on a private port (88 passed) all pass. The live-server script above was also re-run against a server built from cdb80fe (after #1924 was merged in): 23 of 25 cases pass, and the two failures are the pre-existing 195 CSS px overflow from `body { min-width: 320px }` described above.
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
1 participant
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.
The bundled WebUI now uses all 17
@lablup/ui-common@0.1.0-alpha.19components instead of 7. The ten new ones sit behind thewebui/src/design-system/common-*.tsxadapter seam and are re-exported fromprimitives.tsx, so feature screens still import only the design system. The localTooltip,SheetandErrorBannerimplementations are gone fromprimitives.tsx.Count, measured the issue's way (
grep -rn '@lablup/ui-common' webui/src/against the 17 names indist/index.d.ts): 17 of 17. Package imports appear only incommon-adapters.tsx(Button, StatusTag, ProgressBar, EmptyState, Tabs, DataTable),common-select.tsx(Select),common-overlays.tsx(Drawer, Tooltip),common-feedback.tsx(ErrorState, Skeleton),common-layout.tsx(BaseCard, PageHeader, PageLayout, SmoothHeight) andcommon-data.tsx(Badge, StatCard), plus the existingstyles/base.cssimport inmain.tsx.What each adapter keeps
Sheet): stableonClose(the package reruns its focus effect on every identity change, so each SSE-driven shell render moved focus); completes the first-open focus hand-off the package drops while the panel is stillvisibility:hidden;<main>isinertwhile it is open and an Escape that starts outside the panel still closes it (skipping IME composition and already-handled events), because the package only traps focus from inside its panel; the shortcut guard also matches[role="dialog"][aria-modal="true"]; left sheet at the approvedmin(320px, 100vw - 32px)via scoped CSS (two commented!importants outrank the package's<768pxfull width); localized title and close label;data-testid="mobile-nav-sheet"kept by a ref bridge; widening past 960px while open closes it and focuses the desktop sidebar (the old sheet stayed open with adisplay:nonerestore target).ErrorBannername and props: the 23 call sites are unchanged, and refactor(webui): move every user-facing string into the i18n catalog and remove native confirm dialogs #1913/feat(webui): app shell shows loaded models, uses PageLayout/PageHeader, and completes the global shortcuts #1914 rely on the name. A ref bridge removes the package's innerrole="alert" aria-live="polite"so the banner stays the only live region (statusfor info,alertotherwise) and setsrole="none"on the package h2 so no heading lands in sections that own one.actionstays a ReactNode becauseprimaryActioncannot express the chat anchor or icon Buttons. Tones map error/warning/info to danger/warning/accent.{content, children}. The package describes its wrapper, and only while open, so the control is described by an always-present hidden node; the wrapper adds no tab stop; insideNativeModalContextit renders in place instead of portalling out of the top layer. Escape now dismisses it (WCAG 1.4.13).Card, gallery cards; also inside StatCard): keeps.surface-card, passesrole(article, or region for the named focusable table card), turnshoverableoff, and restores the focus outline that BaseCard's:focus-withinrule removes.wide): drops the package's extra 0.5rem inline padding below 768px.focusFallbackputstabIndex=-1plusdata-dialog-focus-fallbackon the Settings h1, so the dialog focus fallback stays a named heading.LoadingStatus: one visible, localizedrole="status"over decorative shapes in the Models table loading slot, since SkeletonRow/SkeletonText mount a status per row), SmoothHeight (Activity operations list, active only while an operation is in flight plus a 400ms settle window restarted on every transition, so a settled list is never height-pinned or focus-clipped), Badge (Models row: selected, source, quantization; rows at 1440 go from 100.5px to 79.5px).Recorded decisions and follow-up
usePrefersReducedMotion: not adopted. It reads only the OS media query and cannot see the in-appdata-reduce-motiontoggle; CSS stays the only mechanism and its global rules also stop the shared components' motion (browser test: with the OS preference unset, the drawer's transitions are 1ms). StatCardanimatestays off for the same reason.formatCompactNumber: not adopted. It falls back to a locale-lesstoLocaleString();format.tsstays the single locale-aware formatter.common-data.tsx) and indocs/webui/ui-common.md, which also records the follow-up to contributeDialog(native top layer, focus trap, Escape with focus restoration) tolablup/ui-common; the same note sits aboveModalDialog. Nothing was opened upstream.Tests first: failing before each swap, and naive reverts
Each test commit precedes its swap commit, and the shared-DOM assertion is last, so the red runs fail there unless noted. All Playwright runs used a private preview port (4902) so no sibling worktree's server was reused.
Also fixed here
.ds-button:hoverapplied to disabled buttons and swapped a disabled primary fill for the 8% selection tint behind white text. Dropping the eyebrow moved the sign-in Connect button up, so after sign-in the pointer rests on the still-disabled Models "Add" button and the existing 1440 signed-in axe check failed about one run in ten; the rule is broken on main too. The hover rules now use:hover:where(:not(:disabled)), which excludes disabled buttons without raising specificity, so the package's primary and danger hover fills still win on enabled buttons (a first version at higher specificity made enabled hover text 1.16:1; the test for that case failed before the correction).Known and left: at 390px the loading or empty Models table scrolls horizontally with nothing focusable inside (
scrollable-region-focusable, also on main); the Skeleton test tolerates exactly that node and the table belongs to #1918. The served-CSP spec exercises the gallery only, so SmoothHeight and Skeleton reach the real CSP only through their CSSOM height/width writes, which strictstyle-src 'self'permits.Validation
ConfirmDialogstays next to the removedSheet/Tooltip; the bundle was rebuilt, not hand-merged.pnpm --dir webui run typecheckandlint: pass.unit: 18 node:test and 359 Vitest tests (47 files) pass, including refactor(webui): key screen copy in the catalog and drop window.confirm #1922'ssrc/i18n/drift.test.ts.python3 scripts/webui/build_bundle.pythen--verify: clean, regeneratedsrc/webui/assets/committed; initial JS 152.7 kB gzip against the 200 KiB budget.check_webui_contract.py(49 fixtures) andcheck_binary_assets.py: pass.src/features/chat/confirmations.test.tsx"refocuses the Clear All trigger once clearing settles if focus is still on body" (from refactor(webui): key screen copy in the catalog and drop window.confirm #1922): itssetTimeout(0)focus claim can run before React commits the re-enabled button. It passed 15 of 15 isolated and 4 of 4 full-suite runs locally, and this PR does not touch that flow; recorded as a flaky refactor(webui): key screen copy in the catalog and drop window.confirm #1922 test, not relaxed here.Live acceptance against the embedded bundle
Run against
mlxcel-server --webui --models-dir models/mlx --no-models-autoload --settings --props --metrics --slotsbuilt from be87e27 (test-fast profile, embedded bundle), signed in with a session key, no model loaded, nothing loaded, unloaded or downloaded. Headless Chromium, driven by a local uncommitted spec.<main>inert; Tab cycles home, Models, Chat, Activity, Settings, Close and wraps back to home without leaving the drawer; Shift+Tab from Close reaches Settings then Activity; Escape closes it and focus returns to the menu button; choosing Activity closes it with focus on the menu button; after a pointer press on the drawer heading, Shift+Tab goes to the document and then to the drawer's last link, never to the page behind. No horizontal overflow with the drawer closed or open.#galleryhave 0 px horizontal overflow with the nav closed and open; the drawer is 320 px and every nav link stays inside the viewport. Not a native-zoom observation.body { min-width: 320px }on main (320 - 195), not this PR; the drawer itself fits (163 px, links inside the viewport).webui/tests/csp.spec.tsviaMLXCEL_WEBUI_CSP_URL=http://127.0.0.1:<port>/webui/andplaywright.csp.config.ts: both cases (light 1440, dark 390) pass with zero CSP violations and no external requests, including the new steps that open the drawer and show the tooltip.h1.page-header__title,settings-title). Both Settings info banners arerole="status"and the gallery warning banner isrole="alert", with no nested live region and the package title set torole="none"(no extra heading).Part of #1910
Closes #1902