[Chakra exit · Phase 2 · PR 9/16] frontend: the rp-connect pages on Registry components, plus a secrets spec - #2645
Merged
Merged
Conversation
Contributor
✅ Clean — no registry drift, off-token colours, or ad-hoc classesApp:
Generated by lookout audit-changes. |
SpicyPete
requested review from
a team,
datamali,
eblairmckee,
weeco and
yougotashovel
and removed request for
a team
September 10, 2026 00:48
SpicyPete
added a commit
that referenced
this pull request
Sep 10, 2026
Four behaviour findings from the review of #2645: - the logs table had no `getRowId`, so expanded-row state was keyed by array index and re-pointed at a different message whenever the quick search changed. Both sibling logs tables already set it. - the secret Combobox appended a pending new name to its own options, which made re-clicking it a deselect that cleared the typed value and silently switched to the existing-secret path. It also suppressed the Create item. `resolveLabel` already falls back to the raw value, so the append was never needed. - the compute-units field clamped in `onChange`, so it could not be cleared: the empty value snapped back to the minimum and the next digit appended to it. The field now holds a draft string and clamps on blur, as Chakra's NumberInput did, while the committed number stays clamped for submit. - Edit and Update lost Chakra's `solid` fill to the Registry's neutral default, while the sibling Create page kept `variant="primary"`. A page-level spec pins the compute-units wiring; `clampTasks`'s unit tests cannot see it, and reverting the fix fails two of the four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SpicyPete
added a commit
that referenced
this pull request
Sep 10, 2026
…tion hook Both were flagged on #2645 as pre-existing and are now folded in by request. The Registry `Alert` renders its own icon, so `pipelines-create`'s `<AlertCircle>` child painted a second one beside the default `<InfoIcon>` — and on a `destructive` alert the default was the wrong glyph anyway. Passing it as `icon` fixes both. The sibling Alert in `pipelines-details` already did this; the one in the intro page takes the default deliberately. `hooks/use-pagination-params.ts` lost its last consumer when `pipelines-details` moved to the Registry pager, which owns page and pageSize itself. Deleted with its test — integration drops 1329 → 1325, its four tests. `nuqs` stays: 19 other modules use it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Modals become Registry Dialogs, the secret forms become Field + Input (the Registry Input carries its own reveal toggle for type=password), and the creatable secret Select becomes the Registry Combobox. The pipelines list keeps its 10-a-page default and its always-on pager, hoists its columns array out of render, and gets the shared SearchInput back with the `/` and Escape shortcuts. Its quick-search setting now goes through `updateSettings` and is read with `useUISettingsStore`: a nested write on the `uiSettings` proxy notifies nobody, so the search box only updated on the next pipelines refresh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pipeline create/edit forms become Field + Input; Chakra's NumberInput becomes a number Input with step controls, taking its value from the DOM event rather than Chakra's (valueAsString, valueAsNumber) pair. The details page loses the last legacy DataTable. Its logs table gets the expander column Chakra used to inject for `subComponent`, a sortable Timestamp header, and the shared SearchInput. Sorting and paging are now the table's own: the legacy `pagination` prop seeded page and pageSize from the URL and then handed control to the table anyway, so only the 10-a-page default carries over — matching the two sibling logs tables. `utils/legacy-data-table.ts` existed only to type that table's columns and goes with it. The intro page moves onto Registry typography, the Registry Alert (which renders its own icon) and DynamicCodeBlock. Coverage: an integration spec for the secrets form, which no variant can reach live — the OSS config declares no SecretService — and a Playwright smoke spec for the intro page, which it can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ings The Registry Input does not enforce min/max the way Chakra's NumberInput did: its steppers set the value outright, so three clicks down from 1 reach -2, and a typed 999 stays 999 where Chakra clamped it to 72. Both pipeline forms fed that straight into tasksToCPU, so cpuShares: '0' or '-200m' could reach CreatePipeline and UpdatePipeline. clampTasks now guards both, with a test. Also: the list's three columns with no DataTableColumnHeader had sorting left on with nothing to trigger it; the two secret submit buttons lose their label to isLoading and needed a name of their own; the details error Alert was painting the default info icon; and four `cond && <FieldError>` lines tripped noLeakedRender, which fix-mode lint reports but cannot fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
useTopLevelRegex fires on inline regexes in locators and label queries; ten of them across the two files this PR adds. The existing reassign spec already keeps its matchers as module constants. These were missed on the first pass because the local lint:check filter anchored on the path at line start, and ultracite's finding lines are ANSI-wrapped — so a real error read as clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four behaviour findings from the review of #2645: - the logs table had no `getRowId`, so expanded-row state was keyed by array index and re-pointed at a different message whenever the quick search changed. Both sibling logs tables already set it. - the secret Combobox appended a pending new name to its own options, which made re-clicking it a deselect that cleared the typed value and silently switched to the existing-secret path. It also suppressed the Create item. `resolveLabel` already falls back to the raw value, so the append was never needed. - the compute-units field clamped in `onChange`, so it could not be cleared: the empty value snapped back to the minimum and the next digit appended to it. The field now holds a draft string and clamps on blur, as Chakra's NumberInput did, while the committed number stays clamped for submit. - Edit and Update lost Chakra's `solid` fill to the Registry's neutral default, while the sibling Create page kept `variant="primary"`. A page-level spec pins the compute-units wiring; `clampTasks`'s unit tests cannot see it, and reverting the fix fails two of the four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment-only. The two spec preambles and the clamp notes had grown into rationale; each is now the constraint alone, with the reasoning left to the PR. Drops the comments that restated their own line — the seeded skeleton, the password type, "still disabled", "Pipeline List". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion hook Both were flagged on #2645 as pre-existing and are now folded in by request. The Registry `Alert` renders its own icon, so `pipelines-create`'s `<AlertCircle>` child painted a second one beside the default `<InfoIcon>` — and on a `destructive` alert the default was the wrong glyph anyway. Passing it as `icon` fixes both. The sibling Alert in `pipelines-details` already did this; the one in the intro page takes the default deliberately. `hooks/use-pagination-params.ts` lost its last consumer when `pipelines-details` moved to the Registry pager, which owns page and pageSize itself. Deleted with its test — integration drops 1329 → 1325, its four tests. `nuqs` stays: 19 other modules use it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SpicyPete
force-pushed
the
chakra-exit/09-rp-connect
branch
from
September 11, 2026 14:04
402c3ce to
3b22227
Compare
The swap pinned `pageSize: 10` and passed a bare `pagination`, which is
neither what the legacy table did nor what the sibling list pages do.
Chakra's DataTable declares `defaultPageSize = 50` and renders its
footer only when `data.length > defaultPageSize`, so master showed 50
pipelines a page with no pager under 50 rows.
Now on the pattern `transforms-list` and `connect/overview` already
use: `initialState` at `DEFAULT_TABLE_PAGE_SIZE` plus
`pagination={rows.length > DEFAULT_TABLE_PAGE_SIZE}`. With the
100-pipelines-per-cluster quota this was up to ten pages where there
had been two. The logs table stays at ten a page, which is what its
two sibling logs tables do.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pty state Comment-only except one prop: `emptyText=""` blanked the Registry DataTable's default "No results.", so filtering the pipelines list to nothing showed an unexplained empty table. Chakra rendered its own empty state off the filtered data, so dropping the prop restores a message. Also drops three copies of the same `enableSorting: false` note — the flag says it — and shortens the two spec preambles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eblairmckee
approved these changes
Sep 11, 2026
eblairmckee
left a comment
Contributor
There was a problem hiding this comment.
📺 tested locally
only minor issue was the plus icon in the "add secrets" button on the create/edit pipeline page is hard coded stroke="white"
From @eblairmckee's review: the plus icon in the "Add secrets" button was `color="white"` on a `variant="outline"` button, which is `bg-transparent text-secondary`. So it was white on the page's white ground - invisible in light mode, not merely off-token. Dropping the prop lets it inherit the button's own label colour. Sweeping the rest of the diff for the same shape turned up seven more, all on the pipeline status icons: `#444` for Starting, Stopping and Stopped, `green` for Running and Completed, `red` for Error and Unknown. `#444` on the dark ground (#161616) is close to invisible, and the CSS named colours are harsh on both. They now use `text-subtle`, `text-success` and `text-destructive`, which resolve per theme: subtle #606164 light / #9c9c9c dark success #276749 light / #2edf86 dark destructive #be1b0e light / #fa7477 dark Unknown stays red rather than becoming a warning - master had it red, and re-reading that status is a product call, not a colour swap. All eight predate this PR and were carried over verbatim; folded in by request rather than left for the follow-up list. Note the registry audit reported "0 off-token palette colours" on this diff - it reads classes, not `color=` props on components. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Fixed that, and found a few other areas it wasn't using semantically correct colours |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes rp-connect off Chakra: nine files under
pages/rp-connect, plus the deletion ofutils/legacy-data-table.ts(its last consumer was here) andhooks/use-pagination-params.ts. Rebased ontomaster, which is at 25, so this takes it to 15. With #2647 (topics) and #2649 (the security ACLs tab) that leaves five — all of them PR 13's own, which are deleted rather than migrated.Opened off
master; UX-179 (#2624) rebases onto this.Reviewer's guide
secrets-create/-update.tsxFormField→Field. Chakra generated the input id and wired the label; the RegistryFielddoes not, so every control has an explicitid/htmlFor.FieldLabel requiredonly paints an asterisk, sorequiredis passed to the control too.PasswordInput→Input type="password", which carries its own reveal toggle.secrets-quick-add.tsxSelect→ the RegistryCombobox(creatable+onCreateOption),Modal→Dialog,useDisclosure→useState. A pending new name is deliberately not added tooptions: an option matching the current selection makes re-clicking it a deselect, which would clear the typed secret, and it also suppresses the Combobox's own Create item.resolveLabelalready falls back to the raw value, so the pending name displays without one.pipelines-list.tsxinitialStateatDEFAULT_TABLE_PAGE_SIZEpluspagination={rows.length > DEFAULT_TABLE_PAGE_SIZE}, matchingtransforms-listandconnect/overview. Chakra's owndefaultPageSizewas 50 and it showed the footer only past that. Columns hoisted to module scope —PageComponentforce-updates on every api-store write, so an inline array re-creates every header and cell on each.SearchField→ the sharedSearchInput, which restores/-to-focus and Escape-to-clear.pipelines-details.tsxDataTable. The logs table gets back the expander column Chakra injected forsubComponent, aDataTableColumnHeaderon Timestamp (the Registry sorts only through that component), and agetRowIdkeyed on partition + offset so an expanded row stays on its message when the quick-search changes.pipelines-create/-edit.tsxNumberInput→Input type="number" showStepControls. Chakra'sonChangewas(valueAsString, valueAsNumber), so the old code stored a string; it now readse.target.value. Nostep="any"— the Registry coercesstepwithNumber(). The RegistryInputenforces neitherminnormaxand its steppers write through the native value setter, so the compute-units field keeps a draft string and clamps viaclampTaskson blur — clamping per keystroke makes the field unclearable, whichpipelines-create.test.tsxpins.redpanda-connect-intro.tsxDynamicCodeBlock. The RegistryAlertrenders its own icon andAlertDescriptionis a grid, so each hint line is a block child.Two deliberate behaviour changes
ButtonGroupspaced its children; the Registry's attaches them (strips inner radii and borders). Both secret forms useflex gap-2instead, which is what they looked like before.usePaginationParamsread?page/?pageSizeand then handed control to the table, so only the 10-a-page default carries over. This matches the two sibling logs tables already onmaster(transform-details,connector-details).Folded in by request
Two items originally flagged as pre-existing are now fixed here:
pipelines-create's Alert painted two icons —<AlertCircle>was passed as a child while the RegistryAlertalready renders a default<InfoIcon>, and on adestructivealert that default was the wrong glyph anyway. Nowicon={<AlertCircle …>}.hooks/use-pagination-params.tsis deleted, with its test — this page was its last consumer, and the Registry pager owns page and pageSize itself. Integration drops 1329 → 1325, exactly its four tests.nuqsstays: 19 other modules use it.Coverage
rp-connecthad no tests at all, and most of it cannot be reached by a live spec: the OSS variant's config declares no pipelines API and noSecretService, soFeatures.pipelinesApiis false and/connect-clustersrenders the intro instead of the list. So:secrets-create.test.tsx(integration, 4 tests) covers what the swap actually risked: both fields resolve by label text, both arerequired, the value field is a password, submit stays disabled until valid, and the duplicate/invalid name errors render withrole="alert".pipelines-create.test.tsx(integration, 4 tests) pins the compute-units field against the real component: it defaults to the minimum, survives being cleared, and clamps to each bound on blur.clampTasks's own unit tests cannot see the clear-then-type regression.rp-connect-intro.spec.ts(Playwright, 2 tests) covers the one reachable surface — the walkthrough, the hint alert, and the install-method select driving the code block. Verified green against a live container, not just written.Gates
type:checkclean ·lint:checkclean on every changed file, with the colour codes stripped — ultracite wrapspath:line:colin ANSI escapes, so a path-anchored filter silently matches nothing · 997 unit · 1431 integration · the new e2e spec passes in theconsolevariant.Pre-existing, flagged not fixed
Per the exit plan's rule — a swap PR whose diff is "identical rendering, different components" stays reviewable; the same PR with behaviour fixes folded in does not.
secrets-quick-addinserts a broken reference for a newly created secret. The existing-secret path emits${secrets.NAME}; the new-secret path emits a baresecrets.NAMEwith no interpolation wrapper. It also uses the name as typed while creating the secret uppercased, somySecretis stored asMYSECRETbut inserted assecrets.mySecret. Two bugs in three lines, both untouched by this diff.setIsCreating(false)sits after an earlyreturn, so a rejected create leavesSelectdisabled behind its spinner andCanceldisabled too — the only exit is the dialog's close X. Verbatim onmaster.max/minon the secret-name input are inert. They only apply to number, range and date inputs; ontype="text"the attribute needed ismaxLength. Carried over unchanged frommaster, which had the samemax={255}.Comboboxexposes noidoraria-label, so the secret-nameFieldLabelcannot be associated with its input. Needs an upstream prop — the label is marked in the source. Same forFieldDescription, which the RegistryFieldnever wires intoaria-describedby.useState-based rather than react-hook-form + Zod asAGENTS.mdasks of new code. Converting four forms is a rewrite, not a swap, and would make this diff unreviewable — worth its own PR.🤖 Generated with Claude Code