Skip to content

[scheduler] Polish EventCalendar rendering for multi-resource events#23161

Merged
rita-codes merged 7 commits into
mui:masterfrom
mustafajw07:feature/23015-calendar-multi-resource
Jul 22, 2026
Merged

[scheduler] Polish EventCalendar rendering for multi-resource events#23161
rita-codes merged 7 commits into
mui:masterfrom
mustafajw07:feature/23015-calendar-multi-resource

Conversation

@mustafajw07

@mustafajw07 mustafajw07 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Closes #23015

Changelog

@code-infra-dashboard

code-infra-dashboard Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploy preview

https://deploy-preview-23161--material-ui-x.netlify.app/
QR code for https://deploy-preview-23161--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-scheduler 0B(0.00%) 0B(0.00%)
@mui/x-scheduler-premium 0B(0.00%) 0B(0.00%)
@mui/x-chat 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.

@mustafajw07

Copy link
Copy Markdown
Contributor Author

Hey @rita-codes, can you review this pr?

@zannager zannager added the scope: scheduler Changes related to the scheduler. label Jul 20, 2026
@rita-codes rita-codes added the type: new feature Expand the scope of the product to solve a new problem. label Jul 21, 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.

Thanks for working on this! The coverage matches the visibility scope of #23015 and CI is green, but two of the issue's acceptance criteria aren't fully pinned yet:

  1. 🟡 The positive assertions don't pin the "renders once" criterion. The acceptance criterion in #23015 is that a multi-resource event renders once in each view, and the characteristic failure mode for multi-resource events is duplicate rendering (one occurrence per assigned resource). expect(screen.getAllByText('Team Sync').length).to.be.greaterThan(0) still passes in that case, so the tests can't catch the main regression they're meant to guard against. Since these are timed single-day events (the invisible layout placeholders only apply to all-day events), asserting the exact count is safe here. Applies to the new tests in all four views.

  2. 🟡 The color criterion is untested. #23015 also lists "color is derived from the first resource" in its scope, but no test in this PR covers it. The behavior is already implemented (schedulerEventSelectors.color resolves through the primary resource id), so it's assertable today — covering it in a single view would be enough, since the selector is shared. Without it, the PR would close the issue with that criterion unpinned.

  3. 🟡 Nit: test descriptions in this repo start with "should ..." — the internals tests added in this PR follow the convention, but the new view tests don't. Worth aligning them for consistency.

@mustafajw07

Copy link
Copy Markdown
Contributor Author

Thanks for working on this! The coverage matches the visibility scope of #23015 and CI is green, but two of the issue's acceptance criteria aren't fully pinned yet:

  1. 🟡 The positive assertions don't pin the "renders once" criterion. The acceptance criterion in [scheduler] EventCalendar: multi-resource rendering polish #23015 is that a multi-resource event renders once in each view, and the characteristic failure mode for multi-resource events is duplicate rendering (one occurrence per assigned resource). expect(screen.getAllByText('Team Sync').length).to.be.greaterThan(0) still passes in that case, so the tests can't catch the main regression they're meant to guard against. Since these are timed single-day events (the invisible layout placeholders only apply to all-day events), asserting the exact count is safe here. Applies to the new tests in all four views.
  2. 🟡 The color criterion is untested. [scheduler] EventCalendar: multi-resource rendering polish #23015 also lists "color is derived from the first resource" in its scope, but no test in this PR covers it. The behavior is already implemented (schedulerEventSelectors.color resolves through the primary resource id), so it's assertable today — covering it in a single view would be enough, since the selector is shared. Without it, the PR would close the issue with that criterion unpinned.
  3. 🟡 Nit: test descriptions in this repo start with "should ..." — the internals tests added in this PR follow the convention, but the new view tests don't. Worth aligning them for consistency.

Thanks for the detailed review! I've addressed all three points:

  • Updated the view tests to assert the exact render count so they verify the "renders once" behavior.
  • Added a test covering the color criterion using the primary resource.
  • Renamed the new view tests to follow the repository's "should ..." naming convention.

The PR has been updated. Could you take another look when you have a chance? Thanks! 🙌

@mustafajw07
mustafajw07 requested a review from rita-codes July 21, 2026 16:18

@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.

Thanks for the quick turnaround, all three points are addressed, approving 🙌

One small cleanup: I'd drop the repeated "Pins the 'renders once' criterion…" comments. The test titles already convey it, so the comments don't add information for future readers 👀

Thanks for taking care of this!!

@mustafajw07

Copy link
Copy Markdown
Contributor Author

Thank you for the review and the approval! 🙌 I've removed those comments as well. Really appreciate all the feedback throughout this PR!

@rita-codes
rita-codes merged commit fca816e into mui:master Jul 22, 2026
22 checks passed
@mustafajw07
mustafajw07 deleted the feature/23015-calendar-multi-resource branch July 22, 2026 11:16
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: new feature Expand the scope of the product to solve a new problem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[scheduler] EventCalendar: multi-resource rendering polish

3 participants