Move output ports to edges level #12133

vitvakatu · 2025-01-24T10:58:24Z

Pull Request Description

Fixes #11922

ports-on-edges-layer.mp4

copy-error-button-works.mp4

Important Notes

See #12131 and #12132.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

Spurious pointerenter events with invalid *Key payloads were causing invalid state of the keyboard provider.

github-actions · 2025-01-24T11:09:10Z

🧪 Storybook is successfully deployed!

📊 Dashboard:

👀 Review changes: https://www.chromatic.com/build?appId=673738989fc3e8430364197e&number=1269
👨‍🎨 Preview storybook: https://673738989fc3e8430364197e-vddjjckxmt.chromatic.com/

vitvakatu · 2025-01-27T11:46:00Z

Oops, not exactly what I wanted, sorry for ping.

farmaazon · 2025-01-28T09:02:52Z

app/gui/src/project-view/components/GraphEditor/GraphNode.vue

+onBlur(() => {
+  graph.setNodeHovered(nodeId.value, false)
+  updateNodeHover(undefined)
+})


Is it this component's blur or window/document blur? I think it should be the latter, but the name does not say it.

Renamed to onWindowBlur, you are right.

farmaazon · 2025-01-28T09:05:34Z

app/gui/src/project-view/components/GraphEditor/GraphNode.vue

@@ -464,8 +478,8 @@ const showMenuAt = ref<{ x: number; y: number }>()
    :class="nodeClass"
    :data-node-id="nodeId"
    @pointerdown.stop
-    @pointerenter="((nodeHovered = true), updateNodeHover($event))"
-    @pointerleave="((nodeHovered = false), updateNodeHover(undefined))"
+    @pointerenter="(graph.setNodeHovered(nodeId, true), updateNodeHover($event))"


Perhaps setNodeHovered should be just a part of updateNodeHover? I think I see these two always called together.

updateNodeHover is used on its own once, though, on pointer move.

farmaazon · 2025-01-28T09:08:12Z

app/gui/src/project-view/components/GraphEditor/GraphNode.vue

+watch(isVisualizationVisible, (val) => {
+  if (!val) {
+    visualizationHovered.value = false
+  }
+})


It's strange this is needed; hiding visualization itself should emit "pointerleave", effectively setting visualizationHovered to false. Is this needed due to the modifiers problem? If yes, then add a comment here.

Yeah, I wasn’t paying enough attention here. We’re not getting pointerleave for whatever reason. I just added a comment about it.

farmaazon · 2025-01-28T09:12:20Z

app/gui/src/project-view/composables/selection.ts

+    setSoleSelected: (element: T) => {
+      setSelection(new Set([element]))
+      onSoleSelected?.(element)
+    },


I think these names are a bit confusing. setSoleSelected technically should be the same as setSelection with a single node. But only the former calls onSoleSelected.

You are right, I removed setSoleSelected, onSoleSelected is now triggered if we pass a single element.

farmaazon · 2025-01-28T09:23:05Z

app/gui/src/project-view/util/autoBlur.ts

+
+const blurHandlers = new Set<() => void>()
+
+/**
+ * Register a callback to be called when the window is blurred.
+ * The callback will be called when the window is blurred, and will be removed when the component is unmounted.
+ */
+export function onBlur(callback: () => void) {
+  blurHandlers.add(callback)
+  onUnmounted(() => {
+    blurHandlers.delete(callback)
+  })
+}
+


It's the same as useEvent(window, 'blur'), isn't it? Because useEvent also removes itself (on scope disposal, but it's effectively same as unmount).

Yes, but here we have a single event handler per application instead of multiple event handlers per node. We use the same pattern for autoBlur, for performance reasons.

kazcw · 2025-01-27T14:34:36Z

app/gui/src/project-view/components/GraphEditor/GraphEdges.vue

+const nodeIdsWithOutputPorts = computed(() =>
+  Array.from(graph.db.nodeIdToNode.entries())
+    .filter(([_, node]) => node.type !== 'output')
+    .map(([id]) => id),
+)


Why not use graph.db.nodeOutputPorts?

I guess no particular reason, just reused the code from some other place. Changed it now as suggested.

ports.

getting pointerleave events

vitvakatu added 18 commits January 17, 2025 13:37

Move node message on top

e08ad82

Use div for node background

f68802e

placing output ports on the edges layer

e79bde1

Propagating size and position information for output ports

f4503fc

Do not update keyboard state on pointerenter events.

092b5b5

Spurious pointerenter events with invalid *Key payloads were causing invalid state of the keyboard provider.

cleanup logs

6a000d3

Bring back pointerenter event

45ac723

try resetting state on blur

5810e2a

Show preview when hovering the node also, not only output port

f8740fa

Add onBlur functionality to dehover the node

fd72283

Fix create node from port button

76c4502

Better cancelation of preview visualization

bab2fb3

remove bgPaths

fccc384

deduplicate setSoleSelected

81436e1

updateNodeHoverAnim → updateNodeOutputHoverAnim

fdc436c

remove nodeOutputHovered

3eeaa3c

minor cleanup

433aadc

Fixing tests

c86ce61

vitvakatu added the -gui label Jan 24, 2025

vitvakatu self-assigned this Jan 24, 2025

vitvakatu requested review from Frizi, farmaazon, kazcw, AdRiley, PabloBuchu, indiv0, somebody1234 and MrFlashAccount as code owners January 24, 2025 10:58

update changelog

226da8f

merge with develop

ffc82b0

vitvakatu requested review from jdunkerley, radeusgd, GregoryTravis, marthasharkey, 4e6, JaroslavTulach, hubertp and Akirathan as code owners January 27, 2025 11:45

vitvakatu force-pushed the wip/vitvakatu/copy-error branch from 76a6a93 to ffc82b0 Compare January 27, 2025 11:48

vitvakatu removed request for hubertp, 4e6, radeusgd, jdunkerley, Akirathan, GregoryTravis, JaroslavTulach and marthasharkey January 27, 2025 11:48

farmaazon reviewed Jan 28, 2025

View reviewed changes

kazcw reviewed Jan 28, 2025

View reviewed changes

vitvakatu added 5 commits January 30, 2025 01:44

Merge branch 'develop' into wip/vitvakatu/copy-error

b487a28

Use graph.db.nodeOutputPorts to collect the list of nodes with output

4f584ac

ports.

Add a comment for hiding preview visualization explaining we are not

b224472

getting pointerleave events

Rename onBlur → onWindowBlur

5a91b5f

avoid confusion for setSoleSelected

281f86f

vitvakatu force-pushed the wip/vitvakatu/copy-error branch from a61697e to 281f86f Compare January 29, 2025 22:00

vitvakatu requested review from kazcw and farmaazon January 30, 2025 14:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move output ports to edges level #12133

Move output ports to edges level #12133

vitvakatu commented Jan 24, 2025

github-actions bot commented Jan 24, 2025 •

edited

Loading

vitvakatu commented Jan 27, 2025 •

edited

Loading

farmaazon Jan 28, 2025

vitvakatu Jan 29, 2025

farmaazon Jan 28, 2025

vitvakatu Jan 29, 2025

farmaazon Jan 28, 2025

vitvakatu Jan 29, 2025

farmaazon Jan 28, 2025

vitvakatu Jan 29, 2025

farmaazon Jan 28, 2025

vitvakatu Jan 29, 2025

kazcw Jan 27, 2025

vitvakatu Jan 29, 2025

Move output ports to edges level #12133

Are you sure you want to change the base?

Move output ports to edges level #12133

Conversation

vitvakatu commented Jan 24, 2025

Pull Request Description

Important Notes

Checklist

github-actions bot commented Jan 24, 2025 • edited Loading

🧪 Storybook is successfully deployed!

📊 Dashboard:

vitvakatu commented Jan 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 24, 2025 •

edited

Loading

vitvakatu commented Jan 27, 2025 •

edited

Loading