Skip to content

Commit

Permalink
Self-argument access chain ports are conditional (#9439)
Browse files Browse the repository at this point in the history
Ports for `PropertyAccess` AST nodes that (transitively) contain a `WidgetSelfArgument` are not targets for a disconnected edge, unless the mod key is held.

https://github.com/enso-org/enso/assets/1047859/68ac1953-c8b1-4e51-8c4c-211595e0c034

(Video shows with and without mod key)
  • Loading branch information
kazcw authored Mar 18, 2024
1 parent 47d9e73 commit e1893b6
Show file tree
Hide file tree
Showing 27 changed files with 331 additions and 102 deletions.
26 changes: 6 additions & 20 deletions app/gui2/e2e/componentBrowser.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { test, type Page } from '@playwright/test'
import assert from 'assert'
import os from 'os'
import * as actions from './actions'
import { expect } from './customExpect'
Expand Down Expand Up @@ -47,23 +46,16 @@ test('Different ways of opening Component Browser', async ({ page }) => {
await locate.graphEditor(page).press('Enter')
await expectAndCancelBrowser(page, 'final.')
// Dragging out an edge
// `click` method of locator could be simpler, but `position` option doesn't work.
const outputPortArea = await locate
.graphNodeByBinding(page, 'final')
.locator('.outputPortHoverArea')
.boundingBox()
assert(outputPortArea)
const outputPortX = outputPortArea.x + outputPortArea.width / 2.0
const outputPortY = outputPortArea.y + outputPortArea.height - 2.0
await page.mouse.click(outputPortX, outputPortY)
const outputPort = await locate.outputPortCoordinates(locate.graphNodeByBinding(page, 'final'))
await page.mouse.click(outputPort.x, outputPort.y)
await page.mouse.click(100, 500)
await expectAndCancelBrowser(page, 'final.')
// 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)
await page.mouse.click(outputPortX, outputPortY)
await page.mouse.click(outputPortX, outputPortY)
await page.mouse.click(outputPort.x, outputPort.y)
await page.mouse.click(outputPort.x, outputPort.y)
await expectAndCancelBrowser(page, 'final.')
})

Expand Down Expand Up @@ -108,14 +100,8 @@ test('Graph Editor pans to Component Browser', async ({ page }) => {
await page.mouse.move(100, 80)
await page.mouse.up({ button: 'middle' })
await expect(locate.graphNodeByBinding(page, 'five')).toBeInViewport()
const outputPortArea = await locate
.graphNodeByBinding(page, 'final')
.locator('.outputPortHoverArea')
.boundingBox()
assert(outputPortArea)
const outputPortX = outputPortArea.x + outputPortArea.width / 2.0
const outputPortY = outputPortArea.y + outputPortArea.height - 2.0
await page.mouse.click(outputPortX, outputPortY)
const outputPort = await locate.outputPortCoordinates(locate.graphNodeByBinding(page, 'final'))
await page.mouse.click(outputPort.x, outputPort.y)
await page.mouse.click(100, 1550)
await expect(locate.graphNodeByBinding(page, 'five')).not.toBeInViewport()
await expectAndCancelBrowser(page, 'final.')
Expand Down
46 changes: 44 additions & 2 deletions app/gui2/e2e/edgeInteractions.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect, test, type Page } from '@playwright/test'
import { test, type Page } from '@playwright/test'
import * as actions from './actions'
import { edgesToNodeWithBinding, graphNodeByBinding } from './locate'
import { expect } from './customExpect'
import * as locate from './locate'
import { edgesToNodeWithBinding, graphNodeByBinding, outputPortCoordinates } from './locate'

/**
* Prepare the graph for the tests. We drag the `ten` node to the right of the `sum` node for better access
Expand Down Expand Up @@ -53,3 +55,43 @@ test('Connect an node to a port via dragging the edge', async ({ page }) => {

await expect(graphNodeByBinding(page, 'prod')).toContainText('ten')
})

test('Conditional ports: Disabled', async ({ page }) => {
await actions.goToGraph(page)
const node = graphNodeByBinding(page, 'filtered')
const conditionalPort = node.locator('.WidgetPort').filter({ hasText: /^filter$/ })

// Check that the `enabled` CSS class is not set on disabled `WidgetPort`s.
await expect(node.locator('.WidgetSelfIcon')).toBeVisible()
await expect(conditionalPort).not.toHaveClass(/enabled/)

// 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'))
await page.mouse.click(outputPort.x, outputPort.y)
await conditionalPort.hover()
await expect(conditionalPort).not.toHaveClass(/isTarget/)
await conditionalPort.click()
await expect(locate.componentBrowser(page)).toExist()
await page.keyboard.press('Escape')
})

test('Conditional ports: Enabled', async ({ page }) => {
await actions.goToGraph(page)
const node = graphNodeByBinding(page, 'filtered')
const conditionalPort = node.locator('.WidgetPort').filter({ hasText: /^filter$/ })

await page.keyboard.down('Meta')
await page.keyboard.down('Control')

await expect(conditionalPort).toHaveClass(/enabled/)
const outputPort = await outputPortCoordinates(graphNodeByBinding(page, 'final'))
await page.mouse.click(outputPort.x, outputPort.y)
await conditionalPort.hover()
await expect(conditionalPort).toHaveClass(/isTarget/)
await conditionalPort.click()
await expect(node.locator('.WidgetToken')).toHaveText(['final'])

await page.keyboard.up('Meta')
await page.keyboard.up('Control')
})
17 changes: 16 additions & 1 deletion app/gui2/e2e/locate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { type Locator, type Page } from '@playwright/test'
import { expect, type Locator, type Page } from '@playwright/test'
import assert from 'assert'
import cssEscape from 'css.escape'

// ==============
Expand Down Expand Up @@ -199,3 +200,17 @@ export async function edgesToNodeWithBinding(page: Page, binding: string) {
const nodeId = await node.getAttribute('data-node-id')
return page.locator(`[data-target-node-id="${nodeId}"]`)
}

// === Output ports ===

/** 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()
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 }
}
12 changes: 12 additions & 0 deletions app/gui2/shared/ast/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,18 @@ export interface MutablePropertyAccess extends PropertyAccess, MutableAst {
}
applyMixins(MutablePropertyAccess, [MutableAst])

/** Unroll the provided chain of `PropertyAccess` nodes, returning the first non-access as `subject` and the accesses
* from left-to-right. */
export function accessChain(ast: Ast): { subject: Ast; accessChain: PropertyAccess[] } {
const accessChain = new Array<PropertyAccess>()
while (ast instanceof PropertyAccess && ast.lhs) {
accessChain.push(ast)
ast = ast.lhs
}
accessChain.reverse()
return { subject: ast, accessChain }
}

interface GenericFields {
children: RawNodeChild[]
}
Expand Down
1 change: 0 additions & 1 deletion app/gui2/src/bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ export const codeEditorBindings = defineKeybinds('code-editor', {

export const interactionBindings = defineKeybinds('current-interaction', {
cancel: ['Escape'],
click: ['PointerMain'],
})

export const componentBrowserBindings = defineKeybinds('component-browser', {
Expand Down
22 changes: 15 additions & 7 deletions app/gui2/src/components/GraphEditor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { useStackNavigator } from '@/composables/stackNavigator'
import { provideGraphNavigator } from '@/providers/graphNavigator'
import { provideGraphSelection } from '@/providers/graphSelection'
import { provideInteractionHandler } from '@/providers/interactionHandler'
import { provideKeyboard } from '@/providers/keyboard'
import { provideWidgetRegistry } from '@/providers/widgetRegistry'
import { useGraphStore, type NodeId } from '@/stores/graph'
import type { RequiredImport } from '@/stores/graph/imports'
Expand All @@ -44,8 +45,9 @@ import { type Usage } from './ComponentBrowser/input'
const DEFAULT_NODE_SIZE = new Vec2(0, 24)
const gapBetweenNodes = 48.0
const keyboard = provideKeyboard()
const viewportNode = ref<HTMLElement>()
const graphNavigator = provideGraphNavigator(viewportNode)
const graphNavigator = provideGraphNavigator(viewportNode, keyboard)
const graphStore = useGraphStore()
const widgetRegistry = provideWidgetRegistry(graphStore.db)
widgetRegistry.loadBuiltins()
Expand Down Expand Up @@ -83,15 +85,19 @@ projectStore.executionContext.on('executionFailed', (e) =>
// === nodes ===
const nodeSelection = provideGraphSelection(graphNavigator, graphStore.nodeRects, {
onSelected(id) {
graphStore.db.moveNodeToTop(id)
const nodeSelection = provideGraphSelection(
graphNavigator,
graphStore.nodeRects,
graphStore.isPortEnabled,
{
onSelected(id) {
graphStore.db.moveNodeToTop(id)
},
},
})
)
const interactionBindingsHandler = interactionBindings.handler({
cancel: () => interaction.handleCancel(),
click: (e) => (e instanceof PointerEvent ? interaction.handleClick(e, graphNavigator) : false),
})
// Return the environment for the placement of a new node. The passed nodes should be the nodes that are
Expand Down Expand Up @@ -147,7 +153,9 @@ useEvent(window, 'keydown', (event) => {
(!keyboardBusy() && graphBindingsHandler(event)) ||
(!keyboardBusyExceptIn(codeEditorArea.value) && codeEditorHandler(event))
})
useEvent(window, 'pointerdown', interactionBindingsHandler, { capture: true })
useEvent(window, 'pointerdown', (e) => interaction.handleClick(e, graphNavigator), {
capture: true,
})
onMounted(() => viewportNode.value?.focus())
Expand Down
1 change: 1 addition & 0 deletions app/gui2/src/components/GraphEditor/GraphNode.vue
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ const documentation = computed<string | undefined>({
:icon="icon"
:connectedSelfArgumentId="connectedSelfArgumentId"
:potentialSelfArgumentId="potentialSelfArgumentId"
:conditionalPorts="props.node.conditionalPorts"
:extended="isOnlyOneSelected"
@openFullMenu="openFullMenu"
/>
Expand Down
3 changes: 3 additions & 0 deletions app/gui2/src/components/GraphEditor/NodeWidgetTree.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const props = defineProps<{
icon: Icon
connectedSelfArgumentId: Ast.AstId | undefined
potentialSelfArgumentId: Ast.AstId | undefined
/** Ports that are not targetable by default; see {@link NodeDataFromAst}. */
conditionalPorts: Set<Ast.AstId>
extended: boolean
}>()
const emit = defineEmits<{
Expand Down Expand Up @@ -92,6 +94,7 @@ provideWidgetTree(
toRef(props, 'icon'),
toRef(props, 'connectedSelfArgumentId'),
toRef(props, 'potentialSelfArgumentId'),
toRef(props, 'conditionalPorts'),
toRef(props, 'extended'),
layoutTransitions.active,
() => {
Expand Down
36 changes: 26 additions & 10 deletions app/gui2/src/components/GraphEditor/widgets/WidgetPort.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useRaf } from '@/composables/animation'
import { useResizeObserver } from '@/composables/events'
import { injectGraphNavigator } from '@/providers/graphNavigator'
import { injectGraphSelection } from '@/providers/graphSelection'
import { injectKeyboard } from '@/providers/keyboard'
import { injectPortInfo, providePortInfo, type PortId } from '@/providers/portInfo'
import { Score, WidgetInput, defineWidget, widgetProps } from '@/providers/widgetRegistry'
import { injectWidgetTree } from '@/providers/widgetTree'
Expand Down Expand Up @@ -88,10 +89,6 @@ const innerWidget = computed(() => {
providePortInfo(proxyRefs({ portId, connected: hasConnection }))
watch(nodeSize, updateRect)
onUpdated(() => nextTick(updateRect))
useRaf(toRef(tree, 'hasActiveAnimations'), updateRect)
const randSlice = randomUuid.slice(0, 4)
watchEffect(
Expand All @@ -104,19 +101,37 @@ watchEffect(
{ flush: 'post' },
)
function updateRect() {
let domNode = rootNode.value
const keyboard = injectKeyboard()
const enabled = computed(() => {
const input = props.input.value
const isConditional = input instanceof Ast.Ast && tree.conditionalPorts.has(input.id)
return !isConditional || keyboard.mod
})
const computedRect = computed(() => {
const domNode = rootNode.value
const rootDomNode = domNode?.closest('.GraphNode')
if (domNode == null || rootDomNode == null) return
if (!enabled.value) return
let _nodeSizeEffect = nodeSize.value
const exprClientRect = Rect.FromDomRect(domNode.getBoundingClientRect())
const nodeClientRect = Rect.FromDomRect(rootDomNode.getBoundingClientRect())
const exprSceneRect = navigator.clientToSceneRect(exprClientRect)
const exprNodeRect = navigator.clientToSceneRect(nodeClientRect)
const localRect = exprSceneRect.offsetBy(exprNodeRect.pos.inverse())
if (portRect.value != null && localRect.equals(portRect.value)) return
portRect.value = localRect
return exprSceneRect.offsetBy(exprNodeRect.pos.inverse())
})
function updateRect() {
const newRect = computedRect.value
if (!Rect.Equal(portRect.value, newRect)) {
portRect.value = newRect
}
}
watch(computedRect, updateRect)
onUpdated(() => nextTick(updateRect))
useRaf(toRef(tree, 'hasActiveAnimations'), updateRect)
</script>

<script lang="ts">
Expand Down Expand Up @@ -159,6 +174,7 @@ export const widgetDefinition = defineWidget(WidgetInput.isAstOrPlaceholder, {
ref="rootNode"
class="WidgetPort"
:class="{
enabled,
connected,
isTarget,
isSelfArgument,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const widgetDefinition = defineWidget(WidgetInput.isAst, {

<template>
<SvgIcon
class="icon nodeCategoryIcon"
class="WidgetSelfIcon icon nodeCategoryIcon"
:name="icon"
@click.right.stop.prevent="tree.emitOpenFullMenu()"
/>
Expand Down
3 changes: 2 additions & 1 deletion app/gui2/src/composables/__tests__/selection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ function selectionWithMockData(sceneMousePos?: Ref<Vec2>) {
rects.set(3, Rect.FromBounds(1, 20, 10, 30))
rects.set(4, Rect.FromBounds(20, 20, 30, 30))
const navigator = proxyRefs({ sceneMousePos: sceneMousePos ?? ref(Vec2.Zero), scale: 1 })
const selection = useSelection(navigator, rects, 0)
const allPortsEnabled = () => true
const selection = useSelection(navigator, rects, allPortsEnabled, 0)
selection.setSelection(new Set([1, 2]))
return selection
}
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/src/composables/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ const hasWindow = typeof window !== 'undefined'
const platform = hasWindow ? window.navigator?.platform ?? '' : ''
export const isMacLike = /(Mac|iPhone|iPod|iPad)/i.test(platform)

export function modKey(e: KeyboardEvent): boolean {
export function modKey(e: KeyboardEvent | MouseEvent): boolean {
return isMacLike ? e.metaKey : e.ctrlKey
}

Expand Down
31 changes: 31 additions & 0 deletions app/gui2/src/composables/keyboard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { isMacLike, useEvent } from '@/composables/events'
import { proxyRefs, ref } from 'vue'

export type KeyboardComposable = ReturnType<typeof useKeyboard>
export function useKeyboard() {
const state = {
alt: ref(false),
shift: ref(false),
meta: ref(false),
ctrl: ref(false),
}

const updateState = (e: MouseEvent | KeyboardEvent) => {
state.alt.value = e.altKey
state.shift.value = e.shiftKey
state.meta.value = e.metaKey
state.ctrl.value = e.ctrlKey
return false
}
useEvent(window, 'keydown', updateState, { capture: true })
useEvent(window, 'keyup', updateState, { capture: true })
useEvent(window, 'pointerenter', updateState, { capture: true })

return proxyRefs({
alt: state.alt,
shift: state.shift,
meta: state.meta,
ctrl: state.ctrl,
mod: isMacLike ? state.meta : state.ctrl,
})
}
Loading

0 comments on commit e1893b6

Please sign in to comment.