Connect now uses new ListComponentsRequestSchema to fetch components + UI improvements #2043
Conversation
frontend/src/components/pages/rp-connect/onboarding/onboarding-wizard.tsx
Show resolved
Hide resolved
| if (readonly) { | ||
| return ( | ||
| <Card size="full"> | ||
| <CardContent> | ||
| <div className="flex items-center gap-4"> | ||
| <Label>Pipeline Name</Label> | ||
| <Text>{name}</Text> | ||
| </div> | ||
|
|
||
| <Accordion collapsible type="single"> | ||
| <AccordionItem value="advanced" variant="outlined"> | ||
| <AccordionTrigger>Advanced Settings</AccordionTrigger> | ||
| <AccordionContent className="space-y-4"> | ||
| <div className="space-y-2"> | ||
| <Label>Description</Label> | ||
| <Text>{description || '-'}</Text> | ||
| </div> | ||
|
|
||
| <div className="space-y-2"> | ||
| <Label>Compute Units: {computeUnits}</Label> | ||
| <Text variant="muted">One compute unit = 0.1 CPU and 400 MB memory</Text> | ||
| </div> | ||
| </AccordionContent> | ||
| </AccordionItem> | ||
| </Accordion> | ||
| </CardContent> | ||
| </Card> | ||
| ); | ||
| } | ||
|
|
| } | ||
|
|
||
| return ( | ||
| <Card size="full"> |
There was a problem hiding this comment.
CC @birdayz should we rinse & repeat for AI features too
There was a problem hiding this comment.
Well, can we also have an input in addition?
For this use case a direct numeric input would be more practical. Users typically have a specific number in mind rather than needing to explore a range. Having to drag the slider to find an exact value adds unnecessary friction compared to just typing the number directly.
Could we add a text input field alongside the slider, or make the slider value editable?
There was a problem hiding this comment.
Oooh, good point. I'll add a number input next to the slider.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Connect pipeline UI by migrating from hardcoded Benthos JSON schemas to dynamic backend queries, improving UX with better validation, secrets handling, and linting while decoupling the new UI from legacy code.
Key Changes:
- Replaced hardcoded Benthos schemas with
ListComponentsRequestSchemabackend API, eliminating schema coercion code - Enhanced secrets detection with automatic normalization to screaming snake case and smart replacement in YAML editor
- Improved wizard validation with per-step validity tracking and disabled "Next" buttons for incomplete forms
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/react-query/api/connect.tsx |
New queries for component lists, config schema, and pipeline linting via backend API |
frontend/src/react-query/api/pipeline.tsx |
Added cache invalidation for getPipeline queries on start/stop mutations |
frontend/src/hooks/use-debounced-value.ts |
New hook for debouncing values with configurable delay |
frontend/src/components/ui/yaml/yaml-editor.tsx |
Dynamic schema injection from props with Monaco YAML autocomplete |
frontend/src/components/ui/yaml/yaml-editor-card.tsx |
Pass-through schema prop to YAML editor component |
frontend/src/components/ui/secret/quick-add-secrets.tsx |
Secret name normalization, duplicate detection, and editor content auto-update |
frontend/src/components/ui/lint-hint/lint-hint-list.tsx |
Memo optimization, loading state support, and improved styling with Pre component |
frontend/src/components/redpanda-ui/components/typography.tsx |
Added dense variant to Pre component for compact code displays |
frontend/src/components/redpanda-ui/components/spinner.tsx |
Size variants (xs/sm/md/lg/xl) added with CVA |
frontend/src/components/redpanda-ui/components/button.tsx |
New destructiveOutline variant and icon prop support |
frontend/src/components/redpanda-ui/components/accordion.tsx |
Variants system (default/contained/outlined) for different styling contexts |
frontend/src/components/pages/rp-connect/utils/yaml.ts |
Error handling improvements for merge/YAML generation failures |
frontend/src/components/pages/rp-connect/utils/wizard.ts |
Components parameter added to template regeneration |
frontend/src/components/pages/rp-connect/utils/schema.ts |
Complete rewrite to use proto types, new field visibility logic, debug logging |
frontend/src/components/pages/rp-connect/utils/categories.ts |
Type signature updated for ConnectComponentSpec |
frontend/src/components/pages/rp-connect/utils/__fixtures__/component-schemas.ts |
Mock component fixtures for testing with new schema structure |
frontend/src/components/pages/rp-connect/types/schema.ts |
Schema types aligned with proto definitions, removed legacy types |
frontend/src/components/pages/rp-connect/types/constants.ts |
Added topics to critical fields, removed managed components filter |
frontend/src/components/pages/rp-connect/types/wizard.ts |
Schema adjustments for optional connection data |
frontend/src/components/pages/rp-connect/pipelines-*.tsx |
Feature flag routing to new PipelinePage component |
frontend/src/components/pages/rp-connect/pipeline/*.tsx |
New unified pipeline page with create/edit/view modes, toolbar, and form validation |
frontend/src/components/pages/rp-connect/onboarding/*.tsx |
Wizard improvements with validity tracking, components prop threading, and sidebar refactoring |
frontend/.claude/skills/frontend-developer/SKILL.md |
New skill documentation for frontend development patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
frontend/src/components/pages/rp-connect/onboarding/connect-tiles.tsx
Outdated
Show resolved
Hide resolved
| cardinality: 'infinite', | ||
| }), | ||
| }); | ||
| await queryClient.invalidateQueries({ |
There was a problem hiding this comment.
I think it was deleted before because some people preferred to have the invalidation happen in the component layer, but let's stick to the pattern we established before and keep it tied to the hook.
| <Alert variant="destructive"> | ||
| <AlertTriangle className="h-4 w-4" /> | ||
| <AlertDescription> |
| if (onError) { | ||
| onError([errorMessage]); | ||
| } else { | ||
| toast.error(errorMessage); | ||
| } |
There was a problem hiding this comment.
Should we show a toast error even if onError callback exists?
There was a problem hiding this comment.
wanted to make it more custom, given this is a reusable component that we use in multiple features. more like a design system component API. in some cases I assume we may not want to show a toast. eg: if it's in a modal, and we don't want to dismiss modal, we may want to surface an alert.
| </Text> | ||
| <Text className="rounded border bg-white px-2 py-1 font-mono text-gray-800 text-sm leading-relaxed"> | ||
| </Pre> | ||
| <Pre variant="dense"> |
There was a problem hiding this comment.
👏 Maybe we should also use DynamicCodeBlock?
There was a problem hiding this comment.
this is a very temporary fix, as in v2 of this I'm actually going to have a logger and completely different lint interface
| toast({ | ||
| status: 'error', | ||
| duration: null, | ||
| isClosable: true, | ||
| title: 'Failed to create pipeline', | ||
| description: formatPipelineError(err), | ||
| }); |
There was a problem hiding this comment.
Let's try to create a ConnectError with ConnectError.from(error), if possible, and follow the format function. the description could be kept intact with formatPipelineError
There was a problem hiding this comment.
this is also coming in v2, where I completely rewrite error and lint handling/UI. this is just a temp fix.
There was a problem hiding this comment.
also this is legacy code, let's not focus on this page, given it's basically getting reverted (with minor improvements) to old UI)
There was a problem hiding this comment.
ended up using ConnectError.from but still using legacy formatPipelineError because it formats lint errors, and int he legacy UI we show those in toasts
| * by the Apache License, Version 2.0 | ||
| */ | ||
|
|
||
| import { useLocation, useParams } from 'react-router-dom'; |
There was a problem hiding this comment.
If we were to update our router, do you think there is any way to abstract this to make it less painful? Just wondering. Nothing actionable
| toast({ | ||
| status: 'success', | ||
| title: 'Pipeline started', | ||
| duration: 4000, | ||
| isClosable: false, | ||
| }); |
There was a problem hiding this comment.
Do we need to do this? I think the state transition should happen automatically, so the user would see the pipeline already starting
| DialogHeader, | ||
| DialogTitle, | ||
| } from 'components/redpanda-ui/components/dialog'; | ||
| import { Group } from 'components/redpanda-ui/components/group'; |
There was a problem hiding this comment.
I think this component is deprecated now, can we use ButtonGroup?
There was a problem hiding this comment.
can do that later when we migrate all of them
| # Check if component exists | ||
| ls src/components/redpanda-ui/components/ |
There was a problem hiding this comment.
Maybe say if file exists and then ls src/components/redpanda-ui/ because we may also have hooks or other directories.
frontend/src/components/pages/rp-connect/onboarding/add-secrets-card.tsx
Outdated
Show resolved
Hide resolved
| {(component.status === ComponentStatus.BETA || | ||
| component.status === ComponentStatus.EXPERIMENTAL || | ||
| component.status === ComponentStatus.DEPRECATED) && | ||
| component.name !== 'redpanda' && ( |
There was a problem hiding this comment.
Is this to prevent the beta status badge on redpanda component?
There was a problem hiding this comment.
yes. there's an open item I have to resolve this once David Yu is back, we're stil deciding what components are beta/experimental and if we even want to show them in this UX
|
|
||
| const handleCancel = useCallback(() => { | ||
| onCancel?.(); | ||
| navigate(-1); |
There was a problem hiding this comment.
Do we have access to goBack in this version of react-router-dom?
| {/* Delete confirmation dialog */} | ||
| <Dialog onOpenChange={setIsDeleteDialogOpen} open={isDeleteDialogOpen}> | ||
| <DialogContent size="md" variant="destructive"> | ||
| <DialogHeader> | ||
| <DialogTitle>Delete Pipeline</DialogTitle> | ||
| <DialogDescription> | ||
| Are you sure you want to delete <strong>{pipelineName}</strong>? This action cannot be undone. | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| <DialogFooter> | ||
| <Button onClick={() => setIsDeleteDialogOpen(false)} variant="outline"> | ||
| Cancel | ||
| </Button> | ||
| <Button disabled={deleteMutation.isPending} onClick={handleDeleteConfirm} variant="destructive"> | ||
| {deleteMutation.isPending ? 'Deleting...' : 'Delete'} | ||
| </Button> | ||
| </DialogFooter> | ||
| </DialogContent> | ||
| </Dialog> |
There was a problem hiding this comment.
Use frontend/src/components/ui/delete-resource-alert-dialog.tsx
There was a problem hiding this comment.
that's a weird pattern to include the trigger in something called a dialog.... left it as is for now, but we should rethink the naming. I also added a triggerVariant because I need to render it as a button and not a dropdown menu item
Changes since original pull requestKey Changes1. Less Opinionated Defaults + Better GuardingField visibility and values controlled by backend schema with intelligent guardrails (previously we had a lot of frontend overrides which would cause diversions from docs representation and lint):
Example output: transaction_isolation_level: read_uncommitted # Required (default: "read_uncommitted")
consumer_group: "" # Required if topic partition is not specified
auto_replay_nacks: false # Required2. Comprehensive Error HandlingAdded try/catch with toast notifications throughout schema → config → YAML pipeline. Custom option always available so users can skip wizard on any error. Schema Parsing (
Schema to Config (
YAML Generation (
Pattern: All errors use consistent toast format: toast.error(
formatToastErrorMessageGRPC({
error: ConnectError.from(error),
action: "Parse component schema",
entity: "Component list",
})
);Impact: Now that's we're relying solely on the backend, breaking backend changes won't crash UI. Users get actionable feedback and can recover. "Custom" is ALWAYS available for users to build their own configs from scratch, regardless of backend state. 3. New Connect UI: Decoupled + Feature FlaggedRipped out all new Connect UI from legacy code. Now lives in Rendering: if (isEmbedded() && enableRpcnTiles) {
// Show new UI
} else {
// Show legacy UI or intro screen
}Rollback: Toggle Decoupling: Zero shared dependencies with MobX/Chakra legacy code. 4. Fixed Redirect Regression (legacy UI)Problem: Root Cause: MobX state timing issue caused:
Fix: Removed redirect logic entirely. Users always land on Connect overview page with proper tabs. File: 5. Reactive UI StatesAll async operations show loading/error/success states via toasts: Loading States:
Error Surfacing:
Success Confirmations:
Pattern: Toast on every user action. No silent failures. 6. Test CoverageWhat's Covered:
What's NOT Tested (future work once Jan merged the container PR for e2e tests):
Intended "Breaking" Changes
Rollback StrategyToggle |
There was a problem hiding this comment.
If that's ok I will commit this separately to our repo so we can start using it sooner
There was a problem hiding this comment.
tbh, in an ideal world, I'd love to have a repo just for claude: skills, commands, etc. that we can pull/update into consuming repos similar to the ui-registry (or we can explore pulling into ui-registry given these are all frontend focused). let's discuss when you're back
| }); | ||
| }; | ||
|
|
||
| export const useLintPipelineConfigQuery = ( |
There was a problem hiding this comment.
Please check if this hook is also in the pipeline.tsx file, maybe we can remove 1 or the other.
There was a problem hiding this comment.
It's not, we use a different lint endpoint with the new grpc endpoint
There was a problem hiding this comment.
Have we already moved it to the UI registry as well?
There was a problem hiding this comment.
no. i do an upstream dump every few weeks
🚨 Breaking Change: ConnectTiles Federated Module
Why: Enables federated module usage without backend API context dependency. Parent components now control data fetching. Other Improvements
Better Error Handling
Migration for Federated Consumers// Before: ConnectTiles managed its own query
<ConnectTiles componentTypeFilter={['input']} />
// After: Parent manages query, passes data down
const { data, isLoading } = useListComponentsQuery();
<ConnectTiles components={data?.components} isLoading={isLoading} componentTypeFilter={['input']} /> |
| version: '', | ||
| examples: [], | ||
| footnotes: '', | ||
| $typeName: 'redpanda.api.dataplane.v1.ComponentSpec', |
There was a problem hiding this comment.
I agree with Ben, please use create from protobuf, it helps to make correct default values in a deep object, or, is there a big reason to don't use?
… on backend schema than opinionated defaults and what fields are actually shown
…oint, better progressive loading states, no more silent errors, better conditional rendering of new UI withini legacy entry point
eblairmckee
force-pushed
the
feat/schema-fix
branch
from
6fa2007 to
1cd29e0
Compare
|
Summary of RPCN API calls: OldBoth with List Connect component schemas
Lint pipelineOn create
On Edit
New✨[NEW!] List Connect component schemas
Lint pipelineOn create
On Edit
✨[NEW!] On editor value change(while user is typing in either edit or create mode) |
This PR addresses a vast majority of bug bash reports, specifically the "major" ones.
Please see latest comment for most recent changes and how we're better guarding against known regressions (and potential future ones due to this entirely new UX that's dependent on the backend)
key changes
replaces hard coded benthos json schemas with new backend query
This did introduce some breaking changes, which affected how we parse the schema, however, it results in eliminated hundreds of lines of code where we had to coerce the schema to meet our expectations discovered during bug bashing. this is a much more simplified approach, and we can use the proto types instead of inferring types from the schema.
However, if the backend schema changes, and parser is unable to parse the schema it won't show any RPCN components (it will still show "custom" and "additional components", though, ensuring connect is always usable)
component schema changes (ComponentSpec)
field schema changes (FieldSpec)
less opinionated defaults
this was a pretty contentious topic, and throughout the early iterations kept flip-flopping on whether we should provide default values for certain fields on
redpandacomponents. those are almost all gone (brokers still gets a default) and we're relying solely on the schema for what fields to surface based on if they are not optional, and not advanced.follow up
We should look at what endpoint the docs site uses, because currently it appears to use the old schema, which will be very confusing to users if they try to use the new UI (see schema changes above). As well as what's considered "common" vs "advanced" (very different in new schema).
better linting
We're now linting (debounce) on yaml editor content changes, and validating against the new backend API (same as the one where we pull component schemas from).
Once I fix the monaco intellisense UI, we'll be able to get autocompletion based on the connect components definitions and properties.
lint ui
Improved styling, however, will be completely revisiting this in the next iteration (form-based UI) so minimal changes for now.
new Connect UI is decoupled from legacy
ripped out all new connect UI from the old and it now lives in its own directory. new UI is conditionally rendered when create/edit/view pages are mounted if
enableRpcnTilesfeature flag is true.better wizard step validation
"Next" button is disabled until current wizard step is valid/populated. Given all steps are skippable, this makes it more clear, as previously Next button was always enabled regardless of form state.
Also fixed the "create" buttons to be more responsive to query loading states to eliminate this bug:
better secrets detection
when user references a secret variable that doesn't exist, we suggest creating one, and automatically convert invalid formatting into correct format (screaming snake case) and replace the invalid secret variable with the correct one after user creates the secret.
better loading state when adding secrets. previously when you clicked "create" both 'missing secrets' and 'create new secret' submit buttons would load, which was confusing. now only the relevant one loads, and they are nested within their own individual
Formcomponents.better error handling when updating yaml
previously when adding a new input/output/processor if existing yaml was invalid, it would convert it to json 😬 now we just surface a toast that you need to fix the yaml.
swapped out tiles in "top 12" based on alex's feedback