Skip to content

Conversation

arjansingh
Copy link
Contributor

@arjansingh arjansingh commented Sep 26, 2025

Summary

Fixes drag handling logic.

Changes

Only check for drag on left-click.

Adds handler logic for following pointer events:

  1. drag termination
  2. context menu
  3. pointer cancel

Adds tests.

Consolidates cleanup tasks.

Screenshots

Fixed State:

Ignore first failed drag, browser window didn't have context.

Fix-Dragging-2025-09-25.mov

┆Issue is synchronized with this Notion page by Unito

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 26, 2025
Copy link

github-actions bot commented Sep 26, 2025

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 09/26/2025, 12:59:58 AM UTC

📈 Summary

  • Total Tests: 462
  • Passed: 431 ✅
  • Failed: 0
  • Flaky: 2 ⚠️
  • Skipped: 29 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 424 / ❌ 0 / ⚠️ 2 / ⏭️ 29
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

Comment on lines +70 to +71
let mockNodeData: VueNodeData
let mockOnPointerUp: ReturnType<typeof vi.fn>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you keep these local to each it()?

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, you could have them be const and resetAllMocks() would take care of the state for you.

await nextTick()

pointerHandlers.onPointerup(createPointerEvent('pointerup'))
await nextTick()
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.

i will remove the ticks.

Comment on lines +84 to +85
isDragging.value = false
layoutStore.isDraggingVueNodes.value = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to eliminate the local isDragging and always use layoutStore as the source of truth?

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'm not against this but it's not a trivial refactor... I could easily end up spending a day or two on it and that's exactly what we don't want me doing.

@arjansingh arjansingh merged commit 62441fa into main Sep 26, 2025
25 checks passed
@arjansingh arjansingh deleted the fix/drag-handlers branch September 26, 2025 19:05
christian-byrne pushed a commit that referenced this pull request Sep 27, 2025
## Summary

Fixes drag handling logic.

## Changes

Only check for drag on left-click.

Adds handler logic for following pointer events:
1. drag termination
2. context menu
3. pointer cancel

Adds tests.

Consolidates cleanup tasks.

## Screenshots

Fixed State:

Ignore first failed drag, browser window didn't have context.


https://github.com/user-attachments/assets/00ec685a-1ef7-4102-b19b-4cdb9b201d22

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5784-fix-do-not-drag-on-right-click-fix-refs-27a6d73d3650812ea797fccf14022568)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:node-interaction area:vue-migration size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants