Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper fix for editing node #9153

Merged
merged 3 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions app/gui2/e2e/componentBrowser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,36 @@ test('Filtering list', async ({ page }) => {
const highlighted = locate.componentBrowserEntry(page).locator('.component-label-segment.match')
await expect(highlighted).toHaveText(['re', '_te'])
})

test('Editing existing nodes', async ({ page }) => {
await actions.goToGraph(page)
const node = locate.graphNodeByBinding(page, 'data')
const ADDED_PATH = '"/home/enso/Input.txt"'

// Start node editing
await locate.graphNodeIcon(node).click({ modifiers: ['Control'] })
await expect(locate.componentBrowser(page)).toBeVisible()
const input = locate.componentBrowserInput(page).locator('input')
await expect(input).toHaveValue('Data.read')

// Add argument and accept
await page.keyboard.press('End')
await input.pressSequentially(` ${ADDED_PATH}`)
await expect(input).toHaveValue(`Data.read ${ADDED_PATH}`)
await page.keyboard.press('Enter')
await expect(locate.componentBrowser(page)).not.toBeVisible()
await expect(node.locator('.WidgetToken')).toHaveText(['Data', '.', 'read'])
await expect(node.locator('.WidgetText input')).toHaveValue(ADDED_PATH)

// Edit again, using "edit" button
await locate.graphNodeIcon(node).click()
await node.locator('.CircularMenu .slot6').click()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That edit button locator could be moved to an utility as well, since the slot6 class is not providing any meaning.

await expect(locate.componentBrowser(page)).toBeVisible()
await expect(input).toHaveValue(`Data.read ${ADDED_PATH}`)
for (let i = 0; i < ADDED_PATH.length; ++i) await page.keyboard.press('Backspace')
await expect(input).toHaveValue('Data.read ')
await page.keyboard.press('Enter')
await expect(locate.componentBrowser(page)).not.toBeVisible()
await expect(node.locator('.WidgetToken')).toHaveText(['Data', '.', 'read'])
await expect(node.locator('.WidgetText')).not.toBeVisible()
})
21 changes: 17 additions & 4 deletions app/gui2/e2e/locate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,23 @@ export function exitFullscreenButton(page: Locator | Page) {

export const toggleFullscreenButton = or(enterFullscreenButton, exitFullscreenButton)

// === Nodes ===

declare const nodeLocatorBrand: unique symbol
type Node = Locator & { [nodeLocatorBrand]: never }

export function graphNode(page: Page | Locator): Node {
return page.locator('.GraphNode') as Node
}
export function graphNodeByBinding(page: Locator | Page, binding: string): Node {
return graphNode(page).filter({
has: page.locator('.binding').and(page.getByText(binding)),
}) as Node
}
export function graphNodeIcon(node: Node) {
return node.locator('.icon')
}

// === Data locators ===

type SanitizeClassName<T extends string> = T extends `${infer A}.${infer B}`
Expand All @@ -128,10 +145,6 @@ function componentLocator<T extends string>(className: SanitizeClassName<T>) {
}

export const graphEditor = componentLocator('GraphEditor')
export const graphNode = componentLocator('GraphNode')
export function graphNodeByBinding(page: Locator | Page, binding: string) {
return graphNode(page).filter({ has: page.locator('.binding').and(page.getByText(binding)) })
}
// @ts-expect-error
export const anyVisualization = componentLocator('GraphVisualization > *')
export const circularMenu = componentLocator('CircularMenu')
Expand Down
4 changes: 2 additions & 2 deletions app/gui2/src/components/GraphEditor/GraphNode.vue
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ const dragPointer = usePointer((pos, event, type) => {
startEvent != null &&
pos.absolute.distanceSquared(startPos) <= MAXIMUM_CLICK_DISTANCE_SQ
) {
nodeSelection?.handleSelectionOf(startEvent, new Set([nodeId.value]))
nodeSelection?.handleSelectionOf(event, new Set([nodeId.value]))
handleNodeClick(event)
menuVisible.value = MenuState.Partial
}
Expand Down Expand Up @@ -256,7 +256,7 @@ const nodeEditHandler = nodeEditBindings.handler({
})

function startEditingNode(position: Vec2 | undefined) {
let sourceOffset = 0
let sourceOffset = props.node.rootSpan.code().length
if (position != null) {
let domNode, domOffset
if ((document as any).caretPositionFromPoint) {
Expand Down
29 changes: 23 additions & 6 deletions app/gui2/src/util/shortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ function modifierFlagsForEvent(event: EventWithModifiers): ModifierFlags {
(event.metaKey ? RAW_MODIFIER_FLAG.Meta : 0)) as ModifierFlags
}

/** These values MUST match the flags on `MouseEvent#button`.
/** These values MUST match the flags on `MouseEvent#buttons`.
* See https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons */
const POINTER_BUTTON_FLAG = {
PointerMain: 1 << 0,
Expand All @@ -57,6 +57,27 @@ const POINTER_BUTTON_FLAG = {
PointerForward: 1 << 4,
} satisfies Record<Pointer, number> as Record<Pointer, PointerButtonFlags>

/**
* Mapping from the MouseEvent's `button` field to PointerButtonFlags.
*
* No, it is not as simple as (1 << event.button) as PointerButtonFlags; compare
* https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons with
* https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button
*/
const flagsOfButtonField = [
POINTER_BUTTON_FLAG.PointerMain,
POINTER_BUTTON_FLAG.PointerAux,
POINTER_BUTTON_FLAG.PointerSecondary,
POINTER_BUTTON_FLAG.PointerBack,
POINTER_BUTTON_FLAG.PointerForward,
]

function buttonFlagsForEvent(event: MouseEvent): PointerButtonFlags {
// event.buttons keeps information about buttons being pressed, but in case of `click` or
// `pointerup` events we also want to know what buttons were just released.
return (event.buttons | (flagsOfButtonField[event.button] ?? 0)) as PointerButtonFlags
}

// ==========================
// === Autocomplete types ===
// ==========================
Expand Down Expand Up @@ -333,11 +354,7 @@ export function defineKeybinds<
const keybinds =
event instanceof KeyboardEvent
? keyboardShortcuts[event.key.toLowerCase() as Key_]?.[eventModifierFlags]
: mouseShortcuts[
(event.type === 'click' // `click` events don't have 'buttons' field initialized.
? POINTER_BUTTON_FLAG.PointerMain
: event.buttons) as PointerButtonFlags
]?.[eventModifierFlags]
: mouseShortcuts[buttonFlagsForEvent(event)]?.[eventModifierFlags]
let handle = handlers[DefaultHandler]
if (keybinds != null) {
for (const bindingName in handlers) {
Expand Down
Loading