Skip to content

[scheduler] show drag placeholder when re-entering the same position#22775

Merged
rita-codes merged 12 commits into
mui:masterfrom
Anexus5919:fix/scheduler-drag-placeholder-reenter
Jun 12, 2026
Merged

[scheduler] show drag placeholder when re-entering the same position#22775
rita-codes merged 12 commits into
mui:masterfrom
Anexus5919:fix/scheduler-drag-placeholder-reenter

Conversation

@Anexus5919

Copy link
Copy Markdown
Contributor

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.

shouldUpdateOccurrencePlaceholder in packages/x-scheduler-internals/src/internals/utils/SchedulerStore/SchedulerStore.utils.ts is the dirty-check that decides whether setOccurrencePlaceholder writes a new placeholder to the store. Its diff was asymmetric: it iterated only over the keys of next, so a key that exists on previous but is absent on next was never compared.

The exact flow that triggers it:

  1. onDragLeave sets the placeholder to { ...current, isHidden: true }. For external drag this always happens, and the selectors hide the placeholder while isHidden is truthy.
  2. On re-enter, onDrag builds a fresh placeholder that does not include isHidden.
  3. shouldUpdateOccurrencePlaceholder(previous = { ..., isHidden: true }, next = { ... no isHidden }) loops only over next's keys, so the isHidden key (present on previous, missing on next) is never inspected. With everything else identical, it returns false, the store is not updated, and the placeholder stays hidden.

This was not specific to isHidden. Any optional key present on previous but dropped from next was ignored by the diff.

Change

Make the diff symmetric. After the existing pass over next's keys, a second pass over previous's keys returns true as soon as it finds a key that no longer exists on next:

// Catch keys present in `previous` but removed from `next` (e.g. `isHidden`).
for (const key in untypedPrevious) {
  if (!(key in untypedNext)) {
    return true;
  }
}

start and end are required on every placeholder, so they never appear in the second pass and the adapter.isEqual special-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-previous and absent-in-next transition that happens with everything else unchanged is the isHidden un-hide case (the bug). The other optional keys (lockSurfaceType, onEventDrop) only change alongside a required key that is already present on next, so the diff result is identical there.

Tests

Added tests in SchedulerStore.utils.test.ts covering shouldUpdateOccurrencePlaceholder, including the regression case: previous with isHidden: true versus an otherwise identical next without isHidden must return true. 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:

  1. Drag the external event into the calendar and hover a column so the placeholder appears.
  2. Drag out of the grid so the placeholder hides.
  3. Drag back into the same column at the same position.

Before this change the placeholder stays hidden until you move to another slot. After this change it reappears immediately.

  • I have followed (at least) the PR section of the contributing guide.

@Anexus5919

Copy link
Copy Markdown
Contributor Author

@rita-codes @michelengelen @zannager Kindly have a review on this PR. Thanks!

@code-infra-dashboard

code-infra-dashboard Bot commented Jun 11, 2026

Copy link
Copy Markdown

Deploy preview

https://deploy-preview-22775--material-ui-x.netlify.app/

Bundle size

Bundle Parsed size Gzip size
@mui/x-data-grid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-premium 0B(0.00%) 0B(0.00%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-charts-premium 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)
@mui/x-license 0B(0.00%) 0B(0.00%)

Details of bundle changes


Check out the code infra dashboard for more information about this PR.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@Anexus5919

Copy link
Copy Markdown
Contributor Author

The failing ci/circleci: test_browser check is unrelated to this PR. The failure is in columns.DataGridPro.test.tsx ("should resize new rows correctly when they are added during column resize"), a timing-sensitive Data Grid resize test (1 failed out of 10,779). This PR only touches x-scheduler-internals, so it looks like a flake. A re-run of the CircleCI job should clear it. Happy to rebase on latest master if that helps re-trigger CI.

@michelengelen

Copy link
Copy Markdown
Member

@rita-codes and @flaviendelangle would you please review this one? I do not have enough experience with this codebase yet!

@michelengelen michelengelen added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. scope: scheduler Changes related to the scheduler. labels Jun 11, 2026
@michelengelen michelengelen changed the title fix(scheduler): show drag placeholder when re-entering the same position [scheduler] show drag placeholder when re-entering the same position Jun 11, 2026

@rita-codes rita-codes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some small requested changes to match our conventions, other than that the fix looks correct! Thank you so much for fixing this!

Anexus5919 and others added 7 commits June 12, 2026 15:40
…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]>
@Anexus5919

Copy link
Copy Markdown
Contributor Author

Left some small requested changes to match our conventions, other than that the fix looks correct! Thank you so much for fixing this!

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
rita-codes enabled auto-merge (squash) June 12, 2026 10:26
@Anexus5919

Anexus5919 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@rita-codes Sorry to ask again 🙏, but could you please re-run the two failing checks: ci/circleci: test_browser and ci/circleci: test_browser_react_18? I'd really appreciate it.

@rita-codes
rita-codes disabled auto-merge June 12, 2026 13:05
@rita-codes
rita-codes merged commit a888c84 into mui:master Jun 12, 2026
21 checks passed
mbrookes pushed a commit to mbrookes/mui-x that referenced this pull request Jun 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: scheduler Changes related to the scheduler. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[scheduler] Drag placeholder stays hidden when re-entering the same position (external drag)

3 participants