Skip to content

Commit

Permalink
Input nodes (#11165)
Browse files Browse the repository at this point in the history
  • Loading branch information
vitvakatu authored Sep 30, 2024
1 parent e3eb1ec commit eea23b1
Show file tree
Hide file tree
Showing 23 changed files with 347 additions and 99 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
- [Fixed issue where drag'n'dropped files were not uploaded in cloud
projects.][11014]
- [Fixed files associations not properly registered on Windows][11030]
- [Input components corresponding to function arguments are now
displayed.][11165]
- [Fixed "rename project" button being broken after not changing project
name][11103]
- [Numbers starting with dot (`.5`) are accepted in Numeric Widget][11108]
Expand All @@ -41,6 +43,7 @@
[11001]: https:/enso-org/enso/pull/11001
[11014]: https:/enso-org/enso/pull/11014
[11030]: https:/enso-org/enso/pull/11030
[11165]: https:/enso-org/enso/pull/11165
[11103]: https:/enso-org/enso/pull/11103
[11108]: https:/enso-org/enso/pull/11108
[11056]: https:/enso-org/enso/pull/11056
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/e2e/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export async function goToGraph(page: Page, closeDocPanel: boolean = true) {
await expect(page.getByTestId('rightDock')).toBeHidden()
}
// Wait for position initialization
await expectNodePositionsInitialized(page, 72)
await expectNodePositionsInitialized(page, -16)
}

export async function expectNodePositionsInitialized(page: Page, yPos: number) {
Expand Down
60 changes: 49 additions & 11 deletions app/gui2/e2e/collapsingAndEntering.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,9 @@ test('Collapsing nodes', async ({ page }) => {
module: 'local.Mock_Project',
name: 'collapsed',
isStatic: true,
aliases: [],
arguments: [{ name: 'five', reprType: 'Any', isSuspended: false, hasDefault: false }],
selfType: 'local.Mock_Project',
returnType: 'Standard.Base.Any.Any',
documentation: [],
annotations: [],
})
const collapsedNode = locate.graphNodeByBinding(page, 'prod')
Expand All @@ -102,8 +100,8 @@ test('Collapsing nodes', async ({ page }) => {
await expect(collapsedNode.locator('.WidgetTopLevelArgument')).toHaveText('five')

await locate.graphNodeIcon(collapsedNode).dblclick()
await actions.ensureNoCircularMenusVisibleDueToHovering(page)
await expect(locate.graphNode(page)).toHaveCount(4)
await expect(locate.graphNode(page)).toHaveCount(5)
await expect(locate.inputNode(page)).toHaveCount(1)
await expect(locate.graphNodeByBinding(page, 'ten')).toExist()
await expect(locate.graphNodeByBinding(page, 'sum')).toExist()
await expect(locate.graphNodeByBinding(page, 'prod')).toExist()
Expand All @@ -114,7 +112,8 @@ test('Collapsing nodes', async ({ page }) => {
// Wait till node is selected.
await expect(locate.graphNodeByBinding(page, 'ten').and(page.locator('.selected'))).toHaveCount(1)
await page.keyboard.press(COLLAPSE_SHORTCUT)
await expect(locate.graphNode(page)).toHaveCount(4)
await expect(locate.graphNode(page)).toHaveCount(5)
await expect(locate.inputNode(page)).toHaveCount(1)

const secondCollapsedNode = locate.graphNodeByBinding(page, 'ten')
await expect(secondCollapsedNode.locator('.WidgetToken')).toHaveText(['Main', '.', 'collapsed1'])
Expand All @@ -124,6 +123,23 @@ test('Collapsing nodes', async ({ page }) => {
await expect(locate.graphNodeByBinding(page, 'ten')).toExist()
})

test('Input node', async ({ page }) => {
await actions.goToGraph(page)
await enterToFunc2(page)

const inputNode = locate.inputNode(page)
await expect(inputNode).toHaveCount(1)
// Input node with identifier should have the icon and an identifier.
await expect(inputNode.locator('.WidgetIcon')).toHaveCount(1)
await expect(inputNode.locator('.WidgetToken')).toContainText('a')

await inputNode.click()
await page.keyboard.press('Delete')
await expect(inputNode).toHaveCount(1)
await inputNode.locator('.More').click({})
await expect(inputNode.getByTestId('removeNode')).toHaveClass(/(?<=^| )disabled(?=$| )/)
})

test('Output node', async ({ page }) => {
await actions.goToGraph(page)
await enterToFunc2(page)
Expand All @@ -145,11 +161,31 @@ test('Output node is not collapsed', async ({ page }) => {
await actions.goToGraph(page)
await enterToFunc2(page)

await locate.outputNode(page).click({ modifiers: ['Shift'] })
await locate
.graphNodeByBinding(page, 'r')
.locator('.grab-handle')
.click({ modifiers: ['Shift'] })
await locate.outputNode(page).click({ modifiers: ['Shift'] })

await page.getByLabel('Group Selected Components').click()
await expect(locate.graphNodeByBinding(page, 'r').locator('.WidgetToken')).toHaveText([
'Main',
'.',
'collapsed',
'a',
])
await expect(locate.inputNode(page)).toHaveCount(1)
})

test('Input node is not collapsed', async ({ page }) => {
await actions.goToGraph(page)
await enterToFunc2(page)

await locate
.graphNodeByBinding(page, 'r')
.locator('.grab-handle')
.click({ modifiers: ['Shift'] })
await locate.inputNode(page).click({ modifiers: ['Shift'] })

await page.getByLabel('Group Selected Components').click()
await expect(locate.graphNodeByBinding(page, 'r').locator('.WidgetToken')).toHaveText([
Expand All @@ -162,7 +198,7 @@ test('Output node is not collapsed', async ({ page }) => {
})

async function expectInsideMain(page: Page) {
await actions.expectNodePositionsInitialized(page, 72)
await actions.expectNodePositionsInitialized(page, -16)
await expect(locate.graphNode(page)).toHaveCount(MAIN_FILE_NODES)
await expect(locate.graphNodeByBinding(page, 'five')).toExist()
await expect(locate.graphNodeByBinding(page, 'ten')).toExist()
Expand All @@ -177,16 +213,18 @@ async function expectInsideMain(page: Page) {
}

async function expectInsideFunc1(page: Page) {
await actions.expectNodePositionsInitialized(page, 216)
await expect(locate.graphNode(page)).toHaveCount(3)
await actions.expectNodePositionsInitialized(page, -88)
await expect(locate.graphNode(page)).toHaveCount(4)
await expect(locate.inputNode(page)).toHaveCount(1)
await expect(locate.graphNodeByBinding(page, 'f2')).toExist()
await expect(locate.graphNodeByBinding(page, 'result')).toExist()
await expect(locate.outputNode(page)).toHaveCount(1)
}

async function expectInsideFunc2(page: Page) {
await actions.expectNodePositionsInitialized(page, 144)
await expect(locate.graphNode(page)).toHaveCount(2)
await actions.expectNodePositionsInitialized(page, -88)
await expect(locate.graphNode(page)).toHaveCount(3)
await expect(locate.inputNode(page)).toHaveCount(1)
await expect(locate.graphNodeByBinding(page, 'r')).toExist()
await expect(locate.outputNode(page)).toHaveCount(1)
}
Expand Down
3 changes: 3 additions & 0 deletions app/gui2/e2e/locate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ export function graphNodeIcon(node: Node) {
export function selectedNodes(page: Page | Locator): Node {
return page.locator('.GraphNode.selected') as Node
}
export function inputNode(page: Page | Locator): Node {
return page.locator('.GraphNode.inputNode') as Node
}
export function outputNode(page: Page | Locator): Node {
return page.locator('.GraphNode.outputNode') as Node
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import {
collapsedNodePlacement,
inputNodePlacement,
mouseDictatedPlacement,
nonDictatedPlacement,
previousNodeDictatedPlacement,
type Environment,
type InputNodeEnvironment,
} from '@/components/ComponentBrowser/placement'
import * as iterable from '@/util/data/iterable'
import { chain, map, range } from '@/util/data/iterable'
Expand Down Expand Up @@ -423,6 +425,38 @@ describe('Collapsed node placement', () => {
})
})

describe('Input node placement', () => {
function environment(inputNodeRects: Rect[], nonInputNodeRects: Rect[]): InputNodeEnvironment {
return {
screenBounds,
nodeRects: [...inputNodeRects, ...nonInputNodeRects],
nonInputNodeRects: nonInputNodeRects,
selectedNodeRects: [],
}
}

test('No input nodes, single component', () => {
const X = 1100
const Y = 700
const nonInputNodeRects = [rectAt(X, Y)]
const result = inputNodePlacement(size, environment([], nonInputNodeRects), gap)
expect(result).toEqual(new Vec2(X, 656))
})

test('No input nodes, two components', () => {
const nonInputNodeRects = [rectAt(1000, 600), rectAt(1300, 800)]
const result = inputNodePlacement(size, environment([], nonInputNodeRects), gap)
expect(result).toEqual(new Vec2(1000, 556))
})

test('One input node, two components', () => {
const nonInputNodeRects = [rectAt(1000, 600), rectAt(1300, 800)]
const inputNodeRects = [rectAt(1000, 556)]
const result = inputNodePlacement(size, environment(inputNodeRects, nonInputNodeRects), gap)
expect(result).toEqual(new Vec2(1124, 556))
})
})

// === Helpers for debugging ===

function generateVueCodeForNonDictatedPlacement(newNode: Rect, rects: Rect[]) {
Expand Down
28 changes: 27 additions & 1 deletion app/gui2/src/components/ComponentBrowser/placement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export function usePlacement(nodeRects: ToValue<Iterable<Rect>>, screenBounds: T
previousNodeDictatedPlacement(nodeSize, environment(selectedNodeRects), gap),
collapse: (selectedNodeRects: Iterable<Rect>, nodeSize: Vec2 = DEFAULT_NODE_SIZE): Vec2 =>
collapsedNodePlacement(nodeSize, environment(selectedNodeRects), gap),
input: (nonInputNodeRects: Iterable<Rect>, nodeSize: Vec2 = DEFAULT_NODE_SIZE): Vec2 =>
inputNodePlacement(nodeSize, { ...environment([]), nonInputNodeRects }, gap),
}
}

Expand All @@ -40,6 +42,10 @@ export interface Environment extends NonDictatedEnvironment {
selectedNodeRects: Iterable<Rect>
}

export interface InputNodeEnvironment extends Environment {
nonInputNodeRects: Iterable<Rect>
}

function themeGap(): Vec2 {
return new Vec2(theme.node.horizontal_gap, theme.node.vertical_gap)
}
Expand Down Expand Up @@ -119,6 +125,26 @@ export function mouseDictatedPlacement(
return mousePosition.add(new Vec2(nodeRadius, nodeRadius))
}

/** The new node should appear above non-input nodes, left aligned to the leftmost node and vertically aligned with the other input nodes. */
export function inputNodePlacement(
nodeSize: Vec2,
{ nonInputNodeRects, nodeRects }: InputNodeEnvironment,
gap = themeGap(),
): Vec2 {
let topMostY
let leftMostX
for (const rect of nonInputNodeRects) {
topMostY = topMostY == null ? rect.top : Math.min(topMostY, rect.top)
leftMostX = leftMostX == null ? rect.left : Math.min(leftMostX, rect.left)
}
assert(
topMostY != null && leftMostX != null,
'inputNodePlacement works only if at least one non-input node is present.',
)
const initialPosition = new Vec2(leftMostX, topMostY - nodeSize.y - gap.y)
return seekHorizontal(new Rect(initialPosition, nodeSize), nodeRects, gap)
}

/** The new node should appear at the average Y-position of selected nodes and with the X-position of the leftmost node.
*
* If the desired place is already occupied by non-selected node, it should be moved down to the closest free space.
Expand Down Expand Up @@ -155,7 +181,7 @@ export function collapsedNodePlacement(
return seekVertical(new Rect(initialPosition, nodeSize), nonSelectedNodeRects, gap)
}

/** Given a preferred location for a node, adjust the top as much as necessary for it not to collide with any of the
/** Given a preferred location for a node, adjust the top as low as necessary for it not to collide with any of the
* provided `otherRects`. */
export function seekVertical(preferredRect: Rect, otherRects: Iterable<Rect>, gap = themeGap()) {
const initialRect = orDefaultSize(preferredRect)
Expand Down
6 changes: 3 additions & 3 deletions app/gui2/src/components/GraphEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -620,21 +620,21 @@ function collapseNodes() {
}
const selectedNodeRects = filterDefined(Array.from(selected, graphStore.visibleArea))
graphStore.edit((edit) => {
const { refactoredExpressionAstId, collapsedNodeIds, outputNodeId } = performCollapse(
const { refactoredExpressionAstId, collapsedNodeIds, outputAstId } = performCollapse(
info.value,
edit.getVersion(topLevel),
graphStore.db,
currentMethodName,
)
const position = collapsedNodePlacement(selectedNodeRects)
edit.get(refactoredExpressionAstId).mutableNodeMetadata().set('position', position.xy())
if (outputNodeId != null) {
if (outputAstId != null) {
const collapsedNodeRects = filterDefined(
Array.from(collapsedNodeIds, graphStore.visibleArea),
)
const { place } = usePlacement(collapsedNodeRects, graphNavigator.viewport)
const position = place(collapsedNodeRects)
edit.get(refactoredExpressionAstId).mutableNodeMetadata().set('position', position.xy())
edit.get(outputAstId).mutableNodeMetadata().set('position', position.xy())
}
})
} catch (err) {
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/src/components/GraphEditor/GraphEdge.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const hoveredPort = computed(() => (mouseAnchor.value ? selection?.hoveredPort :
const isSuggestion = computed(() => 'suggestion' in props.edge && props.edge.suggestion)
const connectedSourceNode = computed(
() => props.edge.source && graph.db.getPatternExpressionNodeId(props.edge.source),
() => props.edge.source && graph.getSourceNodeId(props.edge.source),
)
const sourceNode = computed(() => {
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/src/components/GraphEditor/GraphEdges.vue
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function createEdge(source: AstId, target: PortId) {
const ident = graph.db.getOutputPortIdentifier(source)
if (ident == null) return
const sourceNode = graph.db.getPatternExpressionNodeId(source)
const sourceNode = graph.getSourceNodeId(source)
const targetNode = graph.getPortNodeId(target)
if (sourceNode == null || targetNode == null) {
return console.error(`Failed to connect edge, source or target node not found.`)
Expand Down
15 changes: 11 additions & 4 deletions app/gui2/src/components/GraphEditor/GraphNode.vue
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ const navigator = injectGraphNavigator(true)
const nodeId = computed(() => asNodeId(props.node.rootExpr.externalId))
const potentialSelfArgumentId = computed(() => props.node.primarySubject)
const nodePosition = computed(() => {
// Positions of nodes that are not yet placed are set to `Infinity`.
if (props.node.position.equals(Vec2.Infinity)) return Vec2.Zero
return props.node.position
})
onUnmounted(() => graph.unregisterNodeRect(nodeId.value))
const rootNode = ref<HTMLElement>()
Expand Down Expand Up @@ -187,7 +193,7 @@ watchEffect(() => {
}
const inZone = (pos: Vec2 | undefined) =>
pos != null &&
pos.sub(props.node.position).x <
pos.sub(nodePosition.value).x <
CONTENT_PADDING + ICON_WIDTH + GRAB_HANDLE_X_MARGIN_L + GRAB_HANDLE_X_MARGIN_R
const hovered =
menuHovered.value ||
Expand Down Expand Up @@ -234,7 +240,7 @@ watch(isVisualizationPreviewed, (newVal, oldVal) => {
})
const transform = computed(() => {
const { x, y } = props.node.position
const { x, y } = nodePosition.value
return `translate(${x}px, ${y}px)`
})
Expand Down Expand Up @@ -413,6 +419,7 @@ watchEffect(() => {
selected,
selectionVisible,
['executionState-' + executionState]: true,
inputNode: props.node.type === 'input',
outputNode: props.node.type === 'output',
menuVisible,
menuFull,
Expand All @@ -425,7 +432,7 @@ watchEffect(() => {
<Teleport v-if="navigator && !edited && graphNodeSelections" :to="graphNodeSelections">
<GraphNodeSelection
:data-node-id="nodeId"
:nodePosition="props.node.position"
:nodePosition="nodePosition"
:nodeSize="graphSelectionSize"
:class="{ draggable: true, dragged: isDragged }"
:selected
Expand Down Expand Up @@ -472,7 +479,7 @@ watchEffect(() => {
v-if="isVisualizationVisible"
:nodeSize="nodeSize"
:scale="navigator?.scale ?? 1"
:nodePosition="props.node.position"
:nodePosition="nodePosition"
:isCircularMenuVisible="menuVisible"
:currentType="props.node.vis?.identifier"
:dataSource="{ type: 'node', nodeId: props.node.rootExpr.externalId }"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,15 @@ main =
setupGraphDb(initialCode, graphDb)
const nodes = Array.from(graphDb.nodeIdToNode.keys())
const { extracted, refactored } = unwrap(
prepareCollapsedInfo(new Set(nodes.slice(1, 2)), graphDb),
prepareCollapsedInfo(new Set(nodes.slice(2, 3)), graphDb),
)
expect(extracted.ids).toEqual(new Set(nodes.slice(1, 2)))
expect(extracted.ids).toEqual(new Set(nodes.slice(2, 3)))
expect(extracted.inputs).toEqual(['input', 'four'])
expect(extracted.output).toEqual({
node: nodes[1],
node: nodes[2],
identifier: 'sum',
})
expect(refactored.id).toEqual(nodes[1])
expect(refactored.id).toEqual(nodes[2])
expect(refactored.pattern).toEqual('sum')
expect(refactored.arguments).toEqual(['input', 'four'])
})
Loading

0 comments on commit eea23b1

Please sign in to comment.