Skip to content

[Chakra exit · Phase 2 · PR 9/16] frontend: the rp-connect pages on Registry components, plus a secrets spec - #2645

Merged
SpicyPete merged 10 commits into
masterfrom
chakra-exit/09-rp-connect
Sep 11, 2026
Merged

[Chakra exit · Phase 2 · PR 9/16] frontend: the rp-connect pages on Registry components, plus a secrets spec#2645
SpicyPete merged 10 commits into
masterfrom
chakra-exit/09-rp-connect

Conversation

@SpicyPete

@SpicyPete SpicyPete commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Takes rp-connect off Chakra: nine files under pages/rp-connect, plus the deletion of utils/legacy-data-table.ts (its last consumer was here) and hooks/use-pagination-params.ts. Rebased onto master, 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.

One caveat worth knowing before you read the logs table. pipelines-details renders MessagePreview and ExpandedMessage out of topics/Tab.Messages/, and those are still Chakra until #2647 merges. So this clears rp-connect by the import metric, and the Value column of the logs table still paints Chakra Flex/Text at runtime until that PR lands. Nothing to do here — #2647 is what fixes it — but the area is not visually done on this branch alone.

Opened off master; UX-179 (#2624) rebases onto this.

Reviewer's guide

File What to look at
secrets-create/-update.tsx FormFieldField. Chakra generated the input id and wired the label; the Registry Field does not, so every control has an explicit id/htmlFor. FieldLabel required only paints an asterisk, so required is passed to the control too. PasswordInputInput type="password", which carries its own reveal toggle.
secrets-quick-add.tsx The creatable Select → the Registry Combobox (creatable + onCreateOption), ModalDialog, useDisclosureuseState. A pending new name is deliberately not added to options: 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. resolveLabel already falls back to the raw value, so the pending name displays without one.
pipelines-list.tsx Pagination keeps legacy parity — initialState at DEFAULT_TABLE_PAGE_SIZE plus pagination={rows.length > DEFAULT_TABLE_PAGE_SIZE}, matching transforms-list and connect/overview. Chakra's own defaultPageSize was 50 and it showed the footer only past that. Columns hoisted to module scopePageComponent force-updates on every api-store write, so an inline array re-creates every header and cell on each. SearchField → the shared SearchInput, which restores /-to-focus and Escape-to-clear.
pipelines-details.tsx The last legacy DataTable. The logs table gets back the expander column Chakra injected for subComponent, a DataTableColumnHeader on Timestamp (the Registry sorts only through that component), and a getRowId keyed on partition + offset so an expanded row stays on its message when the quick-search changes.
pipelines-create/-edit.tsx NumberInputInput type="number" showStepControls. Chakra's onChange was (valueAsString, valueAsNumber), so the old code stored a string; it now reads e.target.value. No step="any" — the Registry coerces step with Number(). The Registry Input enforces neither min nor max and its steppers write through the native value setter, so the compute-units field keeps a draft string and clamps via clampTasks on blur — clamping per keystroke makes the field unclearable, which pipelines-create.test.tsx pins.
redpanda-connect-intro.tsx Registry typography + DynamicCodeBlock. The Registry Alert renders its own icon and AlertDescription is a grid, so each hint line is a block child.

Two deliberate behaviour changes

  • Chakra's ButtonGroup spaced its children; the Registry's attaches them (strips inner radii and borders). Both secret forms use flex gap-2 instead, which is what they looked like before.
  • The logs table's URL-seeded pagination is gone. usePaginationParams read ?page/?pageSize and then handed control to the table, so only the 10-a-page default carries over. This matches the two sibling logs tables already on master (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 Registry Alert already renders a default <InfoIcon>, and on a destructive alert that default was the wrong glyph anyway. Now icon={<AlertCircle …>}.
  • hooks/use-pagination-params.ts is 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. nuqs stays: 19 other modules use it.

Coverage

rp-connect had 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 no SecretService, so Features.pipelinesApi is false and /connect-clusters renders 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 are required, the value field is a password, submit stays disabled until valid, and the duplicate/invalid name errors render with role="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:check clean · lint:check clean on every changed file, with the colour codes stripped — ultracite wraps path:line:col in ANSI escapes, so a path-anchored filter silently matches nothing · 997 unit · 1431 integration · the new e2e spec passes in the console variant.

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.

  1. secrets-quick-add inserts a broken reference for a newly created secret. The existing-secret path emits ${secrets.NAME}; the new-secret path emits a bare secrets.NAME with no interpolation wrapper. It also uses the name as typed while creating the secret uppercased, so mySecret is stored as MYSECRET but inserted as secrets.mySecret. Two bugs in three lines, both untouched by this diff.
  2. A failed secret create leaves the submit button spinning. setIsCreating(false) sits after an early return, so a rejected create leaves Select disabled behind its spinner and Cancel disabled too — the only exit is the dialog's close X. Verbatim on master.
  3. max/min on the secret-name input are inert. They only apply to number, range and date inputs; on type="text" the attribute needed is maxLength. Carried over unchanged from master, which had the same max={255}.
  4. The Registry Combobox exposes no id or aria-label, so the secret-name FieldLabel cannot be associated with its input. Needs an upstream prop — the label is marked in the source. Same for FieldDescription, which the Registry Field never wires into aria-describedby.
  5. These forms stay useState-based rather than react-hook-form + Zod as AGENTS.md asks 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

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

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

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

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.

@SpicyPete
SpicyPete requested review from a team, datamali, eblairmckee, weeco and yougotashovel and removed request for a team September 10, 2026 00:48
@SpicyPete SpicyPete self-assigned this Sep 10, 2026
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>
SpicyPete and others added 7 commits September 11, 2026 07:03
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 and others added 2 commits September 11, 2026 08:02
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>
@SpicyPete SpicyPete changed the title [Chakra exit · Phase 2 · PR 9/14] frontend: the rp-connect pages on Registry components, plus a secrets spec [Chakra exit · Phase 2 · PR 9] frontend: the rp-connect pages on Registry components, plus a secrets spec Sep 11, 2026
@SpicyPete SpicyPete changed the title [Chakra exit · Phase 2 · PR 9] frontend: the rp-connect pages on Registry components, plus a secrets spec [Chakra exit · Phase 2 · PR 9/16] frontend: the rp-connect pages on Registry components, plus a secrets spec Sep 11, 2026

@eblairmckee eblairmckee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📺 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>
@SpicyPete

Copy link
Copy Markdown
Contributor Author

Fixed that, and found a few other areas it wasn't using semantically correct colours

@SpicyPete
SpicyPete enabled auto-merge (squash) September 11, 2026 18:26
@SpicyPete
SpicyPete merged commit 42c0473 into master Sep 11, 2026
24 of 25 checks passed
@SpicyPete
SpicyPete deleted the chakra-exit/09-rp-connect branch September 11, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants