feat: Port Calendar Drag & Drop to pointerdrag library - #76
Conversation
|
Tested both PRs together (horde/Core#211 + this one) on top of
Tested in month and day/week views. No console errors, |
TDannhauer
left a comment
There was a problem hiding this comment.
Review
Thanks for the pointerdrag port and the resize timezone fix. Direction is right, and the day/week move/resize path (live client* + getBoundingClientRect() + scrollTop, mode bag on kronDrag/state) is a solid replacement for the old Object.extend(drag, …) pattern. The naive yyyy-MM-dd HH:mm:ss resize payload is a real bugfix.
Depends on horde/Core#211 (pointerdrag.js); that one is approved — merge core first (or in lockstep). Pierre’s confirmation on #74 covers month + day/week for the reported scroll case.
Request changes: all-day week/workweek moves
allday-move uses:
new HordeDraggable(event.value.nodeId, { threshold: 5 });with no ghosting: true.
Head events are position: relative. Without a ghost, the source stays under the cursor and keeps hit-testing. elementFromPoint → closest('.horde-drop-target') resolves to the original day cell → onDrop hits if (drop == el.up()) return and never moves the event to another day.
Old dragdrop2 always built a ghost and hid the source when not in caption/ghosting mode, so cross-day all-day drops used to work.
Suggested fix: enable ghosting: true for all-day (and/or set pointer-events: none on the source while active).
Also, onDragStart / onDrag for allday-move measure against .kronolithViewBody, while minTop / maxTop / clamps come from the view head. Wrong frame for the rubber-band UI (the drop hit-testing issue is the functional break).
Other notes (non-blocking)
- Day/week timed move still will not track a pure wheel scroll until the next
pointermove(core does not replay on external scroll). Month is fine (fixed ghost). Optional: scroll listener in core or here if #74 must hold for timed moves without mouse movement. - PR title is branch-like (
feat/pointerdrag port); commit messages are fine. - No composer pin that core includes
pointerdrag.js— document merge order or bump the core requirement once released.
Test matrix
- Month: drag + wheel mid-drag → ghost follows; drop on correct day
- Day/week timed move/resize
- Week all-day → other day (currently expected broken)
- Drag then release without crossing threshold → opens event (no stale dialog)
- Resize handle vs move (innermost wins)
Request changes for the all-day drop regression before merge.
|
Tested the all-day week/workweek case @TDannhauer flagged. Confirming the regression, but with a nuance that may narrow the fix: Visual drag is broken (as described): no ghost, the source event stays pinned inside its own day cell and only offsets slightly from the cursor — you have to drag far to notice it's even "attached". So the But the drop itself works: if I release over another day, the all-day event does move to that day, and the change is persisted (correct day after reload). So in my environment Tested on both the default theme and my custom theme — same behavior in both, so it's not theme CSS masking/altering the hit-test. So from here it looks like the functional break is limited to the visual feedback (missing ghost + rubber-band measured against the wrong frame), not the cross-day drop logic — at least for a simple one-day-to-another move. Happy to re-test whatever fix you push. |
|
Applied review feedback in 460af18: |
460af18 to
b77db15
Compare
horde/Core#211 and #76 together seem to fix #74 (calendar event drag and drop move / resize) based on more modern javascript
Requires to-be-released new Core version with pointerdrag.js (bump dependency once core got released)