[Chakra exit · Phase 2 · PR 12/14] frontend: the reassign-partitions wizard on Registry components, plus a smoke spec

[SpicyPete]

Chakra exit · Phase 2 (route by route) · PR 12 of 14

Part of the Chakra exit plan and live tracker. Branch chakra-exit/12-reassign-partitions, based on master (rebased onto it 2026-09-09, 0 commits behind).

Sixteen files: ten source (including the new wizard-steps; misc/kowl-table deleted), two unit tests, one Playwright spec, plus package.json and both lockfiles for the react-highlight-words drop. The shell and the three steps share currentStep and the selection maps, so they ship together. No API change: every guard (isEnabled, computeWarning, maximumSelectedReplicationFactor) reads the same state it did before. The route had no Playwright coverage, so a smoke spec lands first, in its own commit — it walks step 1 → step 2 and back, because the step state is what a re-skin can break invisibly. It stops before "Start Reassignment": this suite runs against a live cluster.

The hunks that carry judgement

• components/wizard-steps.tsx replaces Chakra's <Stepper index={…}>. The Registry ships defineStepper, but that owns navigation through its own methods — this wizard keeps currentStep in its own state, so the indicator is a pure function of that index and nothing in it can move the wizard.
• bandwidth-slider — the Registry Slider renders its own track and thumb with no mark or thumb-tooltip slot, so the five marks and the value bubble are positioned against the same value scale. It also counts thumbs from an array: a scalar value fell through to [min, max] and rendered a second, valueless but focusable thumb — one arrow key from writing NaN to maxReplicationTraffic and into setReplicationThrottleRate. Caught in review; it takes [sliderValue], clamped (the default maxReplicationTraffic is 0, so log10 is -Infinity).
• Two tables gain the expander column Chakra's DataTable injected for subComponent. Every table paginates like the legacy DataTable — 50 a page (10 for the active-reassignments list), pager only past that; Chakra defaulted pagination on, the Registry to 10 with a permanent pager. Step 1 also lost click-to-sort on four columns; restored with DataTableColumnHeader, except the topic-name column, whose header is the search control.
• Step 1's table passed a no-op onRowSelectionChange and a placeholder rowSelection; selection is done by the check column, so both simply go. Two columns also shared accessorKey: 'partitions', so both resolved to the same TanStack column id.
• Every checkbox gains an accessible name. They had none: Chakra's took its label from children, and these render bare in table cells. That is also what lets the spec find them.
• CancelReassignmentButton — a Chakra Popover with header/body/footer slots becomes a Registry Popover with plain markup; closeOnBlur={false} and returnFocusOnClose={false} have no Base UI equivalent and were fighting the default focus behaviour.
• Dialog widths follow the nearest rung: Chakra's minW="5xl" (64rem) → xl (56rem), minW="3xl" (48rem) → lg (42rem). The first cut had used full, which is 90vw.

misc/kowl-table (SearchTitle) is deleted: the sort fix put DataTableColumnHeader on the Topic column, the search overlay it carried was unreachable (filterOpen was never set), and nothing else imported it.

Fixed while here (pre-existing)

"Remove throttle" read newThrottleValue from the render that had just cleared it and re-applied the current throttle; the handler takes the value explicitly. The topic-throttle dialog now closes after a successful reset.

Fifth review round (2026-09-09) — the sort menu, and the slider's input lag

The sort menu was destroyed by the page's own poll about a second after it opened (finding). Not a MobX subtlety — nothing on this page is an observer. PageComponent.componentDidMount subscribes to all five Zustand api stores and calls forceUpdate() on any write, so the 3 s reassignments poll re-renders the whole page; the inline columns={[…]} arrays then handed TanStack v9 fresh header/cell function identities, flexRender does createElement on those, and React treats a new function as a new component type — so it unmounted and remounted the header cells, taking the open portal with them. Sorting itself was correct; you had roughly a one-second window to reach Asc/Desc.

All five tables in the wizard now build their columns once — an instance field in the class steps (the arrow bodies read this.props at call time, so nothing goes stale), useMemo keyed on props.assignments in the nested review table. The same remount was also killing the partition-error popovers in cells. topic-sorting.test.tsx gains the guard: open the Size menu, re-render the page, assert the menu is still mounted — it fails on the pre-fix code.

The bandwidth slider and the throttle summary lagged input by up to a poll, and fixing it needed both halves. uiSettings.reassignment.maxReplicationTraffic = x mutates the nested object the proxy's get returns, so updateSettings() never ran and no subscriber was notified; the write now assigns the whole section, which goes through the top-level trap. And StepReview is now a small function wrapper that subscribes with useUISettingsStore((s) => s.reassignment.maxReplicationTraffic) around the class (StepReviewContent), because a plain Component reading through the proxy would not re-render even once the store published. step3-review.test.tsx covers it. (finding)

The wiring is pre-existing — master has the identical assignment and no observer — but it belongs here rather than in §04's flagged pile: master's Chakra slider masked half of it with its own internal drag state, while the Registry Slider is fully controlled, so the thumb froze too. The third finding from the same round, a silent submit failure on Redpanda, is genuinely outside every migration hunk and is split out into #2644.

Also in this PR, unrelated to the migration

The quotas and consumer-group lists now default to ten rows a page, not the legacy-parity 50 (requested directly). Ten is the Registry pager's own default and the first of its "Rows per page" options, and both pages keep the choice in the URL, so a wider page is one select away and survives a reload. The quotas route's pageSize fallback moves with it — .catch(50) only fires for an out-of-range URL value, and landing on 50 when the page defaults to 10 would be its own surprise — and quota-pagination.spec.ts's hardcoded 50 follows. That spec's "no pager below one page" assertion is vacuous for an unrelated, pre-existing reason: it locates [aria-label="pagination"] while the Registry pager labels its group "Pagination", and the pager renders unconditionally on that page. Left as found, so it can be fixed as a spec change.

Effect

Files importing @redpanda-data/ui: 50 → 43 — the six reassign-partitions/ files plus misc/kowl-table.tsx, which this PR deletes. OSS-only surface. (Earlier revisions of this body quoted 81 → 74, against the pre-Phase-2 master.)

Gates

Re-run after the fifth round (2026-09-09): type:check clean · unit 882 tests · integration 1321 tests (the two new specs included), all passing · bun run lint leaves the tree clean, so CI's dirty-check passes · lint:check per changed file reports only pre-existing errors, each verified against an in-place probe of master's copy — see the lint note below.

Second review round (2026-09-08)

• Partition-error popovers open on hover again, are titled (the dialog was unnamed) and take the width their text needs; the cancel-confirm popover is titled and its buttons are a spaced pair (the Registry ButtonGroup is attached-only).
• The Size column sorted on a field that does not exist; it reads logDirSummary.totalSizeBytes. Progress bars paint brand for the active state; the slider bubble was background-on-background; the slider's dead "Unlimited" end is gone.
• Statistic → Registry Stat on the header row; column size hints the Registry never reads are gone from all four tables.

Final review (2026-09-08)

• The remove-throttle dialog's hover note is a Registry Tooltip (the hand-rolled one was clipped inside the Dialog). The dead topic-search path (Highlighter, the quickSearch read) went with SearchTitle. Comments trimmed.
• Lint (corrected 2026-09-09, measured against in-place probes of master's copies): six of the seven errors are pre-existing — noBarrelFile + a dead suppressions/unused in components/icons/index.tsx, noUselessConstructor in active-reassignments.tsx and reassign-partitions.tsx, and noExcessiveCognitiveComplexity in reassign-partitions.tsx and state/ui.ts. The branch also clears master's two noRestrictedImports errors on these files.
• One error is net-new to this branch: a11y/noNoninteractiveTabindex at reassign-partitions.tsx:351 — the fourth round gave the throttle-dialog tooltip a focusable <span tabIndex={0}> trigger so it is reachable by keyboard, which is what Biome objects to. It does not fail CI (bun run lint is fix-mode and this rule's fix is unsafe), but it wants either a <button type="button"> trigger or an explicit suppression before merge.

Table polish (2026-09-08)

• Hiding is off at table level (TABLE_OPTIONS.enableHiding: false) instead of per column, so a column added later cannot bring the header menu's "Hide" item back.

Fourth review round (2026-09-08)

• react-highlight-words and its types leave package.json and both lockfiles — SearchTitle was the last importer. Lockfiles regenerated with the pinned bun (frontend/.bun-version), so CI's dirty-check stays clean.
• Wizard icons come from components/icons (which gains InboxIcon) instead of being imported ad hoc.
• The throttle-dialog tooltip gets its own TooltipProvider (150 ms, not Base UI's 600) and a focusable trigger. See the lint note above.
• Nav buttons use min-w-56/ml-auto classes and the link buttons size="xs", replacing inline styles; the loading skeleton's margin moves onto a wrapper.
• uiSettings.reassignment.quickSearch and pageSizeSelect are dropped — step 1 no longer reads either.

🤖 Generated with Claude Code

[github-actions]

✅ Clean — no registry drift, off-token colours, or ad-hoc classes

App: frontend · Scope: diff vs origin/master · Files: 17

	Count
⚠️ Outdated registry components	0
🛠 Locally-modified components	0
❓ Unknown to registry	0
🎨 Off-token palette colours	0
🔢 Ad-hoc utility classes	0

Generated by lookout audit-changes.

[eblairmckee]

Tested this branch against a 3-broker Redpanda cluster with 61 topics (50/page pager exercised, sizes spanning 351 B to 246 kiB). Step 1's table renders correctly and the four sortable columns are the right four — but sorting is effectively unusable because the page's polling refresh tears down the header menu. Detail inline. Sorting logic is fine; it's the menu lifetime.

[eblairmckee]

The column sort menu is destroyed by the page's auto-refresh about a second after it opens, and the trigger's state desyncs.

Master used Chakra's DataTable, whose headers sorted on a single click. DataTableColumnHeader is a dropdown-menu-trigger (aria-haspopup="menu"), so sorting is now two steps: open the menu, then pick Asc/Desc. On /reassign-partitions that second step usually never happens.

Measured on the Size column:

t	[role=menu] count	items	aria-expanded
before click	0	—	false
+150 ms	1	Asc, Desc	true
+1650 ms	0	—	true
+4650 ms	0	—	true

Two consequences:

1. You get roughly a one-second window to hit Asc/Desc before the refresh unmounts the menu.
2. After it's torn down, aria-expanded stays true while nothing is rendered — the trigger thinks it's open. The next click toggles it to closed, so it reads as a dead button. That's what a user hits: click header, nothing; click again, still nothing.

I first assumed sorting was broken outright — clicking each of the four headers twice, by coordinate and via button.click(), never reordered a row. It isn't broken. Driving it fast enough to beat the refresh sorts correctly:

Desc → rp-seed-00=246 kiB, rp-seed-50=149 kiB, rp-seed-01=101 kiB, rp-seed-02=26.8 kiB

That's genuinely numeric — a string compare would put 26.8 kiB above 246 kiB — so the "Size sorts by real bytes" requirement passes. Worth keeping the seeded spread when you verify a fix, since equal sizes would hide a lexical regression.

The refresh is pre-existing on this page (the "Fetching data…" indicator by the title), so the collision is new only because the sort UI moved into a portal. Worth checking whether the other 12 migrated tables in #2636 sit on polling pages too — same pattern would bite them, and it would be invisible on a static page.

Unrelated but adjacent: no header sets aria-sort — I checked all four before and after sorting, always absent. Screen readers can't announce which column is sorted or in which direction. If DataTableColumnHeader is shared, that's one fix for every migrated table.

[SpicyPete]

Fixed in 03e7b12 — thanks, this was the right diagnosis and the measurement made it easy to confirm.

One correction to the mechanism, because it makes the blast radius bigger than one page: there is no MobX observer here — nothing in this feature is one. PageComponent.componentDidMount subscribes to all five Zustand api stores and calls forceUpdate() on any write, so every page built on PageComponent re-renders on every poll. The inline columns={[…]} arrays then hand v9 fresh header/cell identities, and flexRender does createElement on the column's own function — a new function is a new component type, so React unmounts and remounts the header cell rather than updating it. That is what took the portal with it. The same remount was also killing the partition-error popovers in cells.

All five tables in the wizard now build their columns once: an instance field in the class steps (arrow bodies read this.props at call time, so nothing goes stale) and useMemo keyed on props.assignments in the nested review table. topic-sorting.test.tsx gains a guard — open the Size menu, re-render the page, assert the menu is still mounted; it fails on the pre-fix code.

On aria-expanded staying true: that was the detached node. A remounted trigger renders false, which is what the guard test sees in the DOM dump. The user-visible symptom you described is the same either way.

Your last point stands and I have recorded it: of the merged Phase 2 tables, connect/overview.tsx is the one that combines an inline columns array with DataTableColumnHeader and PageComponent, so it has the same exposure at auto-refresh cadence (10 s once a user turns it on); the four security tabs and topics/…/message-headers.tsx have popovers in cells. Those are other PRs' surface, so they are on the tracker rather than in this diff.

aria-sort is a fair hit and one fix upstream covers every migrated table, but data-table-column-header.tsx is vendored from ui-registry — this repo does not edit that tree — so it needs a registry PR plus a version bump here. Also tracked.

[eblairmckee]

📺 Visually reviewed
I wasn't able to reproduce the "reassignments pending" state, which I'm attributing to lack of polling, but we do get toasts for pending and success states.
the one state I am missing though, is submission errors. I see a brief toast flash, but nothing else, and I can't respawn the toast on subsequent submissions. I presume this is likely a legacy bug, but investigating if there's an easy fix

[eblairmckee]

Two more from testing against a 3-broker cluster, both pre-existing rather than regressions from this PR. This one is the bandwidth slider's input lag; the submit failure is in a separate conversation comment since its code sits well outside the diff.

[eblairmckee]

The bandwidth slider and the throttle summary both lag user input by up to a poll interval — not a race, a missing store notification.

Reported while testing: dragging the slider updates the thumb and the "Traffic throttle" summary line noticeably late, and they lag together.

Root cause is on L184 just below (outside this PR's hunks, so I can't anchor there directly):

onSettingsChange={(x) => {
  settings.maxReplicationTraffic = x;   // settings === uiSettings.reassignment
}}

uiSettings is a Proxy over the Zustand settings store, and its set trap only fires for top-level assignment:

set(_target, prop, value) {
  useUISettingsStore.getState().updateSettings({ [prop]: value });
  return true;
}

uiSettings.reassignment goes through get, which returns the raw nested object out of getState() — not another proxy. So settings.maxReplicationTraffic = x mutates Zustand's state in place and never calls updateSettings(). No set(), no subscriber notification, ever.

The value genuinely changes, so reads are correct — which is why it eventually appears. The tree only re-renders when something unrelated triggers it, and on this page that's the periodic refresh. Hence the delay, and hence both the thumb and the summary lagging in lockstep: they read the same non-reactive value.

Two halves to a real fix, and it needs both:

Notify on write — go through the top-level trap so the store actually publishes:

onSettingsChange={(x) => {
  uiSettings.reassignment = { ...uiSettings.reassignment, maxReplicationTraffic: x };
}}

Subscribe on read — StepReview is a plain Component reading through the proxy, so it won't re-render even once the store publishes. It needs useUISettingsStore(s => s.reassignment.maxReplicationTraffic), which means a function component or a small subscribing wrapper around the slider + summary.

Worth noting the trap: giving BandwidthSlider local useState for its own value would make the thumb feel instant and look fixed, while the summary kept lagging — so it'd hide half the bug. The store write is the thing to fix.

Not introduced here. Master has the identical assignment on its L146, master's Chakra slider is equally controlled (value={sliderValue}), and neither file uses observer. I did not measure whether Chakra's slider masked it better with internal optimistic state, so the perceived severity may have changed even though the wiring didn't. Non-blocking for this PR — flagging because this is the file that owns the line, and because the same nested-mutation pattern likely affects other uiSettings.<section>.<field> = … writes across the app.

[SpicyPete]

Fixed in 03e7b12, both halves as you specified — and you were right that a local useState in BandwidthSlider would have hidden half the bug, so it stayed out.

• Notify on write: the handler now assigns the whole section, so it goes through the proxy's top-level trap: uiSettings.reassignment = { ...uiSettings.reassignment, maxReplicationTraffic: x }.
• Subscribe on read: StepReview is now a small function wrapper that reads useUISettingsStore((s) => s.reassignment.maxReplicationTraffic) and passes it to the class (renamed StepReviewContent), so the slider and the summary both come off the same subscribed value. step3-review.test.tsx asserts the summary flips from disabled to 1 kiB/s on a store write with no forced re-render; it fails on the pre-fix code.

I kept it in this PR rather than the flagged pile, on your own last point: the wiring is pre-existing, but master's Chakra slider masked half of it with internal drag state while the Registry Slider is fully controlled — so the migration is what froze the thumb, not just the summary. That makes it a perceived regression from this PR even though the store write never changed.

Your "same pattern likely affects other uiSettings.<section>.<field> = … writes" is correct — seven remain in four files. Three are benign (topic-list-new, schema-list, connect/cluster-details drive their UI from URL or local state and read the setting only at mount). rp-connect/pipelines-list.tsx:185/187 is live: it reads uiSettings.pipelinesList.quickSearch in render and writes it nested, inside a PageComponent that only re-renders on api-store writes — and Chakra's SearchField is fully controlled, so typing in that box does not appear until the next pipelines refresh. That file is still Chakra and belongs to the queued rp-connect PR, so it is on the tracker to fix there rather than in this diff.

The submit failure from your other comment is split out into #2644, since none of those lines is a migration hunk.

[eblairmckee]

"Start Reassignment" fails silently whenever the bandwidth throttle is above the minimum

Also pre-existing — reassign-partitions.tsx is in this PR's diff but none of the lines below were touched, and the code sits outside the hunks so I can't anchor inline. Raising it here because it's the most impactful thing I hit while testing the wizard, and this is where reviewers are looking.

Symptom. Click Start Reassignment with any throttle above the slider minimum: a toast flashes for a fraction of a second, then nothing. No error, no alert, nothing in the console, and the wizard stays on step 3. Re-clicking behaves the same. The only way to submit is to drag the slider to its minimum.

Why the throttle step fails. setTrafficLimit patches leader.replication.throttled.rate / follower.replication.throttled.rate per broker. I reproduced that call directly against a 3-broker Redpanda cluster:

PATCH /api/operations/configs → HTTP 200
{"patchedConfigs":[
  {"error":"INVALID_CONFIG: Setting broker properties on named brokers is unsupported","resourceName":"0","resourceType":4},
  {"error":"INVALID_CONFIG: Setting broker properties on named brokers is unsupported","resourceName":"1","resourceType":4},
  {"error":"INVALID_CONFIG: Setting broker properties on named brokers is unsupported","resourceName":"2","resourceType":4}]}

Redpanda doesn't support per-broker property patches — it uses cluster config. So the throttle can never be applied on Redpanda, for every broker, every time. Note the HTTP 200: the failure is per-resource in the body, so nothing rejects at the fetch layer.

Why it's silent. setTrafficLimit detects it correctly, then discards it (reassign-partitions.tsx ~L677):

} catch (_err) {
  closeToast(toastRef);   // ← the flash: loading toast opened, then closed
  return false;           // ← error object dropped
}

startReassignment propagates the false with no message (~L547):

const success = await this.setTrafficLimit(request);
if (!success) {
  return false;
}

and onNextPage has if (success) { … } with no else. Three layers, no user-visible outcome. startReassignment's own catch (_err) { closeToast(toastRef); return false; } (~L599) has the same shape — and note the sibling handler's copy promises "See console for more information" while nothing is ever logged.

Suggested fixes, smallest first:

1. Surface the error instead of dropping it:

} catch (err) {
  closeToast(toastRef);
  showToast({
    status: 'error',
    title: 'Could not set bandwidth throttle',
    description: err instanceof Error ? err.message : String(err),
    duration: 6000,
  });
  console.error('setTrafficLimit failed', err);
  return false;
}

The INVALID_CONFIG text is genuinely useful to an operator, so it's worth passing through rather than replacing with a generic string. Same treatment for startReassignment's catch.

2. Give onNextPage an else so a false return can never be a no-op — a submit button that does nothing is the worst failure mode here.

3. Decide whether the throttle should be offered on Redpanda at all. It cannot work there, and the codebase already branches on api.isRedpanda elsewhere in this feature (the non-Redpanda warning in Active Reassignments). Disabling the slider with a short explanation would stop people hitting an unreachable path — worth a product call rather than assuming.

Happy to split this into its own issue if you'd rather keep the PR scoped; it's a functional gap on Redpanda rather than anything about the Registry migration.

[SpicyPete]

Happy to split this into its own issue if you'd rather keep the PR scoped

Split into #2644 — frontend: stop the reassignment wizard failing silently when the bandwidth throttle is set, straight off master since the showToast façade means those catch blocks are textually identical there. Your fixes 1 and 2 are in it: both catches surface err.message (the INVALID_CONFIG text passes through rather than being replaced with a generic string, as you suggested) and log, and the handler whose copy promises "See console for more information" now actually logs. No blanket else in onNextPage — with both catches toasting, every return false path now has a message, so an unconditional else would double up on the "all errors" path that already opens the error modal.

Fix 3 I have deliberately left alone: whether the throttle should be offered on Redpanda at all is a product call, not a bug fix. It cannot work there for any broker, and the feature already branches on api.isRedpanda for the Active Reassignments warning, so disabling the slider with a short explanation is small — but it is someone's decision to make. Flagged on the tracker with your repro.

Your other two findings are fixed in this PR (03e7b12) with replies on each thread. Thank you for the throughput on this one — the response-body paste is what made the silent path obvious.