[CHAIN] feat(ui): add graph export, minimap and fullscreen polish#10800
Conversation
- Narrow GraphEdge.source/target from string | object to string - Remove typeof guards in adapter, graph component, and utilities - Remove unused panX, panY, zoomLevel fields from graph state store - Delete orphaned NodeRelationships component (never integrated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aligns with design decision D4 — GraphCanvas now owns layoutWithDagre() and node enrichment (selection, hasFindings), making AttackPathGraph a thin wrapper around ReactFlowProvider. This prepares PR2 where GraphCanvas needs setEdges() ownership for hover highlight. Also removes unused AttackPathGraphRef deprecated alias and isFilteredView prop from outer component. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rewrite attack-path-graph.tsx from D3 imperative SVG to React Flow declarative components - Add pure layoutWithDagre() function using @dagrejs/dagre maintained fork - Create custom node components: FindingNode (hexagon), ResourceNode (pill), InternetNode (globe) - Implement outer/inner component split (ReactFlowProvider constraint) - Disable export button temporarily (re-enabled in PR3 with html-to-image) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace require() with ESM import for @dagrejs/dagre - Pre-compute resourcesWithFindings set to avoid O(n*e) per-node loop - Extract isFindingNode helper to deduplicate label checks - Remove no-op handleWheel handler and unused initialNodeId prop - Replace dangerouslySetInnerHTML with style children - Add aria-hidden to SVG defs and role/aria-label to graph container Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace nodes.find() inside edges loop with precomputed Map - Fix misleading complexity comment Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add Tier 1 click: toggle finding visibility on resource nodes - Add Tier 2 click: enter filtered subgraph view on finding nodes - Add path highlight on hover with upstream/downstream edges - Add node selection with visual indicator and detail panel sync - Fix graph state not resetting on scan change - Extract NodeDetailPanel to deduplicate fullscreen and main panels Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace nodes.find() inside edges loop with pre-built Map - Update graph-interactions spec to reflect D4 render-body derivation - Update Ctrl+scroll spec to document zoomOnPinch native handling Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Post-rebase conflict resolution: extend the local NodeDetailPanel with onViewFinding / viewFindingLoading props so the master-added finding drawer flow remains wired through the extracted panel component.
…n' into PROWLER-1375/graph-interactions-filtered-view
CodeRabbit CLI and similar tools may create ui/.claude/settings.local.json with machine-specific paths or local credentials; add .claude/ to the ui gitignore so it cannot accidentally reach source control. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…raph Delivers PROWLER-1376 (PR3 of the React Flow migration): - exportGraphAsPNG() rewritten to use modern-screenshot (domToPng) against the React Flow container, with viewport math via getViewportForBounds() to fit all nodes regardless of the user's zoom/pan. Picked modern-screenshot over the better-known html-to-image (inactive since 2025-04) — near-identical API, actively maintained, better modern-CSS support for the React Flow viewport. - Export path now signals missing-container and empty-node cases via explicit Errors instead of silent early returns, so the caller shows a real failure toast instead of a misleading success. - GraphHandle exposes getNodesBounds() so the export honors the React Flow instance's nodeLookup (correct bounds for sub-flows). - Adds a fullscreen Dialog with its own AttackPathGraph instance and controls, plus the React Flow <MiniMap /> in the main view. - Drops dagre/@types/dagre (feature moved to @dagrejs/dagre earlier in the migration); adds modern-screenshot 4.7.0. - Bumps the openspec submodule to reflect the modern-screenshot choice and the new export-failure Scenario in graph-export. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Satisfies Radix Dialog a11y requirement (missing description / aria-describedby warning). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alejandrobailo
left a comment
There was a problem hiding this comment.
LGTM!, Just one detail, It would be nice if the minimap matched the dark/light theme.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aligns with design decision D4 — GraphCanvas now owns layoutWithDagre() and node enrichment (selection, hasFindings), making AttackPathGraph a thin wrapper around ReactFlowProvider. This prepares PR2 where GraphCanvas needs setEdges() ownership for hover highlight. Also removes unused AttackPathGraphRef deprecated alias and isFilteredView prop from outer component. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rewrite attack-path-graph.tsx from D3 imperative SVG to React Flow declarative components - Add pure layoutWithDagre() function using @dagrejs/dagre maintained fork - Create custom node components: FindingNode (hexagon), ResourceNode (pill), InternetNode (globe) - Implement outer/inner component split (ReactFlowProvider constraint) - Disable export button temporarily (re-enabled in PR3 with html-to-image) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace require() with ESM import for @dagrejs/dagre - Pre-compute resourcesWithFindings set to avoid O(n*e) per-node loop - Extract isFindingNode helper to deduplicate label checks - Remove no-op handleWheel handler and unused initialNodeId prop - Replace dangerouslySetInnerHTML with style children - Add aria-hidden to SVG defs and role/aria-label to graph container Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace nodes.find() inside edges loop with precomputed Map - Fix misleading complexity comment Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
- The new export path still needs automated coverage. Please add one test for the success path with a mocked
domToPng()call and one test for the failure cases (containerElementmissing andboundsnull), so we know the toast behavior and the viewport math contract do not regress the next time this helper changes.
|
Question: was it intentional to keep the compact fullscreen |
Address Alan's review comment on #10705: deduplicate the pieces that all three React Flow node components share without forcing a generic node renderer. FindingNode, ResourceNode and InternetNode stay separate; only the boilerplate is extracted. - HiddenHandles: invisible target/source handles used identically by all three nodes. - truncateLabel: shared label truncation helper used by FindingNode and ResourceNode (24/22 chars). - resolveNodeColors: layered fill/border resolution covering selection highlight and finding-alert state. Per-node strokeWidth is intentionally kept local since each node has its own visual weight.
Address Alan's CHANGES_REQUESTED on #10705: add automated coverage so the new React Flow rendering contract is guarded against regressions. - _lib/layout.test.ts: deterministic checks on layoutWithDagre — empty input, node typing/dimensions by labels, run-to-run determinism, top-left position offset, container relationship inversion (RUNS_IN & friends with originalSource/originalTarget preserved on the rfEdge), finding-edge animation/className, and stable rfEdge IDs. - graph-controls.test.tsx: GraphControls Export button is disabled and surfaces "Export available soon" without an onExport callback, and enabled + invokes the callback when one is provided. Mounting the full AttackPathGraph with React Flow in jsdom is fragile (ResizeObserver, dimensions, edge measurement); that coverage is left for the next chained PR which introduces Vitest Browser Mode.
Test additions are an internal contract; user-facing changelogs only use the standard Added/Changed/Fixed/Removed/Security sections.
- Skip browser-replay artifacts under .expect/ to prevent the prettier plugin from crashing the lint pipeline.
- Clear expanded resources and hovered node when graph data changes, preventing stale interaction state across scans and query runs. - Forward View Finding handler to the fullscreen detail panel so related-finding actions are no longer no-ops.
…375/graph-interactions-filtered-view Bring chained PR #10705 up to date in #10756 so both target the same feature branch with consistent React Flow refactor. Conflict resolutions: - nodes/{finding,internet,resource}-node.tsx: take 1374 versions (use HiddenHandles primitive + resolveNodeColors / truncateLabel helpers) - _lib/graph-utils.ts: keep 1375 perf optimization (nodeLabelMap O(1)) - _components/graph/attack-path-graph.tsx: keep 1375 (already includes React Flow refactor + interactions, filtered view, hover highlight) - attack-paths-page.tsx: keep 1375 (already extracts NodeDetailPanel) Inbound from 1374: shared HiddenHandles, resolveNodeColors, truncateLabel, layoutWithDagre tests, graph-controls tests.
Use resolvedTheme from next-themes to drive MiniMap bgColor, maskColor and maskStrokeColor, and reuse getNodeColor/getNodeBorderColor so minimap nodes match the main graph palette in both light and dark modes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the workaround in b3e08a6. The .expect/ directory was local tooling output and is no longer kept under the worktree, so the ignore rule in eslint.config.mjs is dead weight.
…WLER-1376/export-fullscreen-minimap
- Add jsdom unit tests for exportGraphAsPNG validation guards (missing container, viewport, or bounds) and domToPng failure - Add jsdom unit test for exportGraphAsJSON serialization failure - Browser-mode coverage for happy paths is reserved for the vitest-browser harness in the next chained PR
|
@Alan-TheGentleman Addressed changed, added a few tests. The rest are being deferred into the next chained PR |
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
Failure paths covered in df7e0d95, theme-aware minimap in ab5a2a34, onViewFinding wired through fullscreen in f878f121. The success-path test for exportGraphAsPNG() was deferred to #10970 and is being tracked there. Approving.
…port-fullscreen-minimap # Conflicts: # ui/app/(prowler)/attack-paths/(workflow)/query-builder/_components/graph/attack-path-graph.tsx # ui/app/(prowler)/attack-paths/(workflow)/query-builder/_lib/index.ts # ui/app/(prowler)/attack-paths/(workflow)/query-builder/attack-paths-page.tsx
a4fc230
into
PROWLER-1273/react-flow-migration
…0800) Co-authored-by: Pablo F.G <pablo.fernandez@prowler.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🔗 Part of Chained PRs
PROWLER-1273/react-flow-migrationChain Overview
Context
Part of chained PRs for the React Flow migration. This is the Phase 1 complete milestone — it restores PNG export on top of React Flow, adds the built-in minimap, and polishes the fullscreen dialog. With this PR merged, every feature the D3 implementation had is back and the new minimap capability is in.
Description
Builds on the MVP interactions from #10756 to finalize Phase 1 of the migration.
Changes:
exportGraphAsPNG()now usesmodern-screenshot+ React Flow'sgetViewportForBounds()so zoomed and filtered views export correctly (full-graph or filtered-subgraph, never just the visible viewport). The oldXMLSerializer/getBBox()approach is removed — React Flow renders HTML divs, not SVG.GraphHandleexposesgetNodesBounds(): Rect | nullbacked by theuseReactFlow()hook, sonodeLookupis honored and the RF "sub-flows" warning is silenced.onExportis wired back intoGraphControlson both the main view and the fullscreen dialog; PR1 previously disabled it with a placeholder tooltip.<MiniMap pannable zoomable>is added inside<ReactFlow>— new capability, not a replacement.DialogDescriptionadded to the fullscreen dialog to satisfy Radix's a11y contract (silences theMissing Description or aria-describedbywarning).dagre+@types/dagre(the migration uses@dagrejs/dagre).d3and@types/d3stay — used bymap-chart.tsx/threat-map.tsxfor geographic visualizations, unrelated to attack paths.Steps to review
Checklist
Community Checklist
UI
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.