Fix Calendar max height#15127
Conversation
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR fixes calendar height constraints by implementing a consistent height inheritance pattern throughout the calendar component hierarchy. The changes add height: inherit to four calendar-related components (RecordCalendar, RecordCalendarMonth, RecordCalendarMonthBody, and RecordCalendarMonthBodyWeek) to allow proper vertical space utilization. The key addition is a min-height: 700px constraint in RecordCalendarMonth to ensure adequate space for weekly views and prevent scroll wrapper issues. These CSS modifications work together to resolve issue #15121 where the calendar view wasn't utilizing the full container height, creating a more responsive and visually consistent calendar interface.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
packages/twenty-front/src/modules/object-record/record-calendar/components/RecordCalendar.tsx |
4/5 | Changed height from 100% to inherit to allow proper height inheritance from parent container |
packages/twenty-front/src/modules/object-record/record-calendar/month/components/RecordCalendarMonth.tsx |
4/5 | Added height: inherit and min-height: 700px to ensure full container utilization with minimum space guarantee |
packages/twenty-front/src/modules/object-record/record-calendar/month/components/RecordCalendarMonthBody.tsx |
5/5 | Added height: inherit to allow calendar body to inherit parent height constraints |
packages/twenty-front/src/modules/object-record/record-calendar/month/components/RecordCalendarMonthBodyWeek.tsx |
4/5 | Added height calculation (100% / 5) to distribute week components evenly across parent container |
Confidence score: 4/5
- This PR addresses a clear UI issue with targeted CSS changes that follow a logical height inheritance pattern
- Score reflects the straightforward nature of the CSS fixes and consistent implementation across the component hierarchy
- Pay close attention to RecordCalendarMonth.tsx for the min-height:700px value which may need adjustment based on different screen sizes or user feedback
Sequence Diagram
sequenceDiagram
participant User
participant RecordCalendar
participant ScrollWrapper
participant RecordCalendarMonth
participant StyledContainer
participant RecordCalendarMonthBody
participant RecordCalendarMonthBodyWeek
participant RecordCalendarMonthBodyDay
User->>RecordCalendar: "Views calendar page"
RecordCalendar->>RecordCalendar: "Apply height: inherit to StyledContainerContainer"
RecordCalendar->>ScrollWrapper: "Renders with componentInstanceId"
ScrollWrapper->>RecordCalendarMonth: "Renders calendar month"
RecordCalendarMonth->>StyledContainer: "Apply height: inherit and min-height: 700px"
StyledContainer->>RecordCalendarMonthBody: "Renders month body with inherited height"
RecordCalendarMonthBody->>StyledContainer: "Apply height: inherit to container"
RecordCalendarMonthBody->>RecordCalendarMonthBodyWeek: "Maps weekFirstDays and renders weeks"
RecordCalendarMonthBodyWeek->>RecordCalendarMonthBodyWeek: "Apply height: calc(100% / 5) for equal week distribution"
RecordCalendarMonthBodyWeek->>RecordCalendarMonthBodyDay: "Renders individual days in week"
RecordCalendarMonthBodyDay-->>User: "Displays calendar with proper height constraints"
4 files reviewed, no comments
...t/src/modules/object-record/record-calendar/month/components/RecordCalendarMonthBodyWeek.tsx
Outdated
Show resolved
Hide resolved
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:56997 This environment will automatically shut down when the PR is closed or after 5 hours. |
…wenty into 15121_fix-calendar-max-height
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This review covers only the changes made since the last review. The PR adds height: inherit CSS property to multiple calendar components (RecordCalendar, RecordCalendarMonthBody, and RecordCalendarMonthBodyWeek) and adds min-height: 1000px to the RecordCalendarMonth component. These changes aim to fix calendar height constraints by allowing calendar components to inherit their parent container's height rather than being limited by viewport calculations. The implementation creates a height inheritance chain from parent containers down to individual calendar components, with a minimum height safeguard to prevent layout issues. This approach follows the existing pattern used for width constraints in the codebase.
Changed Files
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-front/src/modules/object-record/record-calendar/month/components/RecordCalendarMonth.tsx | 4/5 | Added height: inherit and min-height: 1000px to StyledContainer for proper height inheritance and layout protection |
| packages/twenty-front/src/modules/object-record/record-calendar/month/components/RecordCalendarMonthBody.tsx | 4/5 | Added height: inherit to StyledContainer to enable height inheritance from parent |
| packages/twenty-front/src/modules/object-record/record-calendar/components/RecordCalendar.tsx | 4/5 | Changed height from 100% to inherit in StyledContainerContainer for proper parent height inheritance |
| packages/twenty-front/src/modules/object-record/record-calendar/month/components/RecordCalendarMonthBodyWeek.tsx | 3/5 | Added height: inherit to StyledContainer but existing hardcoded week height calculation may still cause issues |
Confidence score: 3/5
- This PR addresses the calendar height issue but may not fully resolve the underlying problem with hardcoded week height calculations
- Score reflects that while the height inheritance approach is sound, the previous review identified a critical bug in week height calculation that remains unaddressed
- Pay close attention to RecordCalendarMonthBodyWeek.tsx as the hardcoded
calc(100% / 5)height calculation can still cause weeks to be hidden in 6-week months
4 files reviewed, no comments
|
@pvrnn Thanks for opening a PR, I'm starting with a functional review:
Tech review:
|
Ah yes, thanks for the feedback. Also I kept one
after (
I can revert this change if you prefer |
|
Hey @charlesBochet I've addressed your comment last thursday, lmk what you think 🙏 |
Sync main branch Fixes twentyhq#15121
|
LGTM, thank you @pvrnn! |
|
Thanks @pvrnn for your contribution! |





Implementation
Add height: inherit so the Calendar fits the parent’s height:
twenty/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexContainer.tsx
Line 33 in d7e3208
Add
min-height: 1000pxto ensure each week has a minimum height and to avoid breaking the scroll wrapper on the Y-axis. Similarly to what is done for the width.Issue: Calendar view shouldn't have a max height #15121
testing
loom: https://www.loom.com/share/31c65e62074c4b49979afcd58c856f07?sid=ec56606d-e050-4136-8397-222e14313312