Skip to content

refactor(datepicker): use relative units for datepicker sizing#1851

Open
spliffone wants to merge 1 commit intomainfrom
feat/datepicker-relative-sizing
Open

refactor(datepicker): use relative units for datepicker sizing#1851
spliffone wants to merge 1 commit intomainfrom
feat/datepicker-relative-sizing

Conversation

@spliffone
Copy link
Copy Markdown
Member

@spliffone spliffone commented Apr 10, 2026

@spliffone spliffone requested review from a team as code owners April 10, 2026 12:33
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the datepicker styles by replacing fixed pixel values with relative units (rem and ch) and correcting a comment in the SCSS file. A critical issue was identified where units were omitted for block-size and line-height in the .selection class, which would result in invalid CSS.

Comment thread projects/element-ng/datepicker/si-datepicker.component.scss Outdated
@spliffone spliffone force-pushed the feat/datepicker-relative-sizing branch 2 times, most recently from 159b369 to 7717e79 Compare April 10, 2026 18:59
.calendar {
block-size: 28em;
flex: 1;
min-block-size: 24.5rem;
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.

this is too much as there are also some px based margin/paddings. Since we want to avoid calc here (nightmare to maintain):

  • add a rem-based min-block-size to the .calendar ::ng-deep table
  • add a min-block-size with calc for the footer in day-selection and empty dummy footers to the year/month selections
Image

(and also we're showing too many weeks e.g. for February/March 2026)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I will have look.

(and also we're showing too many weeks e.g. for February/March 2026)

Showing so many weeks is on purpose - it was requested from Timo.

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.

@timowolf why? I think this is confusing. Showing half of March when the selected month is February...i.e. what is the added value?

/cc @panch1739

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.

@dr-itz i think is how it works now? Our current date range is already showing all the off-month days. Im not sure why it was decided this way 🤷

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@panch1739 @dr-itz as far as I remember it was introduced with the date-range two-month view. In worst case - one view hat 6 rows since the month started and ended in the middle of a week and on the other side it ended exactly on the last day of the week (only 5 rows). Means the two views had a different height and the divider was on different positions or it looked somehow unbalanced. @timowolf Correct me if I am wrong.

@spliffone spliffone force-pushed the feat/datepicker-relative-sizing branch 2 times, most recently from 99b722f to 8e4723d Compare April 13, 2026 18:07
@github-actions
Copy link
Copy Markdown

⬇️ Download VRTs

@spliffone spliffone force-pushed the feat/datepicker-relative-sizing branch from 8e4723d to 715fc53 Compare April 14, 2026 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants