Skip to content

[scheduler] Improve event popup interactions and toggle behavior#22538

Open
mustafajw07 wants to merge 17 commits into
mui:masterfrom
mustafajw07:feature/21921-Allow-interacting-with-events-and-calendar
Open

[scheduler] Improve event popup interactions and toggle behavior#22538
mustafajw07 wants to merge 17 commits into
mui:masterfrom
mustafajw07:feature/21921-Allow-interacting-with-events-and-calendar

Conversation

@mustafajw07

@mustafajw07 mustafajw07 commented May 20, 2026

Copy link
Copy Markdown
Contributor

Closes #21921

  • Allow switching between events with a single click while popup is open
  • Make clicking the active event toggle and close its popup
  • Close popup on empty calendar click without triggering event creation
  • Preserve normal single-click event creation when no popup is open
  • Apply behavior across EventCalendar views and EventTimeline

Changelog

@code-infra-dashboard

code-infra-dashboard Bot commented May 21, 2026

Copy link
Copy Markdown

Deploy preview

https://deploy-preview-22538--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.

Comment on lines +95 to +98
const dataId = (data as any)?.id;
const currentId = (currentData as any)?.id;
if (isOpen && dataId != null && dataId === currentId) {
onClose();

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.

Codex finding, recurring occurrences with the same event id cannot be switched in one click:

the toggle logic compares only data.id. Premium recurring occurrences share the event id and differ by occurrence.key (eventId::dateKey), so clicking a different occurrence of the same recurring event closes the dialog instead of opening the clicked occurrence.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I’ve updated the logic to account for recurring occurrences as well.

* Sets the ID of the event currently open in the event dialog.
* Pass `null` to clear the open event.
*/
public setOpenEventId = (eventId: SchedulerEventId | null) => {

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.

This seems to only change the store state but doesn't open the dialog?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I’ve updated the implementation to ensure the dialog open state is handled correctly alongside the selected event state.

if (!(node instanceof Element)) {
continue;
}
if (node.matches('[data-event-dialog-trigger]')) {

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.

This seems a bit loose, not sure if it's a realistic scenario but if there are 2 schedulers on the page, clicking an event in scheduler B while scheduler A’s dialog is open will bypass A’s click-away close and can leave both dialogs open

@mustafajw07

Copy link
Copy Markdown
Contributor Author

@rita-codes / @mj12albert
Requested changes have been updated. Please review and let me know if any further changes are needed.

Thanks.

@rita-codes

Copy link
Copy Markdown
Member

Thanks for working on this!!
One thing about the expected behavior here 👀
I checked the preview and one thing is missing: "Clicking on an empty area of the calendar should close the popup without creating a new event."
Right now if you click in an empty area the popup closes but it's creating a new event, could you take a look?
Thanks!!

Screen.Recording.2026-06-08.at.11.18.59.mov

@mustafajw07

Copy link
Copy Markdown
Contributor Author

I checked the preview and one thing is missing: "Clicking on an empty area of the calendar should close the popup without creating a new event."
Right now if you click in an empty area the popup closes but it's creating a new event

I've fixed the issue now.

Clicking on an empty area of the calendar will close the popup without creating a new event. I verified the behavior in the preview and it is working as expected.

@rita-codes rita-codes added the type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. label Jun 9, 2026
@rita-codes

Copy link
Copy Markdown
Member

Thanks for the work on this @mustafajw07! 🙏 I tested it and found a blocking issue, and digging into it made me question the overall approach for closing the popup. Let me walk through how I got there.

The bug

When you open the recurrence tab and pick an option from the frequency dropdown, the popup closes. Same thing happens with the resource/color Select (ResourceAndColorSection.tsx).

Root cause

The popup now closes via a ClickAwayListener wrapping the <Dialog>. A ClickAwayListener only "sees" the subtree of the node it wraps — but @mui/material/Select (and pickers, menus, etc.) render their popup in a separate portal at body level. So clicking a menu item lands outside the dialog's subtree → onClickAway fires → onClose().

This isn't a one-off: it affects every portaled overlay inside the dialog. The !isScopeDialogOpen guard on the ClickAwayListener is actually the same bug already patched by hand for one specific case (the recurring scope dialog). The recurrence/resource dropdowns are the same problem, unpatched.

Why I don't think narrow patches are the way

The current close logic is opt-out: close on any click, unless it's inside the dialog, or an event trigger, or the scope dialog…. That list of exceptions can never be complete:

  1. Every new internal overlay (pickers, menus, autocompletes) breaks it.
  2. More importantly — the form is meant to be extensible with custom fields. Any custom field a consumer drops in (their own Select, a third-party date picker, etc.) opens a portal we don't control, which will read as an "outside click" and close the dialog.

I looked into whether moving to base-ui's Popover (useDismiss + FloatingTree, both already available as deps) would fix it. It solves the internal overlays, but not custom fields: the FloatingTree only protects floating-ui descendants, and arbitrary consumer portals won't be in the tree. So no overlay primitive — Material or base-ui — robustly solves this as long as the close logic is "close on outside click".

Proposed direction: opt-in dismissal

Instead of "close on any outside click, minus a list of exceptions", close only on explicit, intended gestures (an allowlist), which are finite and we fully control:

  • Esc
  • the close button
  • clicking another event (switch)
  • clicking the calendar surface (empty cell)

Any field popup (custom or internal) floats at body level — it's neither on the calendar surface nor another event — so it never triggers a close. This is robust against arbitrary custom portals and reuses the calendar click handler that already exists in useEventCreation.ts (it already knows openEventId). The model becomes "the calendar tells the dialog to close" rather than "the dialog listens to the whole document". As a bonus it makes pointerEvents: none, the data-event-dialog-trigger + composedPath scanning, and the scope-dialog special-case unnecessary.

@flaviendelangle @noraleonte — before @mustafajw07 invests more here, could you confirm how you see this? The chain of reasoning was: recurrence dropdown closes the popupit's a portal/ClickAwayListener issue, not isolatedbase-ui's tree-aware dismiss doesn't cover custom fields eitherso the close logic itself (the opt-out approach) is the wrong shape, and an opt-in allowlist of gestures driven by the calendar is more robust for an extensible form. Does that match how you'd want event-dialog dismissal to work, especially with custom fields in mind?

A few smaller things I also noted, independent of the above:

  • Focus trap vs. non-modal: aria-modal="false" + transparent/pointerEvents:none backdrop, but the <Dialog> keeps its default focus trap (no disableEnforceFocus/disableAutoFocus) — contradictory for a popup meant to let you interact with the calendar behind it.
  • Toggle by key with two formats: trigger open uses occurrence.key, imperative open (setOpenEventId) uses String(processed.id) — they don't match, so toggling an event that was opened via the public API reopens instead of closing.
  • setOpenEventId opens with no anchor (EMPTY_ANCHOR_REF), so programmatic open isn't positioned.
  • No test for the failing case: there's no test that opens a Select/picker inside the dialog and asserts it stays open — that test would currently fail.

@mustafajw07

mustafajw07 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed analysis. I agree this seems more like a dismissal-strategy question than an isolated bug. I'll wait for feedback from @flaviendelangle / @noraleonte on the proposed direction before making larger changes.

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] Allow interacting with events and calendar while event popup is open

4 participants