Skip to content

Commit

Permalink
Move output ports to edges level (#12133)
Browse files Browse the repository at this point in the history
  • Loading branch information
vitvakatu authored Feb 3, 2025
1 parent dcb8957 commit 1cb86a5
Show file tree
Hide file tree
Showing 19 changed files with 213 additions and 116 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [Quick Fix Import Button][12051].
- [Fixed nodes being selected after deleting other nodes or connections.][11902]
- [Redo stack is no longer lost when interacting with text literals][11908].
- [Copy button on error message is fixed][12133].
- [Tooltips are hidden when clicking on a button][12067].
- [Fixed bug when clicking header in Table Editor Widget didn't start editing
it][12064]
Expand All @@ -21,6 +22,7 @@
[12051]: https://github.com/enso-org/enso/pull/12051
[11902]: https://github.com/enso-org/enso/pull/11902
[11908]: https://github.com/enso-org/enso/pull/11908
[12133]: https://github.com/enso-org/enso/pull/12133
[12067]: https://github.com/enso-org/enso/pull/12067
[12064]: https://github.com/enso-org/enso/pull/12064
[12129]: https://github.com/enso-org/enso/pull/12129
Expand Down
2 changes: 1 addition & 1 deletion app/gui/integration-test/project-view/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export async function expectNodePositionsInitialized(page: Page, yPos: number) {
// Wait until edges are initialized and displayed correctly.
await expect(page.getByTestId('broken-edge')).toBeHidden()
// Wait until node sizes are initialized.
await expect(locate.graphNode(page).first().locator('.bgFill')).toBeVisible()
await expect(locate.graphNode(page).first().locator('.nodeBackground')).toBeVisible()
// TODO: The yPos should not need to be a variable. Instead, first automatically positioned nodes
// should always have constant known position. This is a bug caused by incorrect layout after
// entering a function. To be fixed with #9255
Expand Down
21 changes: 14 additions & 7 deletions app/gui/integration-test/project-view/componentBrowser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,18 @@ test('Different ways of opening Component Browser', async ({ page }) => {
await locate.graphEditor(page).press('Enter')
await expectAndCancelBrowser(page, '', 'selected')
// Dragging out an edge
let outputPort = await locate.outputPortCoordinates(locate.graphNodeByBinding(page, 'selected'))
let outputPort = await locate.outputPortCoordinates(
page,
locate.graphNodeByBinding(page, 'selected'),
)
await page.mouse.click(outputPort.x, outputPort.y)
await locate.graphEditor(page).click({ position: { x: 100, y: 500 } })
await expectAndCancelBrowser(page, '', 'selected')
// Double-clicking port
// TODO[ao] Without timeout, even the first click would be treated as double due to previous
// event. Probably we need a better way to simulate double clicks.
await page.waitForTimeout(600)
outputPort = await locate.outputPortCoordinates(locate.graphNodeByBinding(page, 'selected'))
outputPort = await locate.outputPortCoordinates(page, locate.graphNodeByBinding(page, 'selected'))
await page.mouse.click(outputPort.x, outputPort.y)
await page.mouse.click(outputPort.x, outputPort.y)
await expectAndCancelBrowser(page, '', 'selected')
Expand All @@ -79,15 +82,16 @@ test('Opening Component Browser from output port buttons', async ({ page }) => {
// Small (+) button shown when node is hovered
const node = locate.graphNodeByBinding(page, 'selected')
await locate.graphNodeIcon(node).hover()
await expect(locate.createNodeFromPort(node)).toBeVisible()
await locate.createNodeFromPort(node).click({ force: true })
const createNodeFromPortButton = await locate.createNodeFromPortButton(page, node)
await expect(createNodeFromPortButton).toBeVisible()
await createNodeFromPortButton.click({ force: true })
await expectAndCancelBrowser(page, '', 'selected')

// Small (+) button shown when node is selected
await page.keyboard.press('Escape')
await node.click()
await expect(locate.createNodeFromPort(node)).toBeVisible()
await locate.createNodeFromPort(node).click({ force: true })
await expect(createNodeFromPortButton).toBeVisible()
await createNodeFromPortButton.click({ force: true })
await expectAndCancelBrowser(page, '', 'selected')
})

Expand All @@ -111,7 +115,10 @@ test('Graph Editor pans to Component Browser', async ({ page }) => {
await page.mouse.move(100, 280)
await page.mouse.up({ button: 'middle' })
await expect(locate.graphNodeByBinding(page, 'five')).toBeInViewport()
const outputPort = await locate.outputPortCoordinates(locate.graphNodeByBinding(page, 'final'))
const outputPort = await locate.outputPortCoordinates(
page,
locate.graphNodeByBinding(page, 'final'),
)
await page.mouse.click(outputPort.x, outputPort.y)
await locate.graphEditor(page).click({ position: { x: 100, y: 1700 } })
await expect(locate.graphNodeByBinding(page, 'five')).not.toBeInViewport()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ test('Conditional ports: Disabled', async ({ page }) => {

// When a port is disabled, it doesn't react to hovering with a disconnected edge,
// and any attempt to connect to it should open the CB.
const outputPort = await outputPortCoordinates(graphNodeByBinding(page, 'final'))
const outputPort = await outputPortCoordinates(page, graphNodeByBinding(page, 'final'))
await page.mouse.click(outputPort.x, outputPort.y)
await conditionalPort.hover()
await expect(conditionalPort).not.toHaveClass(/isTarget/)
Expand All @@ -101,7 +101,7 @@ test('Conditional ports: Enabled', async ({ page }) => {
await page.keyboard.down(CONTROL_KEY)

await expect(conditionalPort).toHaveClass(/enabled/)
const outputPort = await outputPortCoordinates(graphNodeByBinding(page, 'final'))
const outputPort = await outputPortCoordinates(page, graphNodeByBinding(page, 'final'))
await page.mouse.click(outputPort.x, outputPort.y)
await conditionalPort.hover()
await expect(conditionalPort).toHaveClass(/isTarget/)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ test('Node can open and load visualization', async ({ page }) => {
test('Previewing visualization', async ({ page }) => {
await actions.goToGraph(page)
const node = locate.graphNode(page).last()
const port = await locate.outputPortCoordinates(node)
const port = await locate.outputPortCoordinates(page, node)
await page.keyboard.down('Meta')
await page.keyboard.down('Control')
await expect(locate.anyVisualization(page)).toBeHidden()
Expand Down
16 changes: 13 additions & 3 deletions app/gui/integration-test/project-view/locate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ export const componentMenu = componentLocator('.ComponentMenu')
export const addNewNodeButton = componentLocator('.PlusButton')
export const componentBrowser = componentLocator('.ComponentBrowser')
export const nodeOutputPort = componentLocator('.outputPortHoverArea')
export const createNodeFromPort = componentLocator('.CreateNodeFromPortButton .plusIcon')
export const editorRoot = componentLocator('.CodeMirror')
export const nodeComment = componentLocator('.GraphNodeComment')
export const nodeCommentContent = componentLocator('.GraphNodeComment div[contentEditable]')
Expand Down Expand Up @@ -185,11 +184,22 @@ export async function edgesToNode(page: Page, node: Locator) {
* Returns a location that can be clicked to activate an output port.
* Using a `Locator` would be better, but `position` option of `click` doesn't work.
*/
export async function outputPortCoordinates(node: Locator) {
const outputPortArea = await node.locator('.outputPortHoverArea').boundingBox()
export async function outputPortCoordinates(page: Page, node: Locator) {
const nodeId = await node.getAttribute('data-node-id')
const outputPortArea = await page
.locator(`.GraphNodeOutputPorts[data-output-ports-node-id="${nodeId}"] .outputPortHoverArea`)
.boundingBox()
expect(outputPortArea).not.toBeNull()
assert(outputPortArea)
const centerX = outputPortArea.x + outputPortArea.width / 2
const bottom = outputPortArea.y + outputPortArea.height
return { x: centerX, y: bottom - 2.0 }
}

/** Returns a locator for the create node from port button. */
export async function createNodeFromPortButton(page: Page, node: Locator) {
const nodeId = await node.getAttribute('data-node-id')
return page.locator(
`.GraphNodeOutputPorts[data-output-ports-node-id="${nodeId}"] .CreateNodeFromPortButton .plusIcon`,
)
}
3 changes: 2 additions & 1 deletion app/gui/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import ProjectView from '@/ProjectView.vue'
import { provideAppClassSet } from '@/providers/appClass'
import { provideGuiConfig } from '@/providers/guiConfig'
import { provideTooltipRegistry } from '@/providers/tooltipRegistry'
import { registerAutoBlurHandler } from '@/util/autoBlur'
import { registerAutoBlurHandler, registerGlobalBlurHandler } from '@/util/autoBlur'
import { baseConfig, configValue, mergeConfig, type ApplicationConfigValue } from '@/util/config'
import { urlParams } from '@/util/urlParams'
import { useQueryClient } from '@tanstack/vue-query'
Expand Down Expand Up @@ -38,6 +38,7 @@ const queryClient = useQueryClient()
provideGuiConfig(appConfigValue)
registerAutoBlurHandler()
registerGlobalBlurHandler()
onMounted(() => {
if (appConfigValue.value.window.vibrancy) {
Expand Down
8 changes: 7 additions & 1 deletion app/gui/src/project-view/components/GraphEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ const nodeSelection = provideGraphSelection(
{
isValid: (id) => graphStore.db.isNodeId(id),
onSelected: (id) => graphStore.db.moveNodeToTop(id),
onSoleSelected: (id) => graphStore.db.moveNodeToTop(id),
toSorted: (ids) => {
const idsSet = new Set(ids)
const inputNodes = [
Expand Down Expand Up @@ -644,7 +645,12 @@ const groupColors = computed(() => {
@createNodes="createNodesFromSource"
@toggleDocPanel="toggleRightDockHelpPanel"
/>
<GraphEdges :navigator="graphNavigator" @createNodeFromEdge="handleEdgeDrop" />
<GraphEdges
:navigator="graphNavigator"
@createNodeFromEdge="handleEdgeDrop"
@createNodeFromPort="createNodesFromSource"
@outputPortDoubleClick="handleNodeOutputPortDoubleClick"
/>
<ComponentBrowser
v-if="componentBrowserOpened"
ref="componentBrowser"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
<script setup lang="ts">
import { ref } from 'vue'
import { AstId } from 'ydoc-shared/ast'
const props = defineProps<{ portId: AstId }>()
const emit = defineEmits<{ click: [] }>()
const hovered = ref(false)
</script>

<template>
<g :class="{ CreateNodeFromPortButton: true, hovered }" @click="emit('click')">
<g class="CreateNodeFromPortButton">
<rect :class="{ connection: true }" fill="currentColor"></rect>
<g :class="{ plusIcon: true }">
<mask :id="`${props.portId}_add_node_clip_path`">
Expand All @@ -18,7 +15,7 @@ const hovered = ref(false)
</mask>
<circle :mask="`url(#${props.portId}_add_node_clip_path)`" fill="currentColor"></circle>
</g>
<rect class="hoverArea" @pointerenter="hovered = true" @pointerleave="hovered = false"></rect>
<rect class="hoverArea"></rect>
</g>
</template>

Expand All @@ -31,6 +28,7 @@ const hovered = ref(false)
--topOffset: 40px;
--color-dimmed: color-mix(in oklab, var(--color-node-primary) 60%, white 40%);
--color: var(--color-node-primary);
pointer-events: all;
}
.connection {
Expand Down
4 changes: 2 additions & 2 deletions app/gui/src/project-view/components/GraphEditor/GraphEdge.vue
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const sourceMask = computed<NodeMask | undefined>(() => {
if (!nodeRect) return
const animProgress =
startsInPort.value ?
((sourceNode.value && graph.nodeHoverAnimations.get(sourceNode.value)) ?? 0)
((sourceNode.value && graph.nodeOutputHoverAnimations.get(sourceNode.value)) ?? 0)
: 0
const padding = animProgress * VISIBLE_PORT_MASK_PADDING
if (!maskSource && padding === 0) return
Expand Down Expand Up @@ -287,7 +287,7 @@ const arrowPath = [
const sourceHoverAnimationStyle = computed(() => {
if (!animateFromSourceHover || !base.value || !sourceNode.value) return {}
const progress = graph.nodeHoverAnimations.get(sourceNode.value) ?? 0
const progress = graph.nodeOutputHoverAnimations.get(sourceNode.value) ?? 0
if (progress === 1) return {}
const currentLength = progress * base.value.getTotalLength()
return {
Expand Down
27 changes: 25 additions & 2 deletions app/gui/src/project-view/components/GraphEditor/GraphEdges.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<script setup lang="ts">
import GraphEdge from '@/components/GraphEditor/GraphEdge.vue'
import GraphNodeOutputPorts from '@/components/GraphEditor/GraphNodeOutputPorts.vue'
import type { NodeCreationOptions } from '@/components/GraphEditor/nodeCreation'
import type { GraphNavigator } from '@/providers/graphNavigator'
import { injectGraphSelection } from '@/providers/graphSelection'
import { injectInteractionHandler, type Interaction } from '@/providers/interactionHandler'
Expand All @@ -9,17 +11,21 @@ import { Ast } from '@/util/ast'
import { isAstId, type AstId } from '@/util/ast/abstract'
import { Vec2 } from '@/util/data/vec2'
import { toast } from 'react-toastify'
import { computed } from 'vue'
const graph = useGraphStore()
const selection = injectGraphSelection(true)
const interaction = injectInteractionHandler()
const nodeSelection = injectGraphSelection(true)
const props = defineProps<{
navigator: GraphNavigator
}>()
const emits = defineEmits<{
const emit = defineEmits<{
createNodeFromEdge: [source: AstId, position: Vec2]
createNodeFromPort: [source: NodeId, options: NodeCreationOptions[]]
outputPortDoubleClick: [portId: AstId]
}>()
const MIN_DRAG_MOVE = 10
Expand Down Expand Up @@ -58,7 +64,7 @@ function edgeInteractionClick() {
if (target == null) {
if (graph.mouseEditedEdge?.disconnectedEdgeTarget != null)
disconnectEdge(graph.mouseEditedEdge.disconnectedEdgeTarget)
emits('createNodeFromEdge', source, props.navigator.sceneMousePos ?? Vec2.Zero)
emit('createNodeFromEdge', source, props.navigator.sceneMousePos ?? Vec2.Zero)
} else {
createEdge(source, target)
}
Expand Down Expand Up @@ -113,6 +119,10 @@ function createEdge(source: AstId, target: PortId) {
}
}
}
const nodeIdsWithOutputPorts = computed(() =>
[...graph.db.nodeOutputPorts.allForward()].map(([id]) => id),
)
</script>

<template>
Expand All @@ -125,6 +135,19 @@ function createEdge(source: AstId, target: PortId) {
:edge="graph.outputSuggestedEdge"
animateFromSourceHover
/>
<template v-for="id in nodeIdsWithOutputPorts" :key="id">
<GraphNodeOutputPorts
:nodeId="id"
:forceVisible="graph.nodeHovered.get(id) ?? false"
@newNodeClick="
(nodeSelection?.setSelection(new Set([id])),
emit('createNodeFromPort', id, [{ commit: false, content: undefined }]))
"
@portClick="(event, portId) => graph.createEdgeFromOutput(portId, event)"
@portDoubleClick="(_event, portId) => emit('outputPortDoubleClick', portId)"
@update:hoverAnim="graph.updateNodeOutputHoverAnim(id, $event)"
/>
</template>
</svg>
<svg v-if="graph.mouseEditedEdge" :viewBox="props.navigator.viewBox" class="overlay aboveNodes">
<GraphEdge :edge="graph.mouseEditedEdge" maskSource />
Expand Down
Loading

0 comments on commit 1cb86a5

Please sign in to comment.