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.

Sign up for GitHub

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

Hudson/drag n drop cleanup #116

Merged
merged 4 commits into from
Sep 30, 2024
Merged

Hudson/drag n drop cleanup #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7795504
Change drag-and-drop to pointer events
hudbuddy Sep 30, 2024
85c8c9f
Add preview element while dragging
hudbuddy Sep 30, 2024
cc103dc
Change pointer global listener to document
hudbuddy Sep 30, 2024
5ad4ef7
Increment version
hudbuddy Sep 30, 2024
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
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@api.stream/studio-kit",
"version": "3.0.26",
"version": "3.0.27",
"description": "Client SDK for building studio experiences with API.stream",
"license": "MIT",
"private": false,
Expand Down
263 changes: 175 additions & 88 deletions src/helpers/compositor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,13 @@ class ErrorBoundary extends React.Component<
const onDrop = async (
data: {
dropNodeId: string
dragNodeId: string
dropType: 'layout' | 'transform'
project: InternalProject
},
e: React.DragEvent,
e: PointerEvent,
) => {
e.preventDefault()
e.stopPropagation()
const { dropNodeId, dropType, project } = data
const dragNodeId = e.dataTransfer.getData('text/plain')
const { dropNodeId, dragNodeId, dropType, project } = data
log.debug('Compositor: Dropping', { dropType, dragNodeId, dropNodeId })

if (dropNodeId === dragNodeId) return
Expand Down Expand Up @@ -117,9 +115,13 @@ const onDrop = async (
})
}

let foundDropTarget = false
let draggingNodeIdRef: string | null = null

const DRAG_DISTANCE_BUFFER = 2

const ElementTree = (props: { nodeId: string }) => {
const isDragging = useRef(false)
const activePointerCoords = useRef<{ x: number; y: number } | undefined>()
const isDraggingSelf = useRef(false)
const interactiveRef = useRef<HTMLDivElement>()
const transformRef = useRef<HTMLDivElement>()
const rootRef = useRef<HTMLDivElement>()
Expand Down Expand Up @@ -156,108 +158,161 @@ const ElementTree = (props: { nodeId: string }) => {

let isDraggingChild = node.children.some((x) => x.id === draggingNodeId)

let layoutDragHandlers = isDropTarget
? ({
onDrop: (e: React.DragEvent) => {
foundDropTarget = true
return onDrop(
{
dropType: 'layout',
dropNodeId: node.id,
project,
},
e,
)
},
onDragEnter: (e: React.DragEvent) => {
e.preventDefault()
e.stopPropagation()
rootRef.current?.toggleAttribute(
'data-layout-drop-target-active',
true,
)
},
onDragLeave: (e: React.DragEvent) => {
e.preventDefault()
e.stopPropagation()
rootRef.current?.toggleAttribute(
'data-layout-drop-target-active',
false,
)
},
} as React.HTMLAttributes<HTMLDivElement>)
: {}
let layoutDragHandlers =
isDropTarget && draggingNodeId && !isDraggingSelf.current
? ({
onpointerup: (e) => {
log.debug('Compositor: PointerUp "Layout"', node.id)
return onDrop(
{
dropType: 'layout',
dropNodeId: node.id,
dragNodeId: draggingNodeId,
project,
},
e,
)
},
onpointerenter: (e) => {
e.preventDefault()
e.stopPropagation()
rootRef.current?.toggleAttribute(
'data-layout-drop-target-active',
true,
)
},
onpointerleave: (e) => {
e.preventDefault()
e.stopPropagation()
rootRef.current?.toggleAttribute(
'data-layout-drop-target-active',
false,
)
},
} as Partial<HTMLElement>)
: {}

let transformDragHandlers = isDragTarget
? ({
draggable: true,
// If a target is draggable, it will also be treated as
// a drop target (swap element positions)
ondrop: (e) => {
foundDropTarget = true
return onDrop(
{
dropType: 'transform',
dropNodeId: node.id,
project,
},
// @ts-ignore TODO: Convert all to native drag events
e,
)
},
ondragstart: (e) => {
isDragging.current = true
wrapperEl.toggleAttribute('data-dragging', true)
setDraggingNodeId(node.id)
onpointerdown: (e) => {
// Check for pointer distance move > 1px before treating as a drag
activePointerCoords.current = { x: e.clientX, y: e.clientY }
let latestPointerCoords = activePointerCoords.current

log.debug('Compositor: Dragging', node.id)
foundDropTarget = false
e.dataTransfer.setData('text/plain', node.id)
e.dataTransfer.dropEffect = 'move'
e.dataTransfer.setDragImage(dragImage, 10, 10)
wrapperEl.toggleAttribute('data-dragging', true)
rootRef.current?.toggleAttribute('data-drag-target-active', true)
},
ondragend: (e) => {
isDragging.current = false
setDraggingNodeId(null)
wrapperEl.toggleAttribute('data-dragging', false)
log.debug('Compositor: DragEnd', e)
rootRef.current?.toggleAttribute('data-drag-target-active', false)
wrapperEl.toggleAttribute('data-drop-target-ready', false)

wrapperEl.querySelectorAll('[data-item]').forEach((x) => {
x.toggleAttribute('data-drag-target-active', false)
x.toggleAttribute('data-layout-drop-target-active', false)
x.toggleAttribute('data-transform-drop-target-active', false)
x.toggleAttribute('data-transform-drop-self-active', false)
})
const adjustPreviewPosition = () => {
if (!isDraggingSelf.current) return

// Runs every frame while dragging
dragPreviewEl.style.top = latestPointerCoords.y - 20 + 'px'
dragPreviewEl.style.left = latestPointerCoords.x - 20 + 'px'
dragPreviewEl.toggleAttribute('data-active', true)

requestAnimationFrame(() => adjustPreviewPosition())
}

const onGlobalPointerMove = (e: PointerEvent) => {
if (
!isDraggingSelf.current &&
Boolean(activePointerCoords.current)
) {
const isDragging = Boolean(
Math.abs(e.clientX - activePointerCoords.current.x) >=
DRAG_DISTANCE_BUFFER ||
Math.abs(e.clientY - activePointerCoords.current.y) >=
DRAG_DISTANCE_BUFFER,
)

// Runs only once when dragging starts
if (isDragging) {
setDraggingNodeId(node.id)
isDraggingSelf.current = isDragging
adjustPreviewPosition()
}
}
latestPointerCoords = { x: e.clientX, y: e.clientY }
}

onGlobalPointerMove(e)

const onGlobalPointerUp = (e: PointerEvent) => {
setTimeout(() => {
log.debug('Compositor: DragEnd', e)
activePointerCoords.current = undefined
isDraggingSelf.current = false
setDraggingNodeId(null)

dragPreviewEl.toggleAttribute('data-active', false)
wrapperEl.toggleAttribute('data-dragging', false)
wrapperEl.toggleAttribute('data-drop-target-ready', false)
wrapperEl.querySelectorAll('[data-item]').forEach((x) => {
x.toggleAttribute('data-drag-target-active', false)
x.toggleAttribute('data-layout-drop-target-active', false)
x.toggleAttribute('data-transform-drop-target-active', false)
x.toggleAttribute('data-transform-drop-self-active', false)
})
})

document.removeEventListener('pointermove', onGlobalPointerMove)
document.removeEventListener('pointerup', onGlobalPointerUp)
}

document.addEventListener('pointermove', onGlobalPointerMove)
document.addEventListener('pointerup', onGlobalPointerUp)
Comment on lines +241 to +264
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just something to consider, but I’m concerned we might risk running into memory leaks if we don’t wrap the layout and transform drag handlers in memoization. I know it’s tricky, but it could also make it easier to track changes across render cycles. Right now, it seems harder to know what lingering references we might not be cleaning up, which could lead to a memory leak. Again, this is totally up to you; I’m not enforcing anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree with your concern. truth is, there's so much inside of this file that I'd like to rework, I forced myself to keep everything as much the same as I could for this PR, since it works and is tested.

Currently it's a pretty haphazard combination of React vs native JS, so I am particularly worried about unexpected behavior if we begin to rely on the actual React lifecycle (I've had to lift some variables out of the component scope as a workaround, so component renders are not firing reliably).

I'll keep a close eye on performance and make a ticket for optimizations if needed. Regardless, I think we need to plan for some cleanup effort the next time changes are needed in this file.

},
ondragenter: (e) => {
onpointerup: (e) => {
// If a target is draggable, it will also be treated as
// a drop target (swap element positions)
log.debug('Compositor: PointerUp "Node"', node.id)
if (draggingNodeIdRef && draggingNodeIdRef !== node.id) {
onDrop(
{
dropType: 'transform',
dropNodeId: node.id,
dragNodeId: draggingNodeIdRef,
project,
},
e,
)
}
},
onpointerenter: (e) => {
e.preventDefault()
e.stopPropagation()

if (isDragging.current) {
if (!draggingNodeIdRef) return

if (isDraggingSelf.current) {
log.debug('Compositor: Mouseenter self', node.id)
rootRef.current?.toggleAttribute(
'data-transform-drop-self-active',
true,
)
} else {
log.debug('Compositor: Mouseenter other', node.id)
rootRef.current?.toggleAttribute(
'data-transform-drop-target-active',
true,
)
}

// Timeout to ensure "dragenter" runs after
// "dragleave" for other elements for global updates
setTimeout(() => {
if (!isDragging.current) {
wrapperEl.toggleAttribute('data-drop-target-ready', true)
}
})
// Timeout to ensure "dragenter" runs after
// "dragleave" for other elements for global updates
setTimeout(() => {
if (!isDraggingSelf.current) {
wrapperEl.toggleAttribute('data-drop-target-ready', true)
}
})
}
},
ondragleave: (e) => {
onpointerleave: (e) => {
e.preventDefault()
e.stopPropagation()

if (!draggingNodeIdRef) return

rootRef.current?.toggleAttribute(
'data-transform-drop-self-active',
false,
Expand Down Expand Up @@ -308,6 +363,12 @@ const ElementTree = (props: { nodeId: string }) => {
}
}, [interactiveRef.current])

useEffect(() => {
if (rootRef.current) {
Object.assign(interactiveRef.current, layoutDragHandlers)
}
}, [rootRef.current])

const layoutProps = {
layout,
...(nodeProps.layoutProps ?? {}),
Expand Down Expand Up @@ -335,7 +396,6 @@ const ElementTree = (props: { nodeId: string }) => {
{...(isDropTarget && {
'data-drop-target': true,
})}
{...layoutDragHandlers}
style={{
position: 'relative',
width: nodeProps.size?.x || '100%',
Expand Down Expand Up @@ -488,7 +548,6 @@ const Root = (props: { setStyle: (CSS: string) => void }) => {
<div
{...{
onDrop: (e: React.DragEvent) => {
foundDropTarget = true
e.preventDefault()
},
onDragOver: (e: React.DragEvent) => {
Expand Down Expand Up @@ -518,6 +577,7 @@ const Root = (props: { setStyle: (CSS: string) => void }) => {
)
}

let dragPreviewEl: HTMLElement
let wrapperEl: HTMLElement
let customStyleEl: HTMLStyleElement

Expand Down Expand Up @@ -565,9 +625,14 @@ export const render = (settings: CompositorSettings) => {
justifyContent: 'center',
transformOrigin: 'center',
})
dragPreviewEl = document.createElement('div')
dragPreviewEl.id = 'drag-preview'

containerEl.shadowRoot.appendChild(baseStyleEl)
containerEl.shadowRoot.appendChild(customStyleEl)
containerEl.shadowRoot.appendChild(wrapperEl)
containerEl.shadowRoot.appendChild(dragPreviewEl)

// Scale and center the compositor to fit in the container
const resizeObserver = new ResizeObserver((entries) => {
setScale()
Expand Down Expand Up @@ -654,6 +719,9 @@ const CompositorProvider = ({
...props
}: PropsWithChildren<CompositorSettings>) => {
const [draggingNodeId, setDraggingNodeId] = useState<string | null>(null)
const [dropTargetNodeId, setDropTargetNodeId] = useState<string | null>(null)

draggingNodeIdRef = draggingNodeId

return (
<CompositorContext.Provider
Expand Down Expand Up @@ -747,6 +815,25 @@ ls-layout[layout="Presentation"][props*="\\"cover\\"\\:true"] > :first-child .Na
cursor: grabbing !important;
}

#drag-preview {
position: absolute;
z-index: 2;
top: 0;
left: 0;
width: 100px;
height: 60px;
opacity: 0;
background: rgba(0,0,0,0.2);
border: 3px solid rgba(255,255,255,0.3);
pointer-events: none;
}
#drag-preview[data-active] {
opacity: 1;
}
#compositor-root[data-drop-target-ready] ~ #drag-preview {
display: none;
}

[data-drag-target] {}
[data-drag-target]:hover:not([data-drag-target-active]) > .interactive-overlay {
box-shadow: 0 0 0 3px inset rgba(255, 255, 255, 0.5);
Expand Down
Loading