[scheduler] show drag placeholder when re-entering the same position#22775
Conversation
|
@rita-codes @michelengelen @zannager Kindly have a review on this PR. Thanks! |
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The failing |
|
@rita-codes and @flaviendelangle would you please review this one? I do not have enough experience with this codebase yet! |
…ore/SchedulerStore.utils.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/SchedulerStore.utils.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/SchedulerStore.utils.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/SchedulerStore.utils.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/SchedulerStore.utils.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/SchedulerStore.utils.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
…ore/SchedulerStore.utils.test.ts Co-authored-by: Rita <[email protected]> Signed-off-by: Adarsh Singh <[email protected]>
Thanks for the review, @rita-codes. I've incorporated all of the suggested changes and pushed the updates. I appreciate your feedback and guidance on this. Please let me know if there's anything else you'd like me to update. |
|
@rita-codes Sorry to ask again 🙏, but could you please re-run the two failing checks: |
…ui#22775) Signed-off-by: Adarsh Singh <[email protected]> Co-authored-by: Rita <[email protected]>
Closes #22754
Root cause
When dragging an external event (
StandaloneEvent) into the Event Calendar, if the pointer leaves the drop surface and then re-enters at the same position, the drag placeholder stayed invisible. It only reappeared once the pointer moved to a different slot.shouldUpdateOccurrencePlaceholderinpackages/x-scheduler-internals/src/internals/utils/SchedulerStore/SchedulerStore.utils.tsis the dirty-check that decides whethersetOccurrencePlaceholderwrites a new placeholder to the store. Its diff was asymmetric: it iterated only over the keys ofnext, so a key that exists onpreviousbut is absent onnextwas never compared.The exact flow that triggers it:
onDragLeavesets the placeholder to{ ...current, isHidden: true }. For external drag this always happens, and the selectors hide the placeholder whileisHiddenis truthy.onDragbuilds a fresh placeholder that does not includeisHidden.shouldUpdateOccurrencePlaceholder(previous = { ..., isHidden: true }, next = { ... no isHidden })loops only overnext's keys, so theisHiddenkey (present onprevious, missing onnext) is never inspected. With everything else identical, it returnsfalse, the store is not updated, and the placeholder stays hidden.This was not specific to
isHidden. Any optional key present onpreviousbut dropped fromnextwas ignored by the diff.Change
Make the diff symmetric. After the existing pass over
next's keys, a second pass overprevious's keys returnstrueas soon as it finds a key that no longer exists onnext:startandendare required on every placeholder, so they never appear in the second pass and theadapter.isEqualspecial-case is untouched. The pass is allocation-free and keeps the early-exit, so the drag hot path is unaffected.Why it is safe
Every placeholder construction site was reviewed. No production flow relies on the previous asymmetric behavior. The only present-in-
previousand absent-in-nexttransition that happens with everything else unchanged is theisHiddenun-hide case (the bug). The other optional keys (lockSurfaceType,onEventDrop) only change alongside a required key that is already present onnext, so the diff result is identical there.Tests
Added tests in
SchedulerStore.utils.test.tscoveringshouldUpdateOccurrencePlaceholder, including the regression case:previouswithisHidden: trueversus an otherwise identicalnextwithoutisHiddenmust returntrue. The regression test fails on the original code and passes with the fix. Typecheck, prettier, and eslint are clean.How to reproduce and verify
In the External drag and drop demo on the Event Calendar drag interactions page:
Before this change the placeholder stays hidden until you move to another slot. After this change it reappears immediately.