-
Notifications
You must be signed in to change notification settings - Fork 2.7k
added calendar colors #7757
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
base: preview
Are you sure you want to change the base?
added calendar colors #7757
Conversation
|
WalkthroughRefactors the calendar issue block to a vertically structured, data-driven UI using labels and members, adds priority-driven styling and indicators, introduces avatar and label displays with overflow counts, and wires quick actions with a ref. Exports priorityBlockClasses from PriorityIcon and externalizes priority mappings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant IB as IssueBlock (Calendar)
participant LM as useLabel (labelMap)
participant MM as useMember (getUserDetails)
participant PI as PriorityIcon
participant QA as QuickActions
U->>IB: Render issue block
IB->>LM: Fetch labelMap (by label_ids)
IB->>MM: Resolve assignee_ids → user details
IB->>PI: Render priority indicator (priority)
IB->>IB: Derive stateColor, layout classes
IB->>IB: Render title, state dot, IssueIdentifier
IB->>IB: Render labels (max 2) + "+N"
IB->>IB: Render avatars (max 3) + "+N"
U-->>QA: Hover/press
IB->>QA: Show contextual actions (uses blockRef)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Pull Request Overview
This PR adds calendar colors and enhanced visual details to the calendar view for improved issue display. The changes include priority indicators, assignee avatars, and labels to make issues more visually informative in the calendar layout.
- Refactored priority icon component to export reusable priority styling classes
- Enhanced calendar issue block with priority colors, assignees, and labels
- Improved layout structure from horizontal to vertical card-style design
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
packages/propel/src/icons/priority-icon.tsx | Extracted priority classes and icons to module level for reusability and exported priorityBlockClasses |
apps/web/core/components/issues/issue-layouts/calendar/issue-block.tsx | Enhanced calendar issue block with priority colors, assignee avatars, and label display |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
urgent: "bg-red-500/10 border border-red-500/30", | ||
high: "bg-orange-500/10 border border-orange-500/30", | ||
medium: "bg-yellow-500/10 border border-yellow-500/30", | ||
low: "bg-blue-500/10 border border-blue-500/30", | ||
none: "bg-gray-500/10 border border-gray-500/30", |
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.
The priorityBlockClasses
uses different color schemes than priorityClasses
(e.g., blue for low priority vs custom-primary-100). This inconsistency could confuse users as the same priority level would appear in different colors in different contexts.
urgent: "bg-red-500/10 border border-red-500/30", | |
high: "bg-orange-500/10 border border-orange-500/30", | |
medium: "bg-yellow-500/10 border border-yellow-500/30", | |
low: "bg-blue-500/10 border border-blue-500/30", | |
none: "bg-gray-500/10 border border-gray-500/30", | |
urgent: "bg-red-600/20 border border-red-600", | |
high: "bg-orange-500/20 border border-orange-500", | |
medium: "bg-yellow-500/20 border border-yellow-500", | |
low: "bg-custom-primary-100/20 border border-custom-primary-100", | |
none: "bg-custom-background-80 border border-custom-border-300", |
Copilot uses AI. Check for mistakes.
</Tooltip> | ||
))} | ||
{assignees.length > 3 && ( | ||
<div className="flex items-center justify-center bg-custom-background-80 text-custom-text-200 rounded-full ring-2 ring-white text-xs font-medium"> |
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.
The hardcoded size and styling for the assignee overflow indicator lacks proper sizing classes. It should have explicit width/height classes like w-6 h-6
to ensure consistent sizing with the Avatar component.
<div className="flex items-center justify-center bg-custom-background-80 text-custom-text-200 rounded-full ring-2 ring-white text-xs font-medium"> | |
<div className="flex items-center justify-center w-6 h-6 bg-custom-background-80 text-custom-text-200 rounded-full ring-2 ring-white text-xs font-medium"> |
Copilot uses AI. Check for mistakes.
<div className="absolute left-0 top-0 z-[99999] h-full w-full animate-pulse bg-custom-background-100/20" /> | ||
)} | ||
|
||
<div |
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.
The blockRef
reference was removed from this div (line 115), but it's still being used in the quickActions call (line 155). This will cause the parentRef to be null, potentially breaking the quick actions functionality.
<div | |
<div | |
ref={blockRef} |
Copilot uses AI. Check for mistakes.
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.
Actionable comments posted: 2
🧹 Nitpick comments (9)
packages/propel/src/icons/priority-icon.tsx (3)
15-21
: Align colors with design tokens (custom vs Tailwind palette).priorityClasses mixes custom tokens (custom-primary-100, custom-text-200) with Tailwind brand colors (red/orange/yellow). This can cause theme inconsistencies. Suggest using the design-system tokens across all priorities (bg-/text-/border- custom-*) for consistency and themeability.
23-29
: Make block colors theme-friendly and consistent with icon tokens.priorityBlockClasses uses raw Tailwind colors (e.g., bg-red-500/10), while the icon container uses custom tokens. Consider swapping to custom tokens or centralizing the palette in one place to ensure dark mode/themes render correctly.
Apply this pattern (example only; replace with your tokens):
-export const priorityBlockClasses: Record<TIssuePriorities, string> = { - urgent: "bg-red-500/10 border border-red-500/30", - high: "bg-orange-500/10 border border-orange-500/30", - medium: "bg-yellow-500/10 border border-yellow-500/30", - low: "bg-blue-500/10 border border-blue-500/30", - none: "bg-gray-500/10 border border-gray-500/30", -}; +export const priorityBlockClasses: Record<TIssuePriorities, string> = { + urgent: "bg-custom-red-500/10 border border-custom-red-500/30", + high: "bg-custom-orange-500/10 border border-custom-orange-500/30", + medium: "bg-custom-yellow-500/10 border border-custom-yellow-500/30", + low: "bg-custom-primary-100/10 border border-custom-primary-100/30", + none: "bg-custom-background-80 border border-custom-border-300", +};
31-37
: Add strong typing for icons map.Type the icons mapping to prevent missing/extra keys and improve DX.
-import { AlertCircle, Ban, SignalHigh, SignalLow, SignalMedium } from "lucide-react"; +import { AlertCircle, Ban, SignalHigh, SignalLow, SignalMedium, type LucideIcon } from "lucide-react"; … -const icons = { +const icons: Record<TIssuePriorities, LucideIcon> = { urgent: AlertCircle, high: SignalHigh, medium: SignalMedium, low: SignalLow, none: Ban, };apps/web/core/components/issues/issue-layouts/calendar/issue-block.tsx (6)
21-22
: Defensive store usage and null-safety.
- Hooks useLabel/useMember throw if providers are missing; ensure this component is always under StoreProvider.
- labelMap/assignees may be transiently empty; current .filter(Boolean) is good. Consider defaulting issue.priority type to TIssuePriorities to avoid string drift.
Also applies to: 55-56, 65-67
61-63
: Use strict equality and precompute state color lookups.
- Prefer strict comparison to avoid coercion.
- Optional: derive a Map<stateId,color> once (store or memo) to avoid O(n) find on every render.
- getProjectStates(issue?.project_id)?.find((state) => state?.id == issue?.state_id)?.color || ""; + getProjectStates(issue?.project_id)?.find((state) => state?.id === issue?.state_id)?.color || "";
86-89
: Ensure boolean typing for placement calc.As written, the expression can be an element or boolean. Coerce explicitly for clarity and TS inference.
-const isMenuActionRefAboveScreenBottom = - menuActionRef?.current && - menuActionRef?.current?.getBoundingClientRect().bottom < window.innerHeight - 220; +const isMenuActionRefAboveScreenBottom = !!( + menuActionRef.current && + menuActionRef.current.getBoundingClientRect().bottom < window.innerHeight - 220 +);
121-123
: hover:bg-opacity-80 likely won’t affect bg-*/10 colors.Tailwind’s slash opacity (bg-red-500/10) bakes alpha into the color; bg-opacity utilities won’t apply. Consider hover:opacity-90 on the container or compute hover styles per priority.
- "hover:bg-opacity-80 transition-all duration-200": !isDragging, + "hover:opacity-90 transition-opacity duration-200": !isDragging,Or move hover styles into priorityBlockClasses if you want per-priority hover colors.
170-190
: Label color alpha assumes hex format.
${label.color}20
and...40
assume 6-digit hex. If label.color ever becomes rgb()/hsl()/token, CSS will break. Optionally parse hex and compute rgba(), or gate on a hex regex.
192-211
: Ensure +N avatar counter matches avatar size.The counter lacks fixed dimensions and may misalign. Match Avatar "sm" (usually 24px).
- <div className="flex items-center justify-center bg-custom-background-80 text-custom-text-200 rounded-full ring-2 ring-white text-xs font-medium"> + <div className="flex items-center justify-center h-6 w-6 bg-custom-background-80 text-custom-text-200 rounded-full ring-2 ring-white text-[10px] font-medium"> +{assignees.length - 3} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/issues/issue-layouts/calendar/issue-block.tsx
(3 hunks)packages/propel/src/icons/priority-icon.tsx
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/core/components/issues/issue-layouts/calendar/issue-block.tsx (4)
apps/space/core/hooks/store/use-label.ts (1)
useLabel
(7-11)apps/space/core/hooks/store/use-member.ts (1)
useMember
(7-11)packages/propel/src/icons/priority-icon.tsx (2)
priorityBlockClasses
(23-29)PriorityIcon
(39-85)apps/web/ce/components/issues/issue-details/issue-identifier.tsx (1)
IssueIdentifier
(79-105)
🔇 Additional comments (2)
apps/web/core/components/issues/issue-layouts/calendar/issue-block.tsx (2)
11-11
: Confirm barrel export for priorityBlockClasses.Importing from "@plane/propel/icons" requires the barrel to re-export priorityBlockClasses. Verify the index exports; otherwise this import will fail.
115-125
: Attach blockRef to an element used by quickActions.blockRef is passed to quickActions as parentRef but isn’t attached to any DOM node, so positioning may break. Attach it to the container.
- <div + <div + ref={blockRef} className={cn( "group/calendar-block flex flex-col w-full gap-2 rounded md:px-2 px-4 py-2 min-h-[60px] md:min-h-[70px]", priorityBlockClasses[priority], { "bg-custom-background-90 shadow-custom-shadow-rg border-custom-primary-100": isDragging, - "hover:bg-opacity-80 transition-all duration-200": !isDragging, + "hover:bg-opacity-80 transition-all duration-200": !isDragging, "border-2 border-custom-primary-70 hover:border-custom-primary-70": getIsIssuePeeked(issue.id), } )} >
const handleIssuePeekOverview = (issue: TIssue) => | ||
handleRedirection(workspaceSlug.toString(), issue, isMobile); | ||
|
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.
🛠️ Refactor suggestion
Possible crash when workspaceSlug is undefined.
Calling toString() on undefined throws. Use String() or guard before redirect.
-const handleIssuePeekOverview = (issue: TIssue) =>
- handleRedirection(workspaceSlug.toString(), issue, isMobile);
+const handleIssuePeekOverview = (issue: TIssue) => {
+ const slug = workspaceSlug ? String(workspaceSlug) : "";
+ return handleRedirection(slug, issue, isMobile);
+};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleIssuePeekOverview = (issue: TIssue) => | |
handleRedirection(workspaceSlug.toString(), issue, isMobile); | |
const handleIssuePeekOverview = (issue: TIssue) => { | |
const slug = workspaceSlug ? String(workspaceSlug) : ""; | |
return handleRedirection(slug, issue, isMobile); | |
}; |
🤖 Prompt for AI Agents
In apps/web/core/components/issues/issue-layouts/calendar/issue-block.tsx around
lines 69 to 71, calling workspaceSlug.toString() can crash when workspaceSlug is
undefined; guard against undefined or coerce safely. Replace the direct
.toString() with a safe conversion like String(workspaceSlug) or add an early
guard (if (!workspaceSlug) return or handle fallback) before calling
handleRedirection so you never call toString() on undefined.
const customActionButton = ( | ||
<div | ||
ref={menuActionRef} | ||
className={`w-full cursor-pointer rounded p-1 text-custom-sidebar-text-400 hover:bg-custom-background-80 ${ | ||
isMenuActive ? "bg-custom-background-80 text-custom-text-100" : "text-custom-text-200" | ||
}`} | ||
className={`w-full cursor-pointer rounded p-1 text-custom-sidebar-text-400 hover:bg-custom-background-80 ${isMenuActive ? "bg-custom-background-80 text-custom-text-100" : "text-custom-text-200" | ||
}`} | ||
onClick={() => setIsMenuActive(!isMenuActive)} | ||
> | ||
<MoreHorizontal className="h-3.5 w-3.5" /> | ||
</div> | ||
); |
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.
🛠️ Refactor suggestion
Use a button for the action trigger (accessibility + semantics).
Clickable div lacks keyboard/ARIA support. Switch to a with aria-pressed and an accessible label.
- <div
- ref={menuActionRef}
- className={`w-full cursor-pointer rounded p-1 text-custom-sidebar-text-400 hover:bg-custom-background-80 ${isMenuActive ? "bg-custom-background-80 text-custom-text-100" : "text-custom-text-200"
- }`}
- onClick={() => setIsMenuActive(!isMenuActive)}
- >
- <MoreHorizontal className="h-3.5 w-3.5" />
- </div>
+ <button
+ ref={menuActionRef}
+ type="button"
+ aria-pressed={isMenuActive}
+ aria-label="Open quick actions"
+ className={`w-full rounded p-1 text-custom-sidebar-text-400 hover:bg-custom-background-80 ${isMenuActive ? "bg-custom-background-80 text-custom-text-100" : "text-custom-text-200"}`}
+ onClick={() => setIsMenuActive((v) => !v)}
+ >
+ <MoreHorizontal className="h-3.5 w-3.5" />
+ </button>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const customActionButton = ( | |
<div | |
ref={menuActionRef} | |
className={`w-full cursor-pointer rounded p-1 text-custom-sidebar-text-400 hover:bg-custom-background-80 ${ | |
isMenuActive ? "bg-custom-background-80 text-custom-text-100" : "text-custom-text-200" | |
}`} | |
className={`w-full cursor-pointer rounded p-1 text-custom-sidebar-text-400 hover:bg-custom-background-80 ${isMenuActive ? "bg-custom-background-80 text-custom-text-100" : "text-custom-text-200" | |
}`} | |
onClick={() => setIsMenuActive(!isMenuActive)} | |
> | |
<MoreHorizontal className="h-3.5 w-3.5" /> | |
</div> | |
); | |
const customActionButton = ( | |
<button | |
ref={menuActionRef} | |
type="button" | |
aria-pressed={isMenuActive} | |
aria-label="Open quick actions" | |
className={`w-full rounded p-1 text-custom-sidebar-text-400 hover:bg-custom-background-80 ${isMenuActive ? "bg-custom-background-80 text-custom-text-100" : "text-custom-text-200"}`} | |
onClick={() => setIsMenuActive((v) => !v)} | |
> | |
<MoreHorizontal className="h-3.5 w-3.5" /> | |
</button> | |
); |
🤖 Prompt for AI Agents
In apps/web/core/components/issues/issue-layouts/calendar/issue-block.tsx around
lines 74 to 83, replace the clickable div with a semantic <button type="button">
element: keep the same className and onClick behavior, forward the menuActionRef
to the button ref, add aria-pressed={isMenuActive} to reflect state, and provide
an accessible label via aria-label (or include a visually-hidden text) such as
"More actions" so the control is keyboard-focusable and screen-reader friendly.
Description
Added feature for #7701 . Added colors and more details to calendar view.
Type of Change
Screenshots and Media (if applicable)
before
after
Test Scenarios
References
Summary by CodeRabbit
New Features
Style
Refactor