Skip to content

Commit f5262e9

Browse files
committed
Comment reduction pass
1 parent ea6bf42 commit f5262e9

13 files changed

Lines changed: 51 additions & 100 deletions

File tree

frontend/src/components/pages/rp-connect/onboarding/add-topic-step.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,8 @@ type AddTopicStepProps = {
5555
selectionMode?: 'existing' | 'new' | 'both';
5656
hideTitle?: boolean;
5757
className?: string;
58-
// Renders the form bare — no Card chrome, no min-height, no top margin — so
59-
// it can sit inside a host surface (e.g. a dialog body) that already provides
60-
// framing. The onboarding wizard keeps the default (false) for its full-page
61-
// step layout.
58+
// Renders the form bare (no Card chrome/min-height/margin) so it can sit
59+
// inside a host surface like a dialog body. Defaults to false for the wizard.
6260
inline?: boolean;
6361
};
6462

frontend/src/components/pages/rp-connect/onboarding/add-user-step.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,8 @@ type AddUserStepProps = {
8585
selectionMode?: 'existing' | 'new' | 'both';
8686
hideTitle?: boolean;
8787
className?: string;
88-
// Renders the form bare — no Card chrome, no min-height, no top margin — so
89-
// it can sit inside a host surface (e.g. a dialog body) that already provides
90-
// framing. The onboarding wizard keeps the default (false) for its full-page
91-
// step layout.
88+
// Renders the form bare (no Card chrome/min-height/margin) so it can sit
89+
// inside a host surface like a dialog body. Defaults to false for the wizard.
9290
inline?: boolean;
9391
};
9492

frontend/src/components/pages/rp-connect/pipeline/details-dialog.tsx

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,8 @@ type DetailsDialogProps = {
3232
open: boolean;
3333
onOpenChange: (open: boolean) => void;
3434
pipeline?: Pipeline;
35-
/**
36-
* Called when the user clicks Delete in the Danger Zone. The parent is
37-
* expected to close this dialog and open the delete confirmation — keeps
38-
* the two dialogs from stacking on top of each other.
39-
*/
35+
// Delete clicked in the Danger Zone. The parent should close this dialog and
36+
// open the delete confirmation so the two don't stack.
4037
onRequestDelete?: () => void;
4138
};
4239

@@ -46,8 +43,7 @@ const SectionHeading = ({ className, children }: { className?: string; children:
4643
<h3 className={cn('font-semibold text-base text-foreground', className)}>{children}</h3>
4744
);
4845

49-
// Copy button that only appears when the parent row is hovered. Keeps
50-
// non-copyable rows visually identical to copyable ones at rest.
46+
// Copy button that only appears on row hover.
5147
const HoverCopyButton = ({ value }: { value: string }) => (
5248
<CopyButton
5349
className="shrink-0 opacity-0 transition-opacity group-hover/row:opacity-100"
@@ -92,9 +88,8 @@ const renderValue = (value: string | undefined, children: React.ReactNode | unde
9288
);
9389
};
9490

95-
// Side-by-side row: fixed-width label on the left, value (or children) on the
96-
// right. Long string values can wrap (with `wrap`); short identifiers
97-
// truncate. Copy button appears on hover.
91+
// Fixed-width label on the left, value (or children) on the right. Values wrap
92+
// with `wrap`, else truncate; copy button appears on hover.
9893
const DetailRow = ({ label, value, children, copyable = false, wrap = false }: DetailRowProps) => {
9994
const showCopy = copyable && Boolean(value?.length);
10095
return (
@@ -120,8 +115,7 @@ const ComputeUnitsLabel = () => (
120115
</Tooltip>
121116
);
122117

123-
// Reference rows render badges when populated; otherwise show a muted dash so
124-
// the row layout stays consistent across pipelines.
118+
// Badges when populated; otherwise a muted dash to keep the row layout stable.
125119
const ReferenceList = ({ items }: { items: string[] }) => {
126120
if (items.length === 0) {
127121
return <Text className="text-muted-foreground">{EMPTY_PLACEHOLDER}</Text>;

frontend/src/components/pages/rp-connect/pipeline/index.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -938,8 +938,7 @@ export default function PipelinePage() {
938938
onRequestDelete={
939939
pipeline
940940
? () => {
941-
// Close the details dialog before opening the confirmation so
942-
// we never stack two dialogs on screen at the same time.
941+
// Close the details dialog first so the two don't stack.
943942
setIsViewConfigDialogOpen(false);
944943
setIsDeleteAlertOpen(true);
945944
}

frontend/src/components/pages/rp-connect/pipeline/pipeline-flow-diagram.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,8 @@ export const PipelineFlowDiagram = ({
258258
}
259259
}, [rfNodes, translateExtent]);
260260

261-
// Scroll-chaining: at the diagram's top/bottom pan boundary, swallow the
262-
// wheel event in capture so React Flow can't preventDefault and the page
263-
// scrolls through. Mid-range events fall through to React Flow's pan.
261+
// At the diagram's top/bottom pan boundary, swallow the wheel event in capture
262+
// so the page scrolls instead. Mid-range events fall through to React Flow's pan.
264263
useEffect(() => {
265264
const el = containerRef.current;
266265
if (!(el && contentOverflows && translateExtent && containerSize)) {

frontend/src/components/pages/rp-connect/template-gallery/pipeline-template-types.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,8 @@ type SlotBase = {
2020
section: TemplateSlotSection;
2121
required?: boolean;
2222
// Dotted path into the section's component schema (e.g. `dsn`, `tls.cert_file`).
23-
// When set, the form fills any unset `description` / `required` / `default`
24-
// from the live `FieldSpec` returned by `useListComponentsQuery()`. Slot-level
25-
// values always win — schema only fills blanks.
23+
// When set, the live schema fills any unset `description` / `required` /
24+
// `default` — slot-level values always win.
2625
schemaField?: string;
2726
};
2827

frontend/src/components/pages/rp-connect/template-gallery/slot-fields/secret-slot.tsx

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,8 @@ export const SecretSlotField = ({ slot, control, onSecretCreated, onRequestCreat
6868
required={slot.required}
6969
>
7070
{(field) => {
71-
// Surface a value the listSecrets query hasn't reported yet (e.g. just
72-
// created via the in-dialog step) so the Select can render the
73-
// selected option immediately instead of falling back to the placeholder.
71+
// Include a just-created value the listSecrets query hasn't reported
72+
// yet, so the Select shows it instead of the placeholder.
7473
const selectedValue = (field.value as string | undefined) ?? '';
7574
const options =
7675
selectedValue && !existingSecrets.includes(selectedValue)
@@ -81,10 +80,8 @@ export const SecretSlotField = ({ slot, control, onSecretCreated, onRequestCreat
8180
<div className="flex-1">
8281
<Select onValueChange={field.onChange} value={selectedValue}>
8382
<SelectTrigger data-testid={`slot-${slot.id}`}>
84-
{/* Render-prop child sidesteps Base UI's "label only resolves after the
85-
popup mounts" behavior — without it the trigger shows the placeholder
86-
until the dropdown is first opened, which masks the auto-select after
87-
in-dialog secret creation. */}
83+
{/* Render-prop child sidesteps Base UI resolving the label only after the
84+
popup first mounts, which would hide the auto-selected secret. */}
8885
<SelectValue placeholder="Select an existing secret...">
8986
{(value) => (typeof value === 'string' && value !== '' ? value : null)}
9087
</SelectValue>

frontend/src/components/pages/rp-connect/template-gallery/template-deploy.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,6 @@ const substituteToken = (slot: TemplateSlot, raw: string): string => {
2626

2727
const SLOT_TOKEN_PATTERN = /\$\{slot\.([A-Za-z0-9_-]+)\}/g;
2828

29-
// Lines whose only purpose is to set a value from a `${slot.X}` token get
30-
// dropped entirely when the slot is blank. This keeps optional fields out of
31-
// the emitted YAML — the connector then falls back to its own server-side
32-
// default (e.g. postgres_cdc auto-generates `slot_name` when the key is
33-
// absent, but rejects an empty string). The form's required-slot validation
34-
// already gates submission, so a blank slot value at this point means the
35-
// slot was deliberately optional.
3629
const referencedSlotIdsIn = (line: string): string[] => {
3730
const ids: string[] = [];
3831
for (const match of line.matchAll(SLOT_TOKEN_PATTERN)) {
@@ -53,6 +46,8 @@ export const stitchTemplateYaml = ({
5346
const kept: string[] = [];
5447
for (const line of template.baseYaml.split('\n')) {
5548
const refs = referencedSlotIdsIn(line);
49+
// Drop lines whose slot is blank so optional fields fall back to the
50+
// connector's own default rather than an empty value it may reject.
5651
if (refs.length > 0 && refs.some((id) => isBlank(values[id]))) {
5752
continue;
5853
}

frontend/src/components/pages/rp-connect/template-gallery/template-form-panel.tsx

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,14 @@ export type TemplateFormSubmitPayload = {
110110
template: PipelineTemplate;
111111
};
112112

113-
// Imperative handle so the parent dialog can read the current YAML on cancel
114-
// without re-rendering the form.
113+
// Lets the parent dialog read the current YAML on cancel without re-rendering.
115114
export type TemplateFormPanelHandle = {
116115
getCurrentYaml: () => string;
117116
isDirty: () => boolean;
118117
};
119118

120-
// Instruction from the parent to overwrite a slot's value (e.g. after an
121-
// in-dialog secret creation). Bumping `requestId` re-triggers the apply even
122-
// when slotId+value are unchanged.
119+
// Parent request to overwrite a slot value. Bumping `requestId` re-triggers the
120+
// apply even when slotId/value are unchanged.
123121
export type ApplySlotValueRequest = {
124122
slotId: string;
125123
value: string;
@@ -135,11 +133,10 @@ export type TemplateFormPanelProps = {
135133
// When set, secret slots delegate "Create secret" to the parent instead of
136134
// opening a nested dialog.
137135
onRequestCreateSecret?: (slotId: string, suggestedName: string | undefined) => void;
138-
// When set, topic slots delegate "Create topic" to the parent so a dedicated
139-
// step can host AddTopicStep, mirroring the secret flow.
136+
// When set, topic slots delegate "Create topic" to the parent.
140137
onRequestCreateTopic?: (slotId: string) => void;
141-
// When non-null, the panel will write `value` into the named slot exactly
142-
// once per `requestId` and call `onSlotValueApplied` to acknowledge.
138+
// Writes `value` into the named slot once per `requestId`, then acknowledges
139+
// via `onSlotValueApplied`.
143140
applySlotValue?: ApplySlotValueRequest | null;
144141
onSlotValueApplied?: () => void;
145142
};
@@ -165,8 +162,7 @@ export const TemplateFormPanel = forwardRef<TemplateFormPanelHandle, TemplateFor
165162
});
166163

167164
// Schema-driven defaults can arrive after mount. Reapply them once per
168-
// template, only while the form is still pristine — never clobber user
169-
// input.
165+
// template, but only while pristine so we never clobber user input.
170166
const lastAppliedTemplateId = useRef<string | null>(null);
171167
useEffect(() => {
172168
if (!componentListResponse || lastAppliedTemplateId.current === template.id) {
@@ -188,9 +184,7 @@ export const TemplateFormPanel = forwardRef<TemplateFormPanelHandle, TemplateFor
188184
isDirty: () => form.formState.isDirty,
189185
}));
190186

191-
// Apply an externally-requested slot write (e.g. from the in-dialog secret
192-
// creation step). Routed through props + effect rather than an imperative
193-
// ref so it doesn't depend on the parent's ref-forwarding plumbing.
187+
// Apply an externally-requested slot write (e.g. from the in-dialog secret step).
194188
// biome-ignore lint/correctness/useExhaustiveDependencies: requestId is the intentional change detector; slotId/value are read fresh from applySlotValue and shouldn't refire repeats.
195189
useEffect(() => {
196190
if (!applySlotValue) {

frontend/src/components/pages/rp-connect/template-gallery/template-gallery-dialog.tsx

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,11 @@ import type { AddTopicFormData, BaseStepRef } from '../types/wizard';
4141

4242
export type TemplateGalleryDialogProps = {
4343
open: boolean;
44-
/**
45-
* Called when the dialog is closed via the X button, ESC, or backdrop click.
46-
* If the user got far enough into the guided form to enter a value, the
47-
* dialog will hand back the current stitched YAML via `stashedYaml`. Use this
48-
* to drop the partial config into the host editor so the user can continue
49-
* editing manually.
50-
*/
44+
// Closed via X, ESC, or backdrop. If the user entered any form values,
45+
// `stashedYaml` carries the partial config so the host editor can keep it.
5146
onClose: (stashedYaml: string | null) => void;
52-
/** Called when the user submits the form. Parent owns the deploy mutation. */
47+
// Parent owns the deploy mutation.
5348
onSubmit: (payload: TemplateFormSubmitPayload) => void;
54-
/** Render the form's "Create pipeline" button in a busy state. */
5549
isSubmitting?: boolean;
5650
};
5751

@@ -102,10 +96,9 @@ const StepBackHeader = ({
10296
export const TemplateGalleryDialog = ({ open, onClose, onSubmit, isSubmitting }: TemplateGalleryDialogProps) => {
10397
const [view, setView] = useState<DialogView>({ kind: 'gallery' });
10498
const [selectedTemplate, setSelectedTemplate] = useState<PipelineTemplate | null>(null);
105-
// Slot write requested by the in-dialog secret-creation step. The form panel
106-
// consumes it via a useEffect and then calls back to clear it. Routed
107-
// through props instead of an imperative ref because plain function
108-
// components don't receive `ref` as a prop in React 18.
99+
// Slot write requested by an in-dialog step; the form panel consumes it via
100+
// an effect and clears it. Props rather than a ref since React 18 function
101+
// components don't receive `ref`.
109102
const [applySlotValue, setApplySlotValue] = useState<ApplySlotValueRequest | null>(null);
110103
const [isCreatingTopic, setIsCreatingTopic] = useState(false);
111104
const formHandleRef = useRef<TemplateFormPanelHandle | null>(null);
@@ -133,10 +126,8 @@ export const TemplateGalleryDialog = ({ open, onClose, onSubmit, isSubmitting }:
133126
resetToGallery();
134127
};
135128

136-
// Ensure the next open lands on the gallery even when the parent closes the
137-
// dialog directly (e.g. after `onSubmit`) without routing through `onClose` /
138-
// `onOpenChange`. Without this, the previous view/state would still be set
139-
// when the dialog reopens.
129+
// Reset to the gallery when the parent closes the dialog directly (e.g. after
130+
// `onSubmit`) without routing through `onClose`, so it reopens clean.
140131
useEffect(() => {
141132
if (!open) {
142133
setView({ kind: 'gallery' });
@@ -175,9 +166,8 @@ export const TemplateGalleryDialog = ({ open, onClose, onSubmit, isSubmitting }:
175166
return;
176167
}
177168
if (!result.success && result.error) {
178-
// AddTopicStep surfaces validation errors inside its own form; only
179-
// toast the higher-level "create failed" error so the user still sees
180-
// field-level errors in context.
169+
// AddTopicStep shows field-level validation in its own form; only toast
170+
// the higher-level failure.
181171
toast.error(result.message ?? result.error);
182172
}
183173
} finally {
@@ -187,8 +177,8 @@ export const TemplateGalleryDialog = ({ open, onClose, onSubmit, isSubmitting }:
187177

188178
return (
189179
<Dialog
190-
// Block backdrop dismissal past the gallery step so a stray click can't
191-
// wipe in-progress form values. ESC and the X / Cancel buttons still close.
180+
// Block backdrop dismissal past the gallery so a stray click can't wipe
181+
// in-progress form values. ESC and X / Cancel still close.
192182
disablePointerDismissal={view.kind !== 'gallery'}
193183
onOpenChange={(nextOpen) => (nextOpen ? undefined : closeWithStash())}
194184
open={open}
@@ -243,8 +233,8 @@ export const TemplateGalleryDialog = ({ open, onClose, onSubmit, isSubmitting }:
243233
/>
244234
) : null}
245235

246-
{/* Stays mounted across addSecret to preserve form state; hide via inline */}
247-
{/* style so the flex-1 outer doesn't reserve space (className targets inner). */}
236+
{/* Stays mounted across addSecret/createTopic to preserve form state; */}
237+
{/* hidden via inline style (className targets the inner element). */}
248238
{isFormMounted && selectedTemplate ? (
249239
<DialogBody style={isFormViewActive ? undefined : { display: 'none' }}>
250240
<TemplateFormPanel
@@ -289,12 +279,8 @@ export const TemplateGalleryDialog = ({ open, onClose, onSubmit, isSubmitting }:
289279

290280
{isCreateTopicViewActive ? (
291281
<DialogBody>
292-
{/* LayoutGroup so the popLayout AnimatePresence inside AddTopicStep
293-
forces a re-measure on the sibling button motion.div below.
294-
Without it, the button's `layout` prop only sees positions at
295-
React render time — popLayout shifts surrounding flow via
296-
motion's effects after commit, so the button would otherwise
297-
jump on collapse. */}
282+
{/* LayoutGroup keeps the button below in sync with the popLayout
283+
AnimatePresence inside AddTopicStep — otherwise it jumps on collapse. */}
298284
<LayoutGroup>
299285
<div className="flex flex-col gap-4">
300286
<AddTopicStep hideTitle inline ref={addTopicStepRef} selectionMode="new" />

0 commit comments

Comments
 (0)