Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
10 issues found across 43 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/components/calendar/display-item/display-item.tsx">
<violation number="1" location="apps/web/src/components/calendar/display-item/display-item.tsx:50">
P2: Rule violated: **Avoid deprecated utilities; use Tailwind v4 renamed utilities**
Use `outline-hidden` instead of `outline-none` for Tailwind v4 compatibility. The `outline-none` utility has been renamed to `outline-hidden` in Tailwind v4.</violation>
<violation number="2" location="apps/web/src/components/calendar/display-item/display-item.tsx:57">
P2: Rule violated: **CSS variable naming and usage conventions (Tailwind v4 namespaces)**
CSS variable `--calendar-color` doesn't follow the required namespace pattern. Per Tailwind v4 conventions, design tokens should use the `--color-*` namespace. Consider renaming to `--color-calendar`.</violation>
<violation number="3" location="apps/web/src/components/calendar/display-item/display-item.tsx:100">
P1: When `currentTime` is provided, both `displayStart` and `displayEnd` are set to the same value, making the duration always 0. The end time should likely still use `item.end`.</violation>
<violation number="4" location="apps/web/src/components/calendar/display-item/display-item.tsx:138">
P2: Rule violated: **CSS variable naming and usage conventions (Tailwind v4 namespaces)**
CSS variables `--calendar-color-gap` and `--calendar-color-height` don't follow the required namespace patterns. For spacing/sizing design tokens, use the `--spacing-*` namespace (e.g., `--spacing-calendar-gap`, `--spacing-calendar-height`).</violation>
<violation number="5" location="apps/web/src/components/calendar/display-item/display-item.tsx:156">
P2: The class `b` doesn't appear to be a valid Tailwind utility class. This may be a typo or incomplete class name.</violation>
<violation number="6" location="apps/web/src/components/calendar/display-item/display-item.tsx:209">
P2: Rule violated: **Avoid deprecated utilities; use Tailwind v4 renamed utilities**
Use `outline-hidden` instead of `outline-none` for Tailwind v4 compatibility. The `outline-none` utility has been renamed to `outline-hidden` in Tailwind v4.</violation>
</file>
<file name="apps/web/src/atoms/selected-display-items.ts">
<violation number="1" location="apps/web/src/atoms/selected-display-items.ts:5">
P2: This function creates a new atom instance on every call. When used in components, this causes a new atom to be created on each render, leading to unnecessary re-subscriptions and potential performance issues.
Consider using `atomFamily` from `jotai/utils` which memoizes atoms by their parameters:
```ts
import { atomFamily } from "jotai/utils";
export const isDisplayItemSelected = atomFamily((displayItemId: string) =>
atom((get) => get(selectedDisplayItemIdsAtom).includes(displayItemId))
);
```</violation>
<violation number="2" location="apps/web/src/atoms/selected-display-items.ts:12">
P2: Same issue as above - this creates a new atom instance on every call. Consider using `atomFamily`:
```ts
export const isEventSelected = atomFamily((eventId: string) =>
atom((get) => get(selectedDisplayItemIdsAtom).includes(`event_${eventId}`))
);
```</violation>
</file>
<file name="apps/web/src/components/calendar/hooks/use-event-collection.ts">
<violation number="1" location="apps/web/src/components/calendar/hooks/use-event-collection.ts:130">
P2: Inconsistent `positionedItems` structure when `items` is empty. When `days.length > 0` but `items.length === 0`, this returns `positionedItems: []` instead of `days.map(() => [])`. Consumers expecting one array per day may get `undefined` when indexing, causing potential runtime errors.</violation>
</file>
<file name="apps/web/src/components/calendar/overflow/overflow-indicator.tsx">
<violation number="1" location="apps/web/src/components/calendar/overflow/overflow-indicator.tsx:98">
P2: This branch is unreachable. The early return on line 56 (`if (items.length <= 0) { return null; }`) ensures `items.length` is always > 0 at this point. Consider removing this dead code.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| onTouchStart, | ||
| }: DisplayItemProps) { | ||
| const displayStart = currentTime ?? item.start; | ||
| const displayEnd = currentTime ?? item.end; |
Contributor
There was a problem hiding this comment.
P1: When currentTime is provided, both displayStart and displayEnd are set to the same value, making the duration always 0. The end time should likely still use item.end.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/calendar/display-item/display-item.tsx, line 100:
<comment>When `currentTime` is provided, both `displayStart` and `displayEnd` are set to the same value, making the duration always 0. The end time should likely still use `item.end`.</comment>
<file context>
@@ -0,0 +1,288 @@
+ onTouchStart,
+}: DisplayItemProps) {
+ const displayStart = currentTime ?? item.start;
+ const displayEnd = currentTime ?? item.end;
+
+ const isSelectedAtom = React.useMemo(
</file context>
Suggested change
| const displayEnd = currentTime ?? item.end; | |
| const displayEnd = item.end; |
✅ Addressed in 84b8d99
| <div className="flex min-w-0 grow items-stretch gap-y-1.5"> | ||
| {children} | ||
| <DisplayItemTypeIndicator item={item} /> | ||
| {!isFirstDay ? <div className="b h-lh" /> : null} |
Contributor
There was a problem hiding this comment.
P2: The class b doesn't appear to be a valid Tailwind utility class. This may be a typo or incomplete class name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/calendar/display-item/display-item.tsx, line 156:
<comment>The class `b` doesn't appear to be a valid Tailwind utility class. This may be a typo or incomplete class name.</comment>
<file context>
@@ -0,0 +1,288 @@
+ <div className="flex min-w-0 grow items-stretch gap-y-1.5">
+ {children}
+ <DisplayItemTypeIndicator item={item} />
+ {!isFirstDay ? <div className="b h-lh" /> : null}
+ <span className="pointer-events-none truncate">
+ {title}{" "}
</file context>
✅ Addressed in 74b5820
| return items.includes(displayItemId); | ||
| }); | ||
|
|
||
| export const isEventSelected = (eventId: string) => |
Contributor
There was a problem hiding this comment.
P2: Same issue as above - this creates a new atom instance on every call. Consider using atomFamily:
export const isEventSelected = atomFamily((eventId: string) =>
atom((get) => get(selectedDisplayItemIdsAtom).includes(`event_${eventId}`))
);Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/atoms/selected-display-items.ts, line 12:
<comment>Same issue as above - this creates a new atom instance on every call. Consider using `atomFamily`:
```ts
export const isEventSelected = atomFamily((eventId: string) =>
atom((get) => get(selectedDisplayItemIdsAtom).includes(`event_${eventId}`))
);
```</comment>
<file context>
@@ -0,0 +1,35 @@
+ return items.includes(displayItemId);
+ });
+
+export const isEventSelected = (eventId: string) =>
+ atom((get) => {
+ const items = get(selectedDisplayItemIdsAtom);
</file context>
|
|
||
| export const selectedDisplayItemIdsAtom = atom<string[]>([]); | ||
|
|
||
| export const isDisplayItemSelected = (displayItemId: string) => |
Contributor
There was a problem hiding this comment.
P2: This function creates a new atom instance on every call. When used in components, this causes a new atom to be created on each render, leading to unnecessary re-subscriptions and potential performance issues.
Consider using atomFamily from jotai/utils which memoizes atoms by their parameters:
import { atomFamily } from "jotai/utils";
export const isDisplayItemSelected = atomFamily((displayItemId: string) =>
atom((get) => get(selectedDisplayItemIdsAtom).includes(displayItemId))
);Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/atoms/selected-display-items.ts, line 5:
<comment>This function creates a new atom instance on every call. When used in components, this causes a new atom to be created on each render, leading to unnecessary re-subscriptions and potential performance issues.
Consider using `atomFamily` from `jotai/utils` which memoizes atoms by their parameters:
```ts
import { atomFamily } from "jotai/utils";
export const isDisplayItemSelected = atomFamily((displayItemId: string) =>
atom((get) => get(selectedDisplayItemIdsAtom).includes(displayItemId))
);
```</comment>
<file context>
@@ -0,0 +1,35 @@
+
+export const selectedDisplayItemIdsAtom = atom<string[]>([]);
+
+export const isDisplayItemSelected = (displayItemId: string) =>
+ atom((get) => {
+ const items = get(selectedDisplayItemIdsAtom);
</file context>
| allDayEvents: [], | ||
| positionedEvents: [], | ||
| allDayItems: [], | ||
| positionedItems: [], |
Contributor
There was a problem hiding this comment.
P2: Inconsistent positionedItems structure when items is empty. When days.length > 0 but items.length === 0, this returns positionedItems: [] instead of days.map(() => []). Consumers expecting one array per day may get undefined when indexing, causing potential runtime errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/calendar/hooks/use-event-collection.ts, line 130:
<comment>Inconsistent `positionedItems` structure when `items` is empty. When `days.length > 0` but `items.length === 0`, this returns `positionedItems: []` instead of `days.map(() => [])`. Consumers expecting one array per day may get `undefined` when indexing, causing potential runtime errors.</comment>
<file context>
@@ -2,272 +2,197 @@ import { useMemo } from "react";
- allDayEvents: [],
- positionedEvents: [],
+ allDayItems: [],
+ positionedItems: [],
+ backgroundItems: [],
+ sideItems: [],
</file context>
Suggested change
| positionedItems: [], | |
| positionedItems: days.map(() => []), |
✅ Addressed in 84b8d99
| <div className="max-h-96 overflow-auto px-2 pb-2"> | ||
| {items.length === 0 ? ( | ||
| <div className="py-2 text-sm text-muted-foreground">No events</div> | ||
| <div className="py-2 text-sm text-muted-foreground">No items</div> |
Contributor
There was a problem hiding this comment.
P2: This branch is unreachable. The early return on line 56 (if (items.length <= 0) { return null; }) ensures items.length is always > 0 at this point. Consider removing this dead code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/calendar/overflow/overflow-indicator.tsx, line 98:
<comment>This branch is unreachable. The early return on line 56 (`if (items.length <= 0) { return null; }`) ensures `items.length` is always > 0 at this point. Consider removing this dead code.</comment>
<file context>
@@ -94,15 +95,15 @@ export function OverflowIndicator({
<div className="max-h-96 overflow-auto px-2 pb-2">
{items.length === 0 ? (
- <div className="py-2 text-sm text-muted-foreground">No events</div>
+ <div className="py-2 text-sm text-muted-foreground">No items</div>
) : (
<div className="space-y-2">
</file context>
✅ Addressed in 84b8d99
JeanMeijer
force-pushed
the
feat/add-display-items
branch
from
ec60f35 to
74b5820
Compare
JeanMeijer
force-pushed
the
feat/add-display-items
branch
from
74b5820 to
9dba6d3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Briefly describe what you did and why.
Screenshots / Recordings
Add screenshots or recordings here to help reviewers understand your changes.
Type of Change
Related Areas
Testing
Checklist
Notes
(Optional) Add anything else you'd like to share.
By submitting, I confirm I understand and stand behind this code. If AI was used, I’ve reviewed and verified everything myself.
Summary by cubic
Introduces a unified DisplayItem model and UI, replacing event-only logic across the calendar. This refactor simplifies selection, layout, and positioning, and lays the groundwork for tasks, locations, and journeys.
New Features
Refactors
Written for commit 84b8d99. Summary will update on new commits.