Skip to content

[pickers][DateCalendar] Derive the calendar dimensions from - #23258

Open
Syed-Ali-Abbas-Zaidi wants to merge 3 commits into
mui:masterfrom
Syed-Ali-Abbas-Zaidi:pickers-calendar-size-tokens
Open

[pickers][DateCalendar] Derive the calendar dimensions from#23258
Syed-Ali-Abbas-Zaidi wants to merge 3 commits into
mui:masterfrom
Syed-Ali-Abbas-Zaidi:pickers-calendar-size-tokens

Conversation

@Syed-Ali-Abbas-Zaidi

Copy link
Copy Markdown

@code-infra-dashboard

code-infra-dashboard Bot commented Jul 30, 2026

Copy link
Copy Markdown

Deploy preview

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 🔺+7B(0.00%) 🔺+81B(+0.14%)
@mui/x-date-pickers-pro ▼-271B(-0.10%) 🔺+38B(+0.05%)
@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.

Comment thread packages/x-date-pickers-pro/src/DateRangeCalendar/DateRangeCalendar.tsx Outdated
Comment thread packages/x-date-pickers/src/internals/constants/dimensions.ts Outdated
Comment thread packages/x-date-pickers/src/DateCalendar/DateCalendar.tsx
@yairEO

yairEO commented Jul 31, 2026

Copy link
Copy Markdown

I've checked-out this branch and resizing looks good 🏆

Question:

Why --PickerDay-size isn't defined by default but instead a fallback value is used everywhere?

Recommendation:

font-size should be in em units and not rem units which are very limiting. Using em has enormous benefit of controlling all the font sizes within a single place, at the component's root, as seen at the end of my video below:

mui-data-calendar-resize.mp4

Would need to replace all font-size in rem units to em and simply define at root-level a default font-size: 1rem, or alternatively set root at 0.75rem and the sub-components to 1em and thus they will all inherit it. One place to rule them all (relatively).

@Syed-Ali-Abbas-Zaidi

Copy link
Copy Markdown
Author

I've checked-out this branch and resizing looks good 🏆

Question:

Why --PickerDay-size isn't defined by default but instead a fallback value is used everywhere?

Recommendation:

font-size should be in em units and not rem units which are very limiting. Using em has enormous benefit of controlling all the font sizes within a single place, at the component's root, as seen at the end of my video below:

mui-data-calendar-resize.mp4
Would need to replace all font-size in rem units to em and simply define at root-level a default font-size: 1rem, or alternatively set root at 0.75rem and the sub-components to 1em and thus they will all inherit it. One place to rule them all (relatively).

Thanks for checking out the branch 🙏

Why a fallback instead of a default declaration

Because declaring the variable is what breaks the cascade. Any element that declares --PickerDay-size wins over an inherited value for itself and everything below it that's precisely the bug this PR fixes. PickerDay used to declare --PickerDay-size: 36px on its own root, so setting it on DateCalendar, a wrapper, or the theme did nothing to the day cells: the day's own declaration shadowed it.

Moving that declaration up to the calendar root would only move the problem one level the calendar's declaration would then beat anything you set on a wrapper, in styleOverrides, or on :root.

By only ever reading it (var(--PickerDay-size, 36px)), the value can come from any level and the nearest ancestor wins, which is what you'd expect from a token: :root, theme styleOverrides, a wrapper element, the picker's sx, or a single day through slotProps.

The one real cost is discoverability it won't show up in DevTools computed styles until you set it. The way to get both is to register the property:

@property --PickerDay-size {
  syntax: "<length>";
  inherits: true;
  initial-value: 36px;
}

@zannager zannager added the scope: pickers Changes related to the date/time pickers. label Aug 3, 2026
@yairEO

yairEO commented Aug 6, 2026

Copy link
Copy Markdown

Hi, I saw you ignored my font-size suggestion, may I know why?

@noraleonte noraleonte left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for this — the direction is right, and I want to call out the detail that made it easy to review: deriving the header height and horizontal padding from the existing VIEW_HEIGHT/DIALOG_WIDTH constants, rather than re-deriving the magic numbers by hand. That's what makes the unchanged default rendering provably correct instead of coincidental. 🙏

The constraint we're working under is that this lands in a minor, so it has to be fully non-breaking.
Removing the css vars from the day slots and moving them higher up is a breaking change, and so is making the margin require a unit - even with the updated migration guide.

There's a path could be explored and satisfies both, and it's worth writing down before this gets picked back up. Introduce a new calendar-level token that drives the surrounding dimensions, and keep --PickerDay-size declared on the day sourced from it. Nothing pre-existing changes behavior, and ancestor values still reach the day. Then derive the track from the day size plus a constant 4px rather than from the margin variable — that removes the unit requirement entirely, and costs nothing, since margin 0 already centers a 252px row inside a 320px calendar today.

A good chunk of this survives either way: the MonthCalendar/YearCalendar width change, the skeleton fix, the RANGE_VIEW_HEIGHT cleanup, and the new dimension tests are all keepers.

I'm parking this for now rather than closing it — happy to take it over and build on what's here, or to keep reviewing if you'd like to try another approach yourself. Either way, thanks for digging into this one 💙

@Syed-Ali-Abbas-Zaidi

Copy link
Copy Markdown
Author

@noraleonte Thank you for the detailed review. I have opened a separate PR keeping in mind the things you mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: pickers Changes related to the date/time pickers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants