feat(frontend): extract floating UI primitives into @infrahub/ui and adopt in path-traversal#9480
feat(frontend): extract floating UI primitives into @infrahub/ui and adopt in path-traversal#9480pa-lem wants to merge 30 commits into
Conversation
There was a problem hiding this comment.
2 issues found across 22 files
Confidence score: 4/5
- This PR is likely safe to merge: both findings are low severity (4/10 and 3/10) and mainly affect test/docs quality rather than production runtime behavior.
- The most important issue is in
frontend/packages/ui/src/components/floating-panel/floating-panel.test.tsx, where[role="heading"]does not match semantic<h2>; this can let the hidden-state test pass while a heading is still rendered, weakening regression detection. - In
docs/superpowers/plans/2026-06-04-floating-ui-primitives.md, the success criteria reference afrontend/packages/uibuild expectation that conflicts with the known TS5101tsc -bstate, so the plan can be misleading but is not merge-blocking. - Pay close attention to
frontend/packages/ui/src/components/floating-panel/floating-panel.test.tsxanddocs/superpowers/plans/2026-06-04-floating-ui-primitives.md- fix the heading selector assertion and align success criteria with current TypeScript build constraints.
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Shadow auto-approve: would auto-approve. This PR extracts accessible, tested floating-UI primitives into the design system and adopts them in path-traversal, replacing a local hook and hand-rolled markup with shared components — all changes are in frontend UI code with 48 passing tests and zero impact on business logic, data, or infra-
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cast `.element()` return to `HTMLElement` before calling `.click()` on lines that simulate PNG/SVG export menu clicks — `.element()` returns `HTMLElement | SVGElement` and `SVGElement` has no `.click()` method. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…existing ui build failure
…reload FloatingPanel (from @infrahub/ui workspace source) imports lucide-react, which was not in Vite's pre-optimized dep list. When first encountered mid-suite, Vite triggered a re-optimization + full page reload that reset vi.mock() registry, causing remove-relationships.test.ts to fail with "vi.mocked(...).mockResolvedValueOnce is not a function". Adding lucide-react to test.deps.optimizer.web.include forces it to be pre-bundled before any test runs, eliminating the reload entirely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… for vitest 4 browser mode) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
798ff67 to
2fed087
Compare
…mponents/graph IconButton only re-defaulted Button (ghost/sm/square + required aria-label); replaced all usages with Button directly. Moved the canvas-oriented Toolbar and FloatingPanel primitives into components/graph/ to separate them from the regular primitives.
There was a problem hiding this comment.
2 issues found across 12 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would auto-approve. This PR is a well-structured extraction of floating UI primitives into the design system with thorough testing, documentation, and adoption in the path-traversal feature. It passes AI review, follows project patterns, and does not touch critical systems.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. Although the AI review found no issues and the PR includes tests, it is a refactoring that extracts and adopts shared UI primitives, reordering existing component logic and dependencies—a high-impact change that should be reviewed by a human to ensure correctness and no regressions.
Re-trigger cubic
Moves the canvas-overlay primitives out of @infrahub/ui into a dedicated @infrahub/graph package (depends on @infrahub/ui for Card/Button/useDismiss) — a home for future graph-specific components. Rewires path-traversal consumers; useDismiss stays in @infrahub/ui.
There was a problem hiding this comment.
2 issues found across 26 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/packages/graph/oxlint.config.ts">
<violation number="1" location="frontend/packages/graph/oxlint.config.ts:32">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
Invalid oxlint rule key: plugin is registered as `react-perf` but this rule uses `react_perf` (underscore instead of hyphen). The mismatched prefix means oxlint will silently ignore the rule and the intended `jsx-no-new-function-as-prop` override will not apply.</violation>
</file>
<file name="frontend/packages/graph/src/index.css">
<violation number="1" location="frontend/packages/graph/src/index.css:9">
P3: `graph` duplicates global base CSS already defined in `@infrahub/ui`, which risks style drift and redundant maintenance.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| "react/no-multi-comp": "off", | ||
| "react/only-export-components": "off", | ||
| "react/react-in-jsx-scope": "off", | ||
| "react_perf/jsx-no-new-function-as-prop": "off", |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
Invalid oxlint rule key: plugin is registered as react-perf but this rule uses react_perf (underscore instead of hyphen). The mismatched prefix means oxlint will silently ignore the rule and the intended jsx-no-new-function-as-prop override will not apply.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/packages/graph/oxlint.config.ts, line 32:
<comment>Invalid oxlint rule key: plugin is registered as `react-perf` but this rule uses `react_perf` (underscore instead of hyphen). The mismatched prefix means oxlint will silently ignore the rule and the intended `jsx-no-new-function-as-prop` override will not apply.</comment>
<file context>
@@ -0,0 +1,39 @@
+ "react/no-multi-comp": "off",
+ "react/only-export-components": "off",
+ "react/react-in-jsx-scope": "off",
+ "react_perf/jsx-no-new-function-as-prop": "off",
+ "typescript/explicit-function-return-type": "off",
+ "typescript/explicit-module-boundary-types": "off",
</file context>
| @source "./**/*.{js,ts,jsx,tsx}"; | ||
| @source "../../ui/src/**/*.{js,ts,jsx,tsx}"; | ||
|
|
||
| @font-face { |
There was a problem hiding this comment.
P3: graph duplicates global base CSS already defined in @infrahub/ui, which risks style drift and redundant maintenance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/packages/graph/src/index.css, line 9:
<comment>`graph` duplicates global base CSS already defined in `@infrahub/ui`, which risks style drift and redundant maintenance.</comment>
<file context>
@@ -0,0 +1,24 @@
+@source "./**/*.{js,ts,jsx,tsx}";
+@source "../../ui/src/**/*.{js,ts,jsx,tsx}";
+
+@font-face {
+ font-family: InterVariable;
+ font-weight: 100 900;
</file context>
@infrahub/ui is now unchanged vs develop (less PR noise). useDismiss (+ test) and the vitest harness live in @infrahub/graph alongside Toolbar/FloatingPanel. path-traversal imports useDismiss from @infrahub/graph.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Extracts the PNG/SVG export popover into @infrahub/graph (self-contained: owns its open state + outside-click/Escape dismiss). path-traversal's bottom-toolbar drops its hand-rolled menu + dismiss state.
GraphControls (zoom/fit/edge-style/layout cluster) reads ReactFlow via an @xyflow/react peer dep. Ports a Tooltip into @infrahub/ui so GraphControls keeps styled tooltips. path-traversal's bottom-toolbar adopts both. The app's vitest excludes @infrahub/graph from optimizeDeps so vi.mock reaches GraphControls' @xyflow/react import.
|
Split into stacked PRs for cleaner review: #9496 (Tooltip in @infrahub/ui) and the new @infrahub/graph PR (graph primitives + path-traversal). schema-visualizer adoption will be a third PR in its own repo. |
What
Extracts reusable floating-UI primitives into the
@infrahub/uidesign system and adopts them in the path-traversal feature, so path-traversal and the schema-visualizer can share one source of truth for their floating chrome.New
@infrahub/uiprimitives (each composes existing DS items)Toolbar+Toolbar.Divider— floating toolbar container (role="toolbar", requiresaria-label) + vertical separator.FloatingPanel— floating overlay built onCard/CardHeader/CardContent+Button: header (title/description/close) + scrollable body; optionaldismissable(outside-click + Escape).useDismiss— shared outside-pointerdown + Escape dismissal hook (dedups two prior copies).Toolbar,FloatingPanel) live undersrc/components/graph/, separate from the regular primitives (Button,Card,Modal, …). They remain generic — no graph/ReactFlow coupling.@infrahub/ui(it had no tests before).path-traversal adoption
bottom-toolbar.tsxnow usesToolbar/Toolbar.Divider, the shareduseDismiss(local copy deleted), andButtonfor icon-only controls.path-traversal-page.tsxparameters panel now usesFloatingPanel(was an app-levelContent.Card).Test infra
frontend/app/vitest.config.ts: pre-bundlelucide-reactvia ViteoptimizeDeps.include—@infrahub/uiis consumed from source, soFloatingPanel'slucide-reactimport would otherwise be discovered mid-run and trigger a Vite reload that resetsvi.mock()(breaking unrelated mocked tests).Docs
dev/knowledge/frontend/design-system.md: documents that@infrahub/uiis consumed from source (not built output), and the cross-package Tailwind@sourcerequirement for consumers.Testing
@infrahub/ui: 12 tests · path-traversal: 13 tests (bottom-toolbar) — full app browser suite green.biomeclean; touched files typecheck clean.Notes / follow-ups
opsmill/infrahub-schema-visualizer(paused: that repo can't resolve@infrahub/uistandalone yet — pending a publish/resolution strategy).@infrahub/ui'spnpm buildfails on atsconfigTS5101 baseUrldeprecation already present ondevelop(the app consumes ui from source, so this doesn't affect the app).🤖 Generated with Claude Code