🐛 Fix: dragging an event with its form window open does not work

[that-one-arab]

Description

Closes #298

I'm not sure why we have the below removed piece of code, but it was the culprit behind the issue.

I read the code comments about allowing drag only if form is open, but I couldn't comprehend it.

If it was part of an important UX logic we can revisit this in the future

Videos

Before

Peek.2025-03-13.15-50.webm

After

Peek.2025-03-13.13-47.webm

[tyler-dane]

I'm good with this. I'm guessing I kept that code in there by accident during the initial PR. It seems like it's not needed, so glad we can delete it.

"I read the code comments about allowing drag only if form is open, but I couldn't comprehend it."

I'm assuming the code comment you're referring to is

// If form is open, only allow drag if mouse is still held

or

When form is open, only allows drag if mouse is still held down

These comments are trying to clarify why we need to check whether the form is open: to help us differentiate between a drag and a simple mouse move. A drag happens when the mouse is down and moving.

[tyler-dane]

This change broke this test:

The test validates a useful use-case; we don't want onDrag rendering superfluously, which will cause performance issues that are difficult to debug down the road.

Can you investigate how to preserve the functionality while also not breaking the test? @that-one-arab

[tyler-dane]

Also please look into why CI isn't running for your PRs. CI should run automatically on every push as part of the PR process, so I'm confused why this wasn't caught before merge 🤔

Look at your previous PR - they don't have the green checks, which means CI isn't running. @that-one-arab

[that-one-arab]

This change broke this test:

The test validates a useful use-case; we don't want onDrag rendering superfluously, which will cause performance issues that are difficult to debug down the road.

Can you investigate how to preserve the functionality while also not breaking the test? @that-one-arab

Ah thanks for pointing this out. I'll check this out and ensure that future changes made won't break existing tests

[that-one-arab]

Also please look into why CI isn't running for your PRs. CI should run automatically on every push as part of the PR process, so I'm confused why this wasn't caught before merge 🤔

Look at your previous PR - they don't have the green checks, which means CI isn't running. @that-one-arab

My bad I wasn't aware of the CI checks, I'll make sure it runs in future PRs.

[tyler-dane]

Also please look into why CI isn't running for your PRs. CI should run automatically on every push as part of the PR process, so I'm confused why this wasn't caught before merge 🤔
Look at your previous PR - they don't have the green checks, which means CI isn't running. @that-one-arab

My bad I wasn't aware of the CI checks, I'll make sure it runs in future PRs.

It should run automatically on every push based on .github/workflows/test.yml. If you ever find out why it hasn't been running for your PRs, LMK. Perhaps some setting/config is off