[CHAIN] feat(ui): add graph interactions and filtered view#10756
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.
87f804b to
d01b532
Compare
…n' into PROWLER-1375/graph-interactions-filtered-view
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>
There was a problem hiding this comment.
The local interaction state does not reset when the graph data changes. expandedResources and hoveredNodeId live inside GraphCanvas, but nothing clears them after a new query or scan switch, so this can leak stale expansion/highlight state into the next graph and it also makes the "state resets properly" claim false. Please reset both when data changes, or remount the graph with a stable key derived from the active scan/query.
|
Question: was it intentional to stop passing |
|
Following up on the reuse question from the previous PR and the comment here: #10705 (comment). Do you think it is worth extracting the repeated node primitives for all three node components here instead of keeping every detail duplicated inside each one? I would still keep |
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.
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.
|
@Alan-TheGentleman comment addressed and branch updated with previous chained PR |
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
f878f121 resets expandedResources and hoveredNodeId on data change and wires onViewFinding / viewFindingLoading through NodeDetailPanel in fullscreen. Approving.
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
f878f121 resets expandedResources and hoveredNodeId on data change and wires onViewFinding / viewFindingLoading through NodeDetailPanel in fullscreen. Approving.
…aph-interactions-filtered-view
1d54244
into
PROWLER-1273/react-flow-migration
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 MVP milestone — it adds all user interactions to the graph: node selection, filtered view, and path highlighting.
Description
Implements graph interactions on top of the React Flow rendering from PR #10705.
Changes:
NodeDetailPanelcomponent for fullscreen and main detail panelsGraphCanvasPropswithOmit<AttackPathGraphProps, "className">Steps to review
NodeDetailContentChecklist
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.