Skip to content
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
81 changes: 68 additions & 13 deletions src/renderer/extensions/vueNodes/components/LGraphNode.vue
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,15 @@
</div>
</template>

<!-- Resize handle -->
<div
v-if="!isCollapsed"
class="absolute right-0 bottom-0 h-3 w-3 cursor-se-resize opacity-0 transition-opacity duration-200 hover:bg-white hover:opacity-20"
@pointerdown.stop="startResize"
/>
<!-- Resize handles -->
<template v-if="!isCollapsed">
<div
v-for="handle in cornerResizeHandles"
:key="handle.id"
:class="cn(baseResizeHandleClasses, handle.classes)"
@pointerdown.stop="handleResizePointerDown(handle.direction)($event)"
Copy link

Choose a reason for hiding this comment

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

[quality] medium Priority

Issue: Resize handles missing accessibility attributes
Context: Resize handles lack aria-label or role attributes for screen reader users
Suggestion: Add aria-label describing the resize direction (e.g., "Resize from bottom-right corner")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added role="button" and localized aria-label in b54e919

Copy link
Contributor

Choose a reason for hiding this comment

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

I hate that they still haven't added a role for handles like this :-(

/>
</template>
</div>
</template>

Expand Down Expand Up @@ -147,7 +150,8 @@ import {
} from '@/utils/graphTraversalUtil'
import { cn } from '@/utils/tailwindUtil'

import { useNodeResize } from '../composables/useNodeResize'
import type { ResizeHandleDirection } from '../interactions/resize/resizeMath'
import { useNodeResize } from '../interactions/resize/useNodeResize'
import { calculateIntrinsicSize } from '../utils/calculateIntrinsicSize'
import LivePreview from './LivePreview.vue'
import NodeContent from './NodeContent.vue'
Expand Down Expand Up @@ -243,8 +247,12 @@ onErrorCaptured((error) => {
return false // Prevent error propagation
})

// Use layout system for node position and dragging
const { position, size, zIndex } = useNodeLayout(() => nodeData.id)
const {
position,
size,
zIndex,
moveTo: moveNodeTo
} = useNodeLayout(() => nodeData.id)
const { pointerHandlers, isDragging, dragStyle } = useNodePointerInteractions(
() => nodeData,
handleNodeSelect
Expand Down Expand Up @@ -282,19 +290,66 @@ onMounted(() => {
}
})

const baseResizeHandleClasses =
'absolute h-3 w-3 opacity-0 pointer-events-auto focus-visible:outline focus-visible:outline-2 focus-visible:outline-white/40'
Copy link

Choose a reason for hiding this comment

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

[performance] low Priority

Issue: Resize handles always rendered but invisible by default
Context: All 4 handles are rendered in DOM with opacity-0, which creates unnecessary DOM nodes
Suggestion: Consider using v-show or CSS-only hover reveal to avoid DOM overhead when not needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented in 68255c9 (removed in feda4d9 to keep handles invisible)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed (handles remain invisible with cursor-only indication)

const POSITION_EPSILON = 0.01
Copy link

Choose a reason for hiding this comment

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

[quality] low Priority

Issue: Magic number POSITION_EPSILON without explanation
Context: Value 0.01 is used to check position changes but lacks documentation about why this threshold was chosen
Suggestion: Add comment explaining the epsilon value rationale or extract to a named constant with documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skipping - unclear rationale for the threshold


const cornerResizeHandles = [
{
id: 'se',
direction: { horizontal: 'right', vertical: 'bottom' } as const,
classes: 'right-0 bottom-0 cursor-se-resize'
},
{
id: 'ne',
direction: { horizontal: 'right', vertical: 'top' } as const,
classes: 'right-0 top-0 cursor-ne-resize'
},
{
id: 'sw',
direction: { horizontal: 'left', vertical: 'bottom' } as const,
classes: 'left-0 bottom-0 cursor-sw-resize'
},
{
id: 'nw',
direction: { horizontal: 'left', vertical: 'top' } as const,
classes: 'left-0 top-0 cursor-nw-resize'
}
] satisfies Array<{
id: string
direction: ResizeHandleDirection
classes: string
}>

const { startResize } = useNodeResize(
(newSize, element) => {
// Apply size directly to DOM element - ResizeObserver will pick this up
(result, element) => {
if (isCollapsed.value) return

element.style.width = `${newSize.width}px`
element.style.height = `${newSize.height}px`
// Apply size directly to DOM element - ResizeObserver will pick this up
element.style.width = `${result.size.width}px`
element.style.height = `${result.size.height}px`

const currentPosition = position.value
const deltaX = Math.abs(result.position.x - currentPosition.x)
const deltaY = Math.abs(result.position.y - currentPosition.y)

if (deltaX > POSITION_EPSILON || deltaY > POSITION_EPSILON) {
moveNodeTo(result.position)
}
},
{
transformState
}
)

const handleResizePointerDown = (direction: ResizeHandleDirection) => {
return (event: PointerEvent) => {
if (nodeData.flags?.pinned) return

startResize(event, direction, { ...position.value })
}
}

whenever(isCollapsed, () => {
const element = nodeContainerRef.value
if (!element) return
Expand Down
132 changes: 132 additions & 0 deletions src/renderer/extensions/vueNodes/interactions/resize/resizeMath.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import { clamp } from 'es-toolkit/compat'

import type { Point, Size } from '@/renderer/core/layout/types'

export type ResizeHandleDirection = {
horizontal: 'left' | 'right'
vertical: 'top' | 'bottom'
}

function applyHandleDelta(
startSize: Size,
delta: Point,
handle: ResizeHandleDirection
): Size {
const horizontalFactor = handle.horizontal === 'right' ? 1 : -1
Copy link

Choose a reason for hiding this comment

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

[architecture] medium Priority

Issue: Complex resize math could benefit from more descriptive variable names
Context: Variables like horizontalFactor and verticalFactor are clear but could be more semantic
Suggestion: Consider renaming to directionMultiplierX/Y or leftRightFactor/topBottomFactor for better readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to horizontalMultiplier/verticalMultiplier in fc4a43d

const verticalFactor = handle.vertical === 'bottom' ? 1 : -1

return {
width: startSize.width + delta.x * horizontalFactor,
height: startSize.height + delta.y * verticalFactor
}
}

function clampToMinSize(size: Size, minSize: Size): Size {
return {
width: clamp(size.width, minSize.width, Number.POSITIVE_INFINITY),
height: clamp(size.height, minSize.height, Number.POSITIVE_INFINITY)
}
}

function snapSize(
size: Size,
minSize: Size,
snapFn?: (size: Size) => Size
): Size {
if (!snapFn) return size
const snapped = snapFn(size)
return {
width: Math.max(minSize.width, snapped.width),
height: Math.max(minSize.height, snapped.height)
}
}

function computeAdjustedPosition(
startPosition: Point,
startSize: Size,
nextSize: Size,
handle: ResizeHandleDirection
): Point {
const widthDelta = startSize.width - nextSize.width
const heightDelta = startSize.height - nextSize.height

return {
x:
handle.horizontal === 'left'
? startPosition.x + widthDelta
: startPosition.x,
y:
handle.vertical === 'top'
? startPosition.y + heightDelta
: startPosition.y
}
}

/**
* Computes the resulting size and position of a node given pointer movement
* and handle orientation.
*/
export function computeResizeOutcome({
startSize,
startPosition,
delta,
minSize,
handle,
snapFn
}: {
startSize: Size
startPosition: Point
delta: Point
minSize: Size
handle: ResizeHandleDirection
snapFn?: (size: Size) => Size
}): { size: Size; position: Point } {
const resized = applyHandleDelta(startSize, delta, handle)
const clamped = clampToMinSize(resized, minSize)
const snapped = snapSize(clamped, minSize, snapFn)
const position = computeAdjustedPosition(
startPosition,
startSize,
snapped,
handle
)

return {
size: snapped,
position
}
}

export function createResizeSession(config: {
startSize: Size
startPosition: Point
minSize: Size
handle: ResizeHandleDirection
}) {
const startSize = { ...config.startSize }
const startPosition = { ...config.startPosition }
const minSize = { ...config.minSize }
const handle = config.handle

return (delta: Point, snapFn?: (size: Size) => Size) =>
computeResizeOutcome({
startSize,
startPosition,
minSize,
handle,
delta,
snapFn
})
}

export function toCanvasDelta(
startPointer: Point,
currentPointer: Point,
scale: number
): Point {
const safeScale = scale === 0 ? 1 : scale
Copy link

Choose a reason for hiding this comment

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

[quality] low Priority

Issue: Redundant division by zero protection
Context: toCanvasDelta already protects against scale=0 with safeScale variable
Suggestion: This protection is good, but consider extracting as a utility function since scale division occurs in multiple places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skipping - only used once, extraction wouldn't add value

return {
x: (currentPointer.x - startPointer.x) / safeScale,
y: (currentPointer.y - startPointer.y) / safeScale
}
}
Loading