FE-585: WIP petrinaut add hover + selected emphasis for nodes#8632
FE-585: WIP petrinaut add hover + selected emphasis for nodes#8632alex-e-leon wants to merge 7 commits intomainfrom
Conversation
This reverts commit 0a7cc56.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Updates SDCPN canvas rendering to visually de-emphasize non-related items when anything is selected (background fade plus per-node/arc overlays), and adds styling variants so connected neighbours can be highlighted while unrelated nodes/arcs are lightened. Reviewed by Cursor Bugbot for commit f1f6ff2. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: This WIP PR adds “selection adjacency” awareness to Petrinaut so nodes/edges connected to the current selection can be visually emphasized while unrelated elements are de-emphasized. Changes:
Technical Notes: Arc IDs are derived via 🤖 Was this summary useful? React with 👍 or 👎 |
| /** Check whether a node/edge is connected to any selected item via an arc. */ | ||
| isSelectedConnection: (id: string) => boolean; | ||
| /** Check whether a node/edge is connected to any selected item via an arc. */ | ||
| isNotSelectedConnection: (id: string) => boolean; |
There was a problem hiding this comment.
libs/@hashintel/petrinaut/src/state/editor-context.ts:66 — The JSDoc for isNotSelectedConnection says “connected to any selected item”, but the implementation returns true for items that are neither selected nor connected. Consider adjusting the comment so callers don’t invert the meaning.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| const isSelectedConnection = (id: string) => selectedConnections.has(id); | ||
| const isNotSelectedConnection = (id: string) => | ||
| selection.size > 0 && !isSelected(id) && !selectedConnections.has(id); |
There was a problem hiding this comment.
libs/@hashintel/petrinaut/src/state/editor-provider.tsx:239 — isNotSelectedConnection becomes true for all unselected IDs whenever selection.size > 0, even if the selection is a non-canvas item (e.g. parameter/type), which could unexpectedly dim the whole net. Consider gating this behavior to selections that include places/transitions/arcs (or when selectedConnections is non-empty).
Severity: medium
Other Locations
libs/@hashintel/petrinaut/src/views/SDCPN/sdcpn-view.tsx:400
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| * An item is included if it shares an arc with a selected item: | ||
| * - A selected place includes its connected transitions and arcs. | ||
| * - A selected transition includes its connected places and arcs. | ||
| * - A selected arc includes its source place and target transition. |
There was a problem hiding this comment.
libs/@hashintel/petrinaut/src/lib/get-connections.ts:13 — The docs say a selected arc includes its “source place and target transition”, but output arcs are transition→place as well. Consider rewording to “source and target nodes” to avoid confusion.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f1f6ff2. Configure here.
|
|
||
| const isSelectedConnection = (id: string) => selectedConnections.has(id); | ||
| const isNotSelectedConnection = (id: string) => | ||
| selection.size > 0 && !isSelected(id) && !selectedConnections.has(id); |
There was a problem hiding this comment.
Non-graph item selection fades entire graph unexpectedly
Medium Severity
When a non-graph item (type, parameter, or differential equation) is selected from the sidebar, getNodeConnections returns an empty map because it only traverses transitions/arcs. Combined with hasSelection being true, this causes: (1) the global fadeBgStyle overlay to render over the entire canvas, and (2) isNotSelectedConnection to return true for every graph item, applying the dimmed/faded visual to all nodes and the brightness(1.5) filter to all arcs. The entire Petri net appears heavily washed out when selecting a non-graph item.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f1f6ff2. Configure here.
| const selectedConnections = getNodeConnections( | ||
| petriNetDefinition.transitions, | ||
| new Set(selection.keys()), | ||
| ); |
There was a problem hiding this comment.
Expensive computation runs on every unrelated render
Medium Severity
getNodeConnections is called in the render body of EditorProvider without useMemo, so it recomputes on every state change — sidebar resizes, panel toggles, dragging state updates — even when selection and petriNetDefinition.transitions haven't changed. For large Petri nets with many transitions and arcs, this O(n) computation runs unnecessarily on unrelated interactions.
Reviewed by Cursor Bugbot for commit f1f6ff2. Configure here.
| : notSelectedConnection | ||
| ? { | ||
| filter: `brightness(${1 + NOT_SELECTED_CONNECTION_OVERLAY_OPACITY})`, | ||
| } |
There was a problem hiding this comment.
Arc brightness filter makes strokes completely invisible
Medium Severity
The NOT_SELECTED_CONNECTION_OVERLAY_OPACITY (0.5) was designed as a white overlay opacity for nodes (rgba(255,255,255, 0.5)), but for arcs it's reused as a brightness additive: brightness(1 + 0.5) = brightness(1.5). The default arc stroke #b1b1b7 has channel values ~177; multiplied by 1.5 they clip to 255, producing pure white. Unconnected arcs become completely invisible instead of subtly faded like nodes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f1f6ff2. Configure here.


🌟 What is the purpose of this PR?
WIP - not ready for review
Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR: