New Pipeline UX Bug Bash fixes part 1#2373
Conversation
| <div className="flex items-center gap-2"> | ||
| <Switch | ||
| checked={liveViewEnabled} | ||
| className="h-5 w-9 **:data-[slot=switch-thumb]:size-4.5" |
There was a problem hiding this comment.
Is **:data-[slot=switch-thumb]:size-4.5 required?
There was a problem hiding this comment.
Yes, left a comment inline. User gave feedback that it wasn't discoverable so made it slightly larger, which makes it similar size to refresh button it is next to
There was a problem hiding this comment.
Pull request overview
This PR bundles a set of “bug bash” fixes and UX improvements across the new Pipeline experience, with a heavy focus on Log Explorer live/history behaviors, dialog usability/accessibility, and connector/tile/YAML ergonomics.
Changes:
- Fixes user-facing feedback and correctness issues (topic creation errors, search race/stale results, live log streaming batching).
- Improves Log Explorer UX (live mode states, progress feedback, truncation/table layout, sort icons, live toggle discoverability).
- Refactors and enhances pipeline dialogs/menus (abortable/ref-based dialog lifecycle hook, larger accessible close buttons, command menu category shortcuts & contextual connector dialog titles/placeholders), plus YAML patching utilities.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/react-query/api/topic.tsx | Ensures topic-create failures show a toast error. |
| frontend/src/react-query/api/logs.tsx | Adds progress reporting + batches live log updates to reduce re-renders. |
| frontend/src/react-query/api/logs.test.tsx | Adds tests for live/history progress reporting. |
| frontend/src/hooks/use-ref-form-dialog.ts | Introduces a ref-driven dialog lifecycle helper with timeout handling. |
| frontend/src/hooks/use-ref-form-dialog.test.tsx | Adds unit tests for useRefFormDialog behaviors (success, failure, abort, timeout). |
| frontend/src/components/ui/secret/quick-add-secrets.tsx | Adds cardVariant prop to support outlined dialog styling. |
| frontend/src/components/ui/dialog-close-button.tsx | New close button component with larger accessible hit target + size variants. |
| frontend/src/components/ui/connect/log-explorer.tsx | Adds progress UI, live-mode UX states, live toggle label/tooltip, table layout fixes, icon updates. |
| frontend/src/components/ui/connect/log-explorer.test.tsx | Updates/extends LogExplorer tests for progress UI and live-mode states. |
| frontend/src/components/pages/topics/Tab.Messages/index.tsx | Prevents stale search results via generation guard; adds inline progress and longer debounce. |
| frontend/src/components/pages/rp-connect/utils/yaml.ts | Adds commented-key stripping after patching; adds extractConnectorTopics() helper. |
| frontend/src/components/pages/rp-connect/utils/yaml.test.tsx | Adds tests for extractConnectorTopics and comment stripping behavior. |
| frontend/src/components/pages/rp-connect/utils/user.ts | Renames and uses grantTopicPermissions to decide whether to configure ACLs. |
| frontend/src/components/pages/rp-connect/types/wizard.ts | Renames superuser to grantTopicPermissions in schema. |
| frontend/src/components/pages/rp-connect/pipeline/pipeline-command-menu.tsx | Adds initialFilter support; switches to custom dialog close button; UI tweaks and secret display tweak. |
| frontend/src/components/pages/rp-connect/pipeline/pipeline-command-menu.test.tsx | Updates expectations for secret display text. |
| frontend/src/components/pages/rp-connect/pipeline/index.tsx | Adds sidebar category buttons to open command menu pre-filtered; adopts useRefFormDialog; contextual connector dialog titles/placeholders. |
| frontend/src/components/pages/rp-connect/pipeline/index.test.tsx | Updates tests to open command menu via sidebar buttons and assert initial filter is passed. |
| frontend/src/components/pages/rp-connect/pipeline/details-dialog.tsx | Uses the new DialogCloseButton for better accessibility. |
| frontend/src/components/pages/rp-connect/pipeline/config-dialog.tsx | Uses the new DialogCloseButton for better accessibility. |
| frontend/src/components/pages/rp-connect/onboarding/connect-tiles.tsx | Adds configurable search placeholder. |
| frontend/src/components/pages/rp-connect/onboarding/connect-tile.tsx | Fixes logo sizing/overflow; improves long-name wrapping and layout. |
| frontend/src/components/pages/rp-connect/onboarding/add-user-step.tsx | Renames field, suppresses “user exists” alert while mutation pending. |
| frontend/src/components/pages/rp-connect/onboarding/add-secrets-dialog.tsx | Uses DialogCloseButton; passes outlined card variant into QuickAddSecrets. |
| frontend/src/components/pages/rp-connect/onboarding/add-connector-dialog.tsx | Adds contextual title/search placeholder props + DialogCloseButton. |
| frontend/src/assets/connectors/logos/gnu-logo.tsx | Removes hardcoded SVG dimensions to prevent overflow. |
| frontend/src/assets/connectors/logos/aws-dynamo-db.tsx | Removes hardcoded SVG dimensions to prevent overflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="relative min-h-0"> | ||
| {/* Progress bar overlaying top of table */} | ||
| {isSearching && hasProgress && ( | ||
| <div className="absolute inset-x-0 top-0 z-10"> | ||
| <Progress className="h-1 w-full rounded-none" testId="log-progress-bar" value={null} /> | ||
| </div> |
There was a problem hiding this comment.
Progress is being rendered with value={null}. In the current Progress component implementation, value is used to compute the indicator translate (value || 0), so null results in the indicator being translated to -100% (effectively hidden) rather than showing an indeterminate/loading animation. Either pass a real numeric value (if you can derive one) or extend Progress with an explicit indeterminate mode/animation and use that here.
There was a problem hiding this comment.
this is a ui-registry component, we should explore a fix in the registry itself
eblairmckee
force-pushed
the
fix/log-explorer-live-mode
branch
from
eb9f573 to
53a5984
Compare
|
@claude review |
|
Claude finished @eblairmckee's task in 3m 5s —— View job PR Review: New Pipeline UX Bug Bash fixes part 1
Overall this is a solid set of targeted fixes with good test coverage. A few issues worth addressing before merge: Medium Issues1.
The author noted this needs a registry fix — that should either be tracked as a follow-up issue or the 2. Abort signal not propagated to implementations (
The copilot comment said this was addressed, but the implementation in 3. const pipelineNotRunning = pipeline.state === Pipeline_State.STOPPED
|| pipeline.state === Pipeline_State.ERROR
|| pipeline.state === Pipeline_State.COMPLETED;If a new pipeline state is added (e.g., 4. Two <PipelineCommandMenu ... open={commandMenuFilter !== null} />
{isSlashMenuEnabled ? (
<PipelineCommandMenu ... open={slashCommand.isOpen} variant="popover" />
) : null}Both instances are mounted whenever Minor Issues5. const open = useCallback((t?: TTarget) => {
setTarget((t ?? true) as TTarget);
}, []);The 6. Double icon in submit buttons ( <Button icon={topicDialog.isSubmitting ? <Spinner /> : undefined}>Add</Button>The 7. if (line.length > 0 && line.startsWith(' ') && !line.startsWith(' ') && !line.startsWith(' #')) {
componentEnd = i;This assumes 2-space YAML indentation, which matches 8. <Text as="span" className="block truncate" variant="bodyMedium">
{cullText(text, 200)}
</Text>Both Looks Good
|
|
@claude I removed the changes to tab.messages, what do you mean by: Generation counter for stale search results in Tab.Messages: Clean fix for the race condition. |
|
Claude finished @eblairmckee's task in 1m 4s —— View job
Clarification: Generation counter in Tab.MessagesYou're right to call this out — those changes were reverted in commit To be specific about what was originally there (from commit
Since you reverted all of that as out of scope, ignore that "Looks Good" item entirely — it doesn't apply to the current PR state. |
|
addressed claude comments in f698674
won't do. see earlier comment
fixed
pipelineNotRunning — Changed to !== Pipeline_State.RUNNING for forward-compatibility with new states
Verified both are inert when closed (effects bail out, query has enabled: open). No change needed -- architecturally necessary for different rendering modes (modal dialog vs positioned popover). |
| type LiveAction = | ||
| | { type: 'reset' } | ||
| | { type: 'start' } | ||
| | { type: 'addMessage'; msg: TopicMessage } |
There was a problem hiding this comment.
Not sure if it's intentional that we no longer have addMessage type handling
There was a problem hiding this comment.
Where is this used? It's quite complex, might be a good follow-up for simplification.
There was a problem hiding this comment.
the "add topic" dialog and "add user" dialog (both in the new pipeline UX)
| {header.column.getIsSorted() === 'asc' && ' \u2191'} | ||
| {header.column.getIsSorted() === 'desc' && ' \u2193'} | ||
| {header.column.getIsSorted() === 'asc' && <ArrowUp className="ml-1 inline size-3.5" />} | ||
| {header.column.getIsSorted() === 'desc' && <ArrowDown className="ml-1 inline size-3.5" />} |
| expect(screen.getByText('${secrets.MY_API_KEY}')).toBeInTheDocument(); | ||
| expect(screen.getByText('${secrets.DB_PASSWORD}')).toBeInTheDocument(); | ||
| expect(screen.getByText('secrets.MY_API_KEY')).toBeInTheDocument(); | ||
| expect(screen.getByText('secrets.DB_PASSWORD')).toBeInTheDocument(); |
Bug bash fixes: Log Explorer live mode, improved pipeline dialog UX, connector tiles are context-aware, pipeline command menu design improvements
See bug bash doc
Usability Improvements
Log Explorer
Log Explorer: no progress feedback during search
Users saw a bare spinner with no indication of progress when loading logs. Added a progress bar overlaying the table top edge with scanned bytes and message count stats. Spinner shows briefly before the first progress event arrives, then switches to stats.
Log Explorer: confusing states in live mode
Users enabling live mode on a stopped pipeline saw an empty table with no explanation. Added contextual states: "Pipeline is not running — Switch to Recent Logs" when the pipeline is stopped, "Listening for new log messages..." when live mode is active but no messages have arrived, and specific empty-state text for history mode ("No logs found in the last 5 hours"). Live mode now auto-enables when the pipeline starts running and auto-disables when it stops.
Log Explorer: live toggle was hard to discover
Users didn't notice the live toggle. Added a descriptive label ("Enable live logs" / "Live logs enabled") with an info-icon tooltip explaining the behavior, and increased the switch size for visibility.
Pipeline Creation
Pipeline user dialog: smarter topic and ACL awareness
New
extractConnectorTopics()reads topics configured on a connector from YAML and passes them toAddUserStep, so ACLs are configured for the right topic automatically. Multi-topic connectors show a warning alert prompting manual ACL setup. Renamed misleadingsuperuserform field tograntTopicPermissions.Pipeline dialogs couldn't be dismissed during loading
Users were trapped waiting for slow form submissions with no way to cancel. New
useRefFormDialoghook manages the full dialog lifecycle — closing a dialog during submission cancels the pending operation viaAbortController(signal passed totriggerSubmitso implementors can wire it to RPC calls) with a configurable timeout (default 30s). Submit buttons show text + spinner icon instead of spinner replacing text.Pipeline sidebar: category shortcuts for command menu
Made types of variables you can insert via UI more apparent (secrets, contextual variables, users, topics, etc). Users had to open the command menu and then filter to find variables, secrets, topics, or users. Replaced the single "Insert" button with buttons for each category, each opening the command menu pre-filtered via a new
initialFilterprop.Functional Improvements
Log Explorer
Log Explorer: live streaming performance
useLogLivedispatched a React state update per incoming message, causing O(n^2) re-renders as message count grew. Replaced with batchedpendingRef+ 200ms flush interval matching history mode's pattern.Pipeline Creation
Topic creation errors silently swallowed
useCreateTopicMutationonErrorformatted the error message but never passed it totoast.error()— users saw no feedback when topic creation failed. Now correctly shows the error toast.UI Improvements
Dialog close buttons: accessible hit target
Pipeline dialog close buttons had a 14px icon — too small for reliable click/tap. New
DialogCloseButtoncomponent with CVA size variants (small|medium) and minimum 24px icon, applied across all pipeline dialogs (add topic, add user, config, details, add connector, add secrets, command menu sub-dialogs).Connector tile logos overflow fixed
Certain SVG logos (DynamoDB, GNU) had hardcoded
width/heightattributes that overrode CSS sizing, causing overflow. Removed hardcoded attributes, replaced inline styles withclassName="h-6 w-6 object-contain"in a fixed-size flex container. Long connector names now word-wrap withbreak-wordsinstead of being clipped.Log explorer table respects parent container bounds
The log table's message column could push the table wider than its parent container, creating a horizontal scrollbar. Added
table-fixedlayout andtruncateon the message column so long messages ellipsis instead of overflowing.Log explorer sort indicators
Column sort arrows replaced unicode glyphs with lucide
ArrowUp/ArrowDownicons for visual consistency.Connector dialog contextual titles
AddConnectorDialognow acceptstitleandsearchPlaceholderprops. Pipeline page passes contextual values ("Add an input" / "Search inputs...", "Add a resource" / "Search resources...") instead of generic "Add a connector".Other
stripCommentedKeys: After patching a YAML key (e.g.,topics), removes the commented-out placeholder line (# topics: Required - ...) scoped to the specific component block (e.g., only withinkafka_franz:, not sibling components in the same section).QuickAddSecretscard variant prop: Secrets dialog now passescardVariant="outlined"for visual consistency within dialogs.