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

Event is being dragged instead of clicked #150

Open
1 task done
tyler-dane opened this issue Oct 7, 2024 · 7 comments
Open
1 task done

Event is being dragged instead of clicked #150

tyler-dane opened this issue Oct 7, 2024 · 7 comments
Labels
bug Something isn't working needs more info Not ready for coding just yet

Comments

@tyler-dane
Copy link
Contributor

tyler-dane commented Oct 7, 2024

Prerequisites

  • Using an up-to-date main branch

Expected Behavior

Given that a user clicks an event,
while also (accidentally) slightly moves mouse,
then the change is registered as click
and not a drag.

Current Behavior

The above scenario is treated like a drag, which results in the event shifting slightly before the form is opened

Steps to Reproduce

  1. While slightly moving your mouse, click an event

  2. Notice: the event is dragged instead of staying in place

Possible Solution (Not obligatory)

Since much of the event drafting logic is built on the mouseDown, mouseUp, and onClick event handlers, we need to be careful that whatever solution we find to this problem doesn't create even more downstream issues.

As a result, a POC or diagram that highlights the proposed solution would be a good place to start before diving into creating a PR

Context

This results in bad UX by forcing users to be very strict about how they click events. After this happens a few times, users lose confidence in Compass's ability to help them accomplish their tasks easily. Clicking and dragging events are some of the most popular interactions users have with a calendar app, so we need to make this more reliable and smooth.

@tyler-dane tyler-dane added the bug Something isn't working label Oct 7, 2024
@DiegoMutre
Copy link
Contributor

Hey @tyler-dane 👋, can I work on this one?

@tyler-dane
Copy link
Contributor Author

Hey @tyler-dane 👋, can I work on this one?

You bet!

@DiegoMutre
Copy link
Contributor

Hey @tyler-dane, sorry for bothering you, I want to know if this is the issue because when I use other browsers, like MS Edge, Firefox, Chrome or Brave, I don't get this issue. But, when I use my main browser, Arc, I get this:

Screen.Recording.2024-10-19.152035.mp4

Maybe is this one (recorded in Chrome). As you can see, when slightly moving the mouse the event does shift.

Screen.Recording.2024-10-19.152937.mp4

You can add a screen recording to understand better the issue.
Btw, something like how google calendar handles the drag is expected?

Screen.Recording.2024-10-19.153434.mp4

Thanks for your patience.

@tyler-dane
Copy link
Contributor Author

tyler-dane commented Oct 20, 2024

Hey @DiegoMutre - no need to apologize! The behavior is confusing, and the fix isn't super clear and might not be straightforward.

See this video to see the buggy behavior. Notice how after a simple click during a mouse move the event is being registered as a 'drag' event instead of a 'click', which results in the event moving position on the grid.

The bug is more apparent when the mouse moves downwards, due to how the draft event uses a position offset (another behavior that is not ideal).

The bug behavior should be consistent across browsers. It didn't appear on those other browsers because of how you were clicking and moving the mouse. If you click slowly while the mouse is in the same spot it doesn't happen.

BugDemo.mov

@tyler-dane
Copy link
Contributor Author

Unassigning due to inactivty

@murilo9
Copy link
Contributor

murilo9 commented Dec 8, 2024

I was thinking about adding some logic inside GridEvent's onMouseDown event so we can distinguish between clicks and drags. If user holds for more than, say, 500ms then it's a drag, otherwise it's a click.
The tricky point is that we would need to add more listeners to the onMouseDown events (so we can stop the time counter), making the logic considerably more complex.
I don't quite understand the logic inside MainGridEvents, though. I see it renders some instances of GridEventMemo, where the onMouseDown function dispatches a store action (which I don't know what it does) if event form is open, and calls editTimedEvent otherwise (which I thought it would open the event form as the name suggests, but it actually starts a drag event).

@tyler-dane
Copy link
Contributor Author

I like the idea of updating onMouseDown to distinguish between clicks and drags using a timer. However, I agree that it could get complicated if not done carefully.

Regarding the confusion about how the onMouseDown listener interacts with the store to start a drag event: this is admittedly very confusing. The complexity came due to the need to support the following functionality:

  • Discarding a draft event and its form upon grid click
  • Triggering a new event from a shortcut (c)
  • Dragging
  • Resizing
  • Starting a new draft from an empty grid click

Instead of diving into a solution, let's pause on this. I will follow up with a detailed requirements spec. Then we can look at that spec and decide the best path forward.

Eventually, we can write tests based on that spec. The tests and refactoring will hopefully make this code more manageable.

@tyler-dane tyler-dane moved this from Ready to Backlog in 🏗 Compass Roadmap Dec 9, 2024
@tyler-dane tyler-dane added the needs more info Not ready for coding just yet label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs more info Not ready for coding just yet
Projects
Status: Backlog
Development

No branches or pull requests

3 participants