-
Notifications
You must be signed in to change notification settings - Fork 11.6k
feat: improve overlapping events display in weekly calendar view #24880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add cascading layout algorithm for overlapping events - First event at 80% width, subsequent events offset by 8% - Implement hover behavior to bring events to front (z-index 100) - Extract overlap logic into reusable utility functions - Add comprehensive unit tests for overlap detection and layout calculation - Sort events by start time, then by duration for consistent rendering Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- Create comprehensive playground page with 8 test scenarios - Include two overlapping, three cascading, non-overlapping, same start time, chain overlaps, dense day, touching events, and mixed statuses - Add focused view with scenario selector and grid view for side-by-side comparison - Update playground index to include weekly calendar link - Each scenario includes description, expected behavior, and collapsible event data Co-Authored-By: [email protected] <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 5 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/weekly-calendar/page.tsx">
<violation number="1" location="apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/weekly-calendar/page.tsx:344">
User-facing copy should go through t() for localization; this hardcoded “Weekly Calendar Playground” string bypasses translation and breaks the project’s localization convention. Please wrap it (and other visible strings in this component) with the appropriate t() call.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| return ( | ||
| <div className="space-y-8 p-6"> | ||
| <div> | ||
| <h1 className="text-3xl font-bold">Weekly Calendar Playground</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User-facing copy should go through t() for localization; this hardcoded “Weekly Calendar Playground” string bypasses translation and breaks the project’s localization convention. Please wrap it (and other visible strings in this component) with the appropriate t() call.
Prompt for AI agents
Address the following comment on apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/weekly-calendar/page.tsx at line 344:
<comment>User-facing copy should go through t() for localization; this hardcoded “Weekly Calendar Playground” string bypasses translation and breaks the project’s localization convention. Please wrap it (and other visible strings in this component) with the appropriate t() call.</comment>
<file context>
@@ -0,0 +1,409 @@
+ return (
+ <div className="space-y-8 p-6">
+ <div>
+ <h1 className="text-3xl font-bold">Weekly Calendar Playground</h1>
+ <p className="text-subtle mt-2">
+ Test the overlapping events cascading layout with various scenarios. Events should cascade with
</file context>
- Refactor useCalendarStore to use Zustand's createStore with React Context - Each Calendar component now creates its own isolated store instance - Maintains backward compatibility with global store fallback - Fixes issue where multiple Calendar instances on same page shared state This allows the playground page to render multiple Calendar instances without state conflicts between them. Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 1 file
- Dynamically compress offset step based on overlap group size - Ensure leftOffset + width never exceeds 100% to prevent bleeding into next day - Add width clamping as safety guard against rounding errors - Add 3 new tests for overflow prevention with dense event scenarios Fixes issue where 10+ overlapping events would cascade beyond day boundary and bleed into the next day's column. The algorithm now calculates the maximum safe step size per overlap group while maintaining visual cascade. Co-Authored-By: [email protected] <[email protected]>
- Add safetyMarginPercent (0.5%) to prevent rounding and CSS box model overflow - Implement floor3 rounding for width to guarantee left + width <= 100 - safetyMargin - Calculate width from rounded left offset to avoid rounding mismatch - Remove inset-x-1 class conflict that was setting both left and right positioning - Change from marginLeft to left positioning for proper control - Add test for 20+ overlapping events to verify no overflow - Update existing tests to verify safety margin is respected This fixes the slight overflow issue reported by the user where events were still bleeding into adjacent day columns despite the initial dynamic step compression fix. The root cause was a combination of: 1. CSS conflict: inset-x-1 class setting both left and right positioning 2. Rounding mismatch: width calculated from unrounded left offset 3. No safety margin for CSS box model effects (borders, padding) All 21 tests now pass including new safety margin verification. Co-Authored-By: [email protected] <[email protected]>
- Increase from 11 to 21 overlapping events in Dense Day scenario - Update title from '10+ Events' to '20+ Events' - Add 10 more diverse events with staggered start times - Better stress test for overflow prevention with safety margin This provides a more comprehensive test case for the overflow prevention fix and matches the user's request for 20+ events instead of 10+. Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 4 files
…com:calcom/cal.com into devin/overlapping-events-cascade-1762191291
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 5 files
- Replace simple title-only tooltip with rich content tooltip - Display event title, time range, description, and status - Add color indicator matching the event's visual style - Set min-width (200px) and max-width (300px) for better readability - Use inverted theme colors for better contrast in tooltip Co-Authored-By: [email protected] <[email protected]>
…vents-cascade-1762191291' into devin/overlapping-events-cascade-1762191291
- Show tooltip on right side for Monday-Thursday (days 1-4) - Show tooltip on left side for Friday-Sunday (days 5-0) - Calculate day of week using dayjs from event start date Co-Authored-By: [email protected] <[email protected]>
- Ensure very short events (e.g., 3 minutes) are still visible - Use CSS max() to apply minimum height while respecting duration-based height - Prevents events from becoming too small to interact with Co-Authored-By: [email protected] <[email protected]>
…com:calcom/cal.com into devin/overlapping-events-cascade-1762191291
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed changes from recent commits (found 2 issues).
2 issues found across 3 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="packages/features/calendars/weeklyview/components/event/Event.tsx">
<violation number="1" location="packages/features/calendars/weeklyview/components/event/Event.tsx:122">
`displayType !== "single-line"` is supposed to allow multi-line content, but the title keeps `whitespace-nowrap`, so longer titles are still forced to one clipped line. Please drop the nowrap (and shrink) so taller events can actually wrap their text.</violation>
</file>
<file name="packages/features/calendars/weeklyview/components/event/EventList.tsx">
<violation number="1" location="packages/features/calendars/weeklyview/components/event/EventList.tsx:69">
Setting a hard 15px minimum height makes short back-to-back events visually overlap even when their times do not, so the UI shows incorrect durations and obscures subsequent events. Please keep the rendered height proportional to the true duration (or rework the layout to avoid inflating the block beyond its actual end).</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| </div> | ||
| )} | ||
| {displayType !== "single-line" && ( | ||
| <p className={classNames("shrink-0 whitespace-nowrap text-left leading-4")}>{event.title}</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
displayType !== "single-line" is supposed to allow multi-line content, but the title keeps whitespace-nowrap, so longer titles are still forced to one clipped line. Please drop the nowrap (and shrink) so taller events can actually wrap their text.
Prompt for AI agents
Address the following comment on packages/features/calendars/weeklyview/components/event/Event.tsx at line 122:
<comment>`displayType !== "single-line"` is supposed to allow multi-line content, but the title keeps `whitespace-nowrap`, so longer titles are still forced to one clipped line. Please drop the nowrap (and shrink) so taller events can actually wrap their text.</comment>
<file context>
@@ -101,25 +103,30 @@ export function Event({
+ </div>
+ )}
+ {displayType !== "single-line" && (
+ <p className={classNames("shrink-0 whitespace-nowrap text-left leading-4")}>{event.title}</p>
+ )}
+ {displayType !== "single-line" && !event.options?.hideTime && (
</file context>
| <p className={classNames("shrink-0 whitespace-nowrap text-left leading-4")}>{event.title}</p> | |
| <p className={classNames("w-full text-left leading-4")}>{event.title}</p> |
| width: `${layout.widthPercent}%`, | ||
| zIndex, | ||
| top: `calc(${eventStartDiff}*var(--one-minute-height))`, | ||
| height: `max(15px, calc(${eventDuration}*var(--one-minute-height)))`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting a hard 15px minimum height makes short back-to-back events visually overlap even when their times do not, so the UI shows incorrect durations and obscures subsequent events. Please keep the rendered height proportional to the true duration (or rework the layout to avoid inflating the block beyond its actual end).
Prompt for AI agents
Address the following comment on packages/features/calendars/weeklyview/components/event/EventList.tsx at line 69:
<comment>Setting a hard 15px minimum height makes short back-to-back events visually overlap even when their times do not, so the UI shows incorrect durations and obscures subsequent events. Please keep the rendered height proportional to the true duration (or rework the layout to avoid inflating the block beyond its actual end).</comment>
<file context>
@@ -66,7 +66,7 @@ export function EventList({ day }: Props) {
zIndex,
top: `calc(${eventStartDiff}*var(--one-minute-height))`,
- height: `calc(${eventDuration}*var(--one-minute-height))`,
+ height: `max(15px, calc(${eventDuration}*var(--one-minute-height)))`,
transform: isHovered ? "scale(1.02)" : "scale(1)",
opacity: hoveredGroupIndex !== null && !isHovered && isInHoveredGroup ? 0.6 : 1,
</file context>
| height: `max(15px, calc(${eventDuration}*var(--one-minute-height)))`, | |
| height: `calc(${eventDuration}*var(--one-minute-height))`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 9 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/page.tsx">
<violation number="1" location="apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/page.tsx:13">
Please wrap this new link title with the t() localization helper so it adheres to our frontend translation requirement.</violation>
</file>
<file name="packages/features/calendars/weeklyview/state/store.ts">
<violation number="1" location="packages/features/calendars/weeklyview/state/store.ts:44">
Sorting the incoming `events` array in place mutates the props object passed into the calendar, which can leak side effects to callers expecting their data to stay untouched. Please sort a copy before storing it.</violation>
</file>
<file name="apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/weekly-calendar/page.tsx">
<violation number="1" location="apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/weekly-calendar/page.tsx:490">
Wrap this user-facing string with the t() localization helper so it participates in translations.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
apps/web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/page.tsx
Show resolved
Hide resolved
...web/app/(use-page-wrapper)/settings/(admin-layout)/admin/playground/weekly-calendar/page.tsx
Show resolved
Hide resolved
|
@eunjae-lee can you address all the AI reviews? Some of them are valid |
packages/features/calendars/weeklyview/components/event/EventList.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
E2E results are ready! |
What does this PR do?
This PR fixes three critical bugs in the weekly calendar component's overlapping events display:
inset-x-1class was setting both left and right positioning, conflicting with inline width stylesLink to Devin run: https://app.devin.ai/sessions/4df3fcc8688d45b0ba228ba391088626
Requested by: @eunjae-lee ([email protected])
Visual Demo
Before
After
Screenshot.2025-11-04.at.14.35.37.mp4
Mandatory Tasks
How should this be tested?
Testing the Fixes
/settings/admin/playground/weekly-calendar(with[email protected])Calendarcomponent is used