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

Delegate called twice: dayView(dayView: DayView, didUpdate event: EventDescriptor) #356

Open
MadalinSava opened this issue Apr 6, 2023 · 5 comments
Labels

Comments

@MadalinSava
Copy link

When I drag an event after long-pressing it (without releasing!), after dropping it I'm seeing the delegate function always called twice.
This is caused by the snap animation in commitEditing. What happens on mouse release:

  1. handlePanGesture is called and since the state is ended, it calls commitEditing
  2. the snap animation starts
  3. immediately, before the animation ends, timelineDidLongPress is called
  4. since editedEventView is still there (animation did not finish and completionHandler was not called to set it to nil), the animation starts again
  5. the completion handler is called for both animations
@richardtop richardtop added the bug label Apr 6, 2023
@richardtop
Copy link
Owner

Are you having any issues because of this?

@MadalinSava
Copy link
Author

Kind of, when the event is updated the app is reacting to changes. Right now there's no visible issue but I will add some more functionalities around this.
A simple solution would be to use a weak reference to editedEventView in TimelinePagerView:370 (for the completionHandler in the commitEditing function, and add a guard to unwrap it. The second time the completion handler is called is should be nil since after the first call to dayView(dayView:didUpdate) I'm calling endEventEditing().

@richardtop
Copy link
Owner

richardtop commented Apr 7, 2023

Would you be able to implement this change?
To clarify, you experience this issue in the scenario of long pressing an event, editing it and then long pressing the timeline?

@MadalinSava
Copy link
Author

Long press into drag and drop (without releasing the touch in between).
I'll add this to my todo list, should be a simple fix but need to test it thoroughly.

@richardtop
Copy link
Owner

Feel free to propose the PR at this stage, so that I could test it as well. I also recommend testing with https://github.com/richardtop/CalendarApp since it's an end-to-end implementation of the calendar instead of the one embedded into this application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants