refactor(pipeline-server): redesign pipeline detail UI#562
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (96)
📝 WalkthroughWalkthroughThis PR restructures the pipeline-server frontend's navigation and inspection system, moving from query-parameter-based routing to nested file-based routes, refactoring sidebar UI to conditionally show pipeline navigation or source files, extracting graph utilities to shared modules, and replacing a centralized inspect data hook with prop-driven components. Server-side overview endpoint moved from global to per-source scope. Multiple components removed or redesigned; new route detail pages and layout components added. Extensive test updates. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
🌏 Preview DeploymentsNote No deployable apps affected by changes in this PR. Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
luxass
changed the title
…o source Remove the global overview API endpoint, query, and dashboard components that are superseded by per-source overview. The home route now redirects to the last active source (or the first available source). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite the source page with a Vercel-inspired dashboard layout: - Left column: activity chart, status overview, compact recent executions - Right column: search toolbar with list/grid view toggle, pipeline files - Error banner below header for source issues - New PipelineFileRow (expandable list) and PipelineFileCard (grid) components - Recent executions as a compact stacked list instead of a wide table Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eline layouts Streamline the sidebar by removing inline source queries, simplify the source file list and switcher components, and clean up pipeline header and summary layouts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ile card and source issues dialog
The home/ folder was a leftover from the removed dashboard. These components (activity-chart, status-overview-panel, shared) are now used on the source overview page, so the folder name should reflect that. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When navigating into a pipeline, the sidebar now transforms from the file tree into pipeline-specific navigation: back link, pipeline identity, Overview/Inspect/Executions nav links, pipeline info stats, and an Execute button. This replaces the tab bar that was previously in the main content area. - Remove PipelineTabs component (navigation moved to sidebar) - Simplify PipelineHeader (remove version selector and execute button) - Add context-aware rendering to PipelineSidebar Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Introduced new routes for inspecting pipeline outputs, transforms, and routes. - Implemented detailed pages for each output and transform, including navigation and filtering capabilities. - Added a sidebar for easy navigation between different inspection tabs. - Enhanced user experience with informative messages when no outputs or transforms are found.
- Implemented redirection for routes and outputs when no outputs or transforms are defined in the pipeline. - Improved UI elements for better accessibility and user experience. - Added detailed sections for dependencies, transforms, and outputs in the route details.
- Introduced new routes for inspecting outputs and transforms within the pipeline. - Created `OutputDetailPage` to display details of specific outputs. - Added `RouteDetailPage` for inspecting individual routes, including dependencies and transforms. - Updated route tree to include new routes for outputs and transforms. - Enhanced the existing transforms index to redirect to the first transform's detail page.
…ate files Move routes, transforms, and outputs list components from the monolithic inspect-sidebar into individual files under inspect/list/. Each list component now derives its own data internally instead of receiving pre-computed data from the parent. Remove unnecessary useMemo wrappers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Updated output key generation to use index directly for better clarity. - Enhanced layout of the `OutputsSection` and `TransformDetailPage` for improved readability and consistency.
…e utils Introduce a single FlowNodeData discriminated union type used by both the execution graph and definition graph. Move all graph utility code (layout, conversion, filtering) into lib/graph-utils.ts and lib/graph-layout.ts, eliminating duplicated layout algorithms. Merge route detail into a single card with output nodes in the neighbor graph that navigate to output detail pages. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ail pages Merge transform and output detail pages into single-card layouts with divide-y sections, matching the route detail pattern. Consistent bg-muted/5 header styling across all three detail views. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrite renderFileRoute to accept a React element + options with
initialLocation, matching the renderWithFileRoutes pattern. All route
tests now use renderFileRoute(<div />, { initialLocation: "..." }).
Rewrite inspect route tests for the new sub-route architecture
(routes/$routeId, transforms/$transformName, outputs/$outputKey).
Add graph-layout and graph-definition-utils unit tests. Fix pre-existing
type errors in execution-table, use-pipeline-versions, and execution
detail page.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…r tests Split TreeNode into FolderNode/FileNode to fix conditional useCallback hook call. Update use-pipeline-versions tests to match new hook API. Fix browser test assertions to match current UI (multiple elements, changed text). Mark complex tests as todo for later. Refactor renderFileRoute to accept React element + options. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the pipeline-server UI around a redesigned pipeline detail/inspect experience, shifting overview content into more structure-oriented components, updating routing, and adjusting both client/server “overview” APIs and tests accordingly.
Changes:
- Replace the workspace-level overview page/API with source-scoped overview (
/api/sources/:sourceId/overview) and redirect/into a source (using a “last active source” hint). - Redesign pipeline detail pages: new summary/structure/latest-execution sections, new inspect layout + sidebar, and new definition graph utilities/layout.
- Update Vitest browser project config to enable TanStack Router route-tree generation and adjust/trim browser & server test suites.
Reviewed changes
Copilot reviewed 96 out of 96 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared-ui/src/ui/sidebar.tsx | Removes an inline comment in sidebar gap markup. |
| packages/pipelines/pipeline-server/vitest.config.ts | Adds Vite plugins for router + React in browser tests and path refactor. |
| packages/pipelines/pipeline-server/test/server/overview.test.ts | Removes old /api/overview server test. |
| packages/pipelines/pipeline-server/test/server/graph-layout.test.ts | Adds tests for new layered layout helper. |
| packages/pipelines/pipeline-server/test/server/graph-definition-utils.test.ts | Adds tests for new definition graph utilities. |
| packages/pipelines/pipeline-server/test/browser/routes/s.$sourceId.index.test.tsx | Updates source route test + stubs new source overview API. |
| packages/pipelines/pipeline-server/test/browser/routes/s.$sourceId.$sourceFileId.index.test.tsx | Removes source-file index route test (route now redirects). |
| packages/pipelines/pipeline-server/test/browser/routes/s.$sourceId.$sourceFileId.$pipelineId.inspect.transforms.test.tsx | Updates transforms inspect tests for new nested routes. |
| packages/pipelines/pipeline-server/test/browser/routes/s.$sourceId.$sourceFileId.$pipelineId.index.test.tsx | Updates pipeline overview route test assertions and execution pagination stubs. |
| packages/pipelines/pipeline-server/test/browser/routes/s.$sourceId.$sourceFileId.$pipelineId.executions.index.test.tsx | Updates executions list route test for new header/links. |
| packages/pipelines/pipeline-server/test/browser/routes/s.$sourceId.$sourceFileId.$pipelineId.executions.$executionId.index.test.tsx | Marks detailed execution tests as it.todo and updates render helper usage. |
| packages/pipelines/pipeline-server/test/browser/routes/s.$sourceId.$sourceFileId.$pipelineId.executions.$executionId.graph.test.tsx | Updates execution graph route tests and stubs executions API. |
| packages/pipelines/pipeline-server/test/browser/routes/root-error.test.tsx | Updates render helper signature usage. |
| packages/pipelines/pipeline-server/test/browser/routes/pipeline-command-palette.test.tsx | Updates render helper signature usage. |
| packages/pipelines/pipeline-server/test/browser/routes/index.test.tsx | Removes old home route test (home now redirects). |
| packages/pipelines/pipeline-server/test/browser/router-test-utils.tsx | Removes bespoke router test helper. |
| packages/pipelines/pipeline-server/test/browser/route-test-utils.tsx | Refactors file-route rendering helper signature and wrapper usage. |
| packages/pipelines/pipeline-server/test/browser/hooks/use-pipeline-versions.test.tsx | Updates tests for new select/deselect semantics. |
| packages/pipelines/pipeline-server/test/browser/components/source/source-file-card.test.tsx | Removes tests for removed SourceFileCard. |
| packages/pipelines/pipeline-server/test/browser/components/pipeline/version-selector.test.tsx | Updates tests for new dropdown-based version selector + storage key. |
| packages/pipelines/pipeline-server/test/browser/components/pipeline/quick-actions-card.test.tsx | Removes tests for removed QuickActionsCard. |
| packages/pipelines/pipeline-server/test/browser/components/pipeline/pipeline-header.test.tsx | Switches header tests to route-level rendering; marks as todo. |
| packages/pipelines/pipeline-server/test/browser/components/home/status-overview-panel.test.tsx | Updates import path after moving overview components. |
| packages/pipelines/pipeline-server/test/browser/components/home/sources-panel.test.tsx | Removes tests for removed SourcesPanel. |
| packages/pipelines/pipeline-server/test/browser/components/home/activity-chart.test.tsx | Updates import path after moving overview components. |
| packages/pipelines/pipeline-server/test/browser/components/execution/execution-table.test.tsx | Loosens some assertions and updates “View graph” selection. |
| packages/pipelines/pipeline-server/test/browser/components/app/source-switcher.test.tsx | Updates assertions to match new switcher behavior/labels. |
| packages/pipelines/pipeline-server/test/browser/components/app/pipeline-sidebar.test.tsx | Marks sidebar tests as todo. |
| packages/pipelines/pipeline-server/test/browser/components/app/pipeline-sidebar-source-file-list.test.tsx | Marks file list test as todo. |
| packages/pipelines/pipeline-server/src/server/routes/sources.overview.ts | Introduces source-scoped overview route + sourceId validation. |
| packages/pipelines/pipeline-server/src/server/routes/index.ts | Exports sourcesOverviewRouter. |
| packages/pipelines/pipeline-server/src/server/app.ts | Mounts source-scoped overview under /api/sources. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/route.tsx | Stores “last active source” in loader. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/index.tsx | Redirects source-file index back to source route. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/route.tsx | Prefetches executions and simplifies header wrapper. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/inspect/transforms/index.tsx | Adds transforms index that redirects to first transform. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/inspect/routes/index.tsx | Adds routes index (graph + redirect to first route). |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/inspect/route.tsx | Adds inspect layout with sticky sidebar. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/inspect/outputs/index.tsx | Adds outputs index that redirects to first output. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/inspect/outputs/$outputKey.tsx | Adds output detail page with navigation and metadata. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/inspect/index.tsx | Redirects /inspect to /inspect/routes. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/index.tsx | Redesigns pipeline overview UI (latest execution, summary, structure, executions card). |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/graphs.tsx | Removes graphs listing page. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/executions/index.tsx | Redesigns executions list page header + layout. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/executions/$executionId/index.tsx | Redesigns execution detail header and summary line. |
| packages/pipelines/pipeline-server/src/client/routes/s.$sourceId/$sourceFileId/$pipelineId/executions/$executionId/graph.tsx | Redesigns execution graph page with new header and empty state. |
| packages/pipelines/pipeline-server/src/client/routes/index.tsx | Replaces home overview with redirect-to-source + empty state. |
| packages/pipelines/pipeline-server/src/client/queries/source-overview.ts | Adds source-scoped overview query. |
| packages/pipelines/pipeline-server/src/client/queries/overview.ts | Removes workspace-level overview query. |
| packages/pipelines/pipeline-server/src/client/lib/last-active-source.ts | Adds localStorage helper for last active source. |
| packages/pipelines/pipeline-server/src/client/lib/graph-utils.ts | Adds shared flow conversion, filtering, and layout entrypoints. |
| packages/pipelines/pipeline-server/src/client/lib/graph-layout.ts | Adds generic layered layout implementation. |
| packages/pipelines/pipeline-server/src/client/hooks/use-pipeline-versions.ts | Reworks selection persistence with useSyncExternalStore. |
| packages/pipelines/pipeline-server/src/client/hooks/use-inspect-data.ts | Removes inspect search-param based navigation hook. |
| packages/pipelines/pipeline-server/src/client/components/source/source-issues-dialog.tsx | Tightens issue typing and updates dialog sizing classes. |
| packages/pipelines/pipeline-server/src/client/components/source/source-file-card.tsx | Removes old source file card component. |
| packages/pipelines/pipeline-server/src/client/components/source/pipeline-file-row.tsx | Adds expandable file row UI for source pages. |
| packages/pipelines/pipeline-server/src/client/components/source/pipeline-file-card.tsx | Adds file card UI for source pages. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/version-selector.tsx | Replaces button list with dropdown + internal storage-backed hook. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/route-structure-metrics.tsx | Adds reusable metrics strip component. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/quick-actions-card.tsx | Removes old quick actions card. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/pipeline-tabs.tsx | Removes old pipeline tab strip. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/pipeline-summary.tsx | Adds pipeline summary card. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/pipeline-structure.tsx | Adds structure card with deep links into inspect. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/pipeline-header.tsx | Simplifies header to breadcrumb + description + file path. |
| packages/pipelines/pipeline-server/src/client/components/pipeline/latest-execution.tsx | Adds latest execution card with links to logs/graph. |
| packages/pipelines/pipeline-server/src/client/components/overview/status-overview-panel.tsx | Moves types to shared schemas + adds compact mode. |
| packages/pipelines/pipeline-server/src/client/components/overview/shared.ts | Moves overview types to shared schemas. |
| packages/pipelines/pipeline-server/src/client/components/overview/activity-chart.tsx | Moves overview types to shared schemas + adds compact mode. |
| packages/pipelines/pipeline-server/src/client/components/inspect/route-transforms-section.tsx | Converts transform navigation to route-based links. |
| packages/pipelines/pipeline-server/src/client/components/inspect/route-outputs-section.tsx | Converts outputs section to route-based links + card UI. |
| packages/pipelines/pipeline-server/src/client/components/inspect/route-flow-section.tsx | Removes old upstream/downstream flow section. |
| packages/pipelines/pipeline-server/src/client/components/inspect/route-dependencies-section.tsx | Converts dependency navigation to route-based links + card UI. |
| packages/pipelines/pipeline-server/src/client/components/inspect/list/sidebar-transforms-list.tsx | Adds sidebar transforms list with filtering. |
| packages/pipelines/pipeline-server/src/client/components/inspect/list/sidebar-shared.tsx | Adds shared derivation helpers for sidebar lists. |
| packages/pipelines/pipeline-server/src/client/components/inspect/list/sidebar-routes-list.tsx | Adds sidebar routes list with rich filtering. |
| packages/pipelines/pipeline-server/src/client/components/inspect/list/sidebar-outputs-list.tsx | Adds sidebar outputs list with filtering. |
| packages/pipelines/pipeline-server/src/client/components/inspect/inspect-sidebar.tsx | Adds inspect sidebar with tabs and search. |
| packages/pipelines/pipeline-server/src/client/components/inspect/definition-node.tsx | Adds custom ReactFlow node renderers for definition graph. |
| packages/pipelines/pipeline-server/src/client/components/inspect/definition-graph.tsx | Adds definition graph component (layout + selection + minimap). |
| packages/pipelines/pipeline-server/src/client/components/home/sources-panel.tsx | Removes old sources panel component. |
| packages/pipelines/pipeline-server/src/client/components/graph/pipeline-graph.tsx | Switches execution graph utilities to shared #lib/graph-utils. |
| packages/pipelines/pipeline-server/src/client/components/graph/nodes.tsx | Updates node types + dimensions to shared graph utils. |
| packages/pipelines/pipeline-server/src/client/components/graph/graph-utils.ts | Removes old graph utility implementation. |
| packages/pipelines/pipeline-server/src/client/components/execution/status-badge.tsx | Updates badge styling and adds centralized style map. |
| packages/pipelines/pipeline-server/src/client/components/app/source-switcher.tsx | Replaces DropdownMenu with custom searchable popover. |
| packages/pipelines/pipeline-server/src/client/components/app/source-file-list.tsx | Refactors tree node rendering and file row toggling behavior. |
Comments suppressed due to low confidence (1)
packages/pipelines/pipeline-server/src/server/routes/sources.overview.ts:30
- The overview API was moved to
/api/sources/:sourceId/overviewand now validatessourceId, but there’s no corresponding server test coverage (the previous/api/overviewtest was removed). Adding asources.overviewroute test would help prevent regressions in windowing/zero-fill logic and the 400/404 behaviors.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| style={{ width: OUTPUT_NODE_WIDTH, minHeight: OUTPUT_NODE_HEIGHT }} | ||
| className="cursor-pointer rounded-lg border border-dashed border-border bg-card px-3 py-1.5 shadow-sm transition-shadow hover:shadow-md" | ||
| > | ||
| <Handle type="target" position={Position.Left} className="h-2! w-2! border-none! bg-muted-foreground!" /> |
There was a problem hiding this comment.
Tailwind’s important modifier is written as !h-2 / !w-2 (prefix), not h-2!. This handle uses the unsupported suffix form, so the intended sizing/styling may not be applied.
Comment on lines
+7
to
10
| const pipelineServerRoot = fileURLToPath(new URL("./", import.meta.url)); | ||
|
|
||
| const browserSetupFile = `${pipelineServerRoot}/test/browser/setup.ts`; | ||
|
|
There was a problem hiding this comment.
pipelineServerRoot is a platform-specific filesystem path; building child paths via template strings with /... can produce mixed separators on Windows. Prefer new URL("test/browser/setup.ts", import.meta.url) + fileURLToPath, or path.join(pipelineServerRoot, "test", ...), for cross-platform correctness.
Comment on lines
+64
to
+76
| for (let i = 0; i < queue.length; i += 1) { | ||
| const nodeId = queue[i]; | ||
| if (!nodeId) continue; | ||
|
|
||
| const layer = layers.get(nodeId) ?? 0; | ||
|
|
||
| for (const childId of graphIndex.outgoing.get(nodeId) ?? []) { | ||
| const nextLayer = layer + 1; | ||
| if (nextLayer > (layers.get(childId) ?? -1)) { | ||
| layers.set(childId, nextLayer); | ||
| } | ||
| queue.push(childId); | ||
| } |
There was a problem hiding this comment.
assignLayers pushes every child onto queue unconditionally. If the graph contains a cycle, layers will keep increasing and the queue will grow without bound (effectively an infinite loop / OOM). Even for DAGs, this can create a lot of duplicate work. Consider only enqueueing when a node’s layer actually increases and/or add explicit cycle detection / max-iterations guard so malformed graphs can’t hang the UI.
| </div> | ||
| </div> | ||
|
|
||
| <Handle type="source" position={Position.Right} className="h-2! w-2! border-none! bg-muted-foreground!" /> |
There was a problem hiding this comment.
Tailwind’s important modifier is written as !h-2 / !w-2 (prefix), not h-2!. This handle uses the unsupported suffix form, so the intended sizing/styling may not be applied.
Comment on lines
90
to
92
| <DialogContent | ||
| className="grid-rows-[auto_auto_minmax(0,1fr)_auto] h-[90vh] !w-[72rem] !max-w-[calc(100vw-2rem)] overflow-hidden p-0" | ||
| className="grid-rows-[auto_auto_minmax(0,1fr)_auto] h-[90vh] w-6xl! max-w-[calc(100vw-2rem)]! overflow-hidden p-0" | ||
| data-testid="source-issues-dialog" |
There was a problem hiding this comment.
The class string uses w-6xl! / max-w-[...]!, but Tailwind’s important modifier is prefix-based (e.g. !w-6xl, !max-w-[...]). As written these utilities likely won’t be generated/applied, which may regress the dialog sizing.
Comment on lines
+80
to
110
| <div ref={containerRef} className="relative"> | ||
| <SidebarMenuButton | ||
| data-testid="source-switcher-trigger" | ||
| size="lg" | ||
| className="w-full data-[state=open]:bg-sidebar-accent data-[state=open]:text-sidebar-accent-foreground" | ||
| data-state={open ? "open" : "closed"} | ||
| onClick={() => setOpen((prev) => !prev)} | ||
| > | ||
| <DropdownMenuItem | ||
| data-testid="source-switcher-option:all" | ||
| onSelect={(event) => { | ||
| event.preventDefault(); | ||
| handleSelect(null); | ||
| }} | ||
| onClick={() => handleSelect(null)} | ||
| className={currentSourceId == null ? "bg-accent text-accent-foreground" : undefined} | ||
| <div className="bg-sidebar-primary text-sidebar-primary-foreground flex aspect-square size-8 items-center justify-center rounded-lg"> | ||
| <GitBranch className="size-4" /> | ||
| </div> | ||
| <div className="flex flex-col gap-0.5 leading-none min-w-0"> | ||
| <span className="font-semibold truncate"> | ||
| {currentSource ? currentSource.label : sources[0]?.label ?? "No sources"} | ||
| </span> | ||
| {currentSource && ( | ||
| <span className="text-xs text-muted-foreground"> | ||
| {currentSource.fileCount} | ||
| {" "} | ||
| {currentSource.fileCount === 1 ? "file" : "files"} | ||
| </span> | ||
| )} | ||
| </div> | ||
| <ChevronsUpDown className="ml-auto size-4 shrink-0 text-muted-foreground" /> | ||
| </SidebarMenuButton> | ||
|
|
||
| {open && ( | ||
| <div | ||
| data-testid="source-switcher-popup" | ||
| className="absolute left-0 right-0 top-full z-50 mt-1 overflow-hidden rounded-lg border border-border/60 bg-popover text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 slide-in-from-top-2 duration-100" | ||
| > |
There was a problem hiding this comment.
SourceSwitcher replaced the shared DropdownMenu with a custom popover, but the new markup lacks common accessibility hooks (e.g. aria-expanded, aria-controls, aria-haspopup on the trigger; role="listbox"/"menu" + roving focus / arrow-key navigation for options). Consider reusing the shared dropdown/menu primitives (or adding the missing ARIA + keyboard support) to avoid regressing keyboard/screen-reader usability.
Comment on lines
+30
to
+33
| function Wrapper({ children }: { children: React.ReactNode }) { | ||
| // @ts-expect-error - the router provider types don't allow for the full flexibility of our router options, but in practice this works fine | ||
| return <RouterProvider router={router}>{children}</RouterProvider>; | ||
| } |
There was a problem hiding this comment.
The test helper currently relies on @ts-expect-error to pass children into RouterProvider. This is brittle (and the suppression reason doesn’t match the actual type error). Consider rendering <RouterProvider router={router} /> directly (and removing the unused ui parameter), or wrap RouterProvider in a separate wrapper component that doesn’t require invalid props, so tests don’t depend on suppressed type errors.
Comment on lines
+22
to
+23
| <Handle type="target" position={Position.Left} className="h-2! w-2! border-none! bg-muted-foreground!" /> | ||
|
|
There was a problem hiding this comment.
Tailwind’s important modifier is written as !h-2 / !w-2 (prefix), not h-2!. As written, these classes likely won’t apply, which can break handle sizing/styling in the graph. Please switch to the supported ! syntax (or remove the modifier if not needed).
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.
Summary
Testing
Summary by CodeRabbit
New Features
Improvements
Changes