[Feat]: Add MonthlyCalendarDataView and WeeklyCalendarDataView for displaying timesheet #3388
Conversation
…eration and timesheet grouping
WalkthroughThe changes introduce two new components, Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx (2)
9-23: Consider adding prop validation and documentation.The props interface could benefit from JSDoc documentation and stricter type definitions:
datashould specify the expected shape more preciselyonDateClickcallback should include error handling guidanceclassNamescould use a more specific type for styling customization+/** + * Props for the MonthlyTimesheetCalendar component + * @param data - Array of grouped timesheet data + * @param onDateClick - Callback fired when a date is clicked + * @param renderDayContent - Custom renderer for day cell content + * @param locale - Date-fns locale object for internationalization + * @param daysLabels - Custom labels for days of the week + * @param noDataText - Text to display when no data is available + * @param classNames - Custom CSS classes for styling + */ type MonthlyCalendarDataViewProps = { - data?: GroupedTimesheet[]; + data?: readonly GroupedTimesheet[]; - onDateClick?: (date: Date) => void; + onDateClick?: (date: Date) => Promise<void> | void; renderDayContent?: (date: Date, plan?: GroupedTimesheet) => React.ReactNode; locale?: Locale; - daysLabels?: string[]; + daysLabels?: readonly string[]; noDataText?: string; classNames?: { container?: string; header?: string; grid?: string; day?: string; noData?: string; }; };
44-49: Optimize performance with proper memoization.The current implementation could benefit from additional memoization:
- Memoize event handlers
- Consider using
useCallbackfor thegroupedData.getoperationconst [currentMonth, setCurrentMonth] = useState(new Date()); const calendarDates = useMemo(() => generateFullCalendar(currentMonth), [currentMonth]); +const getGroupedData = useCallback( + (date: Date) => groupedData.get(format(date, "yyyy-MM-dd")), + [groupedData] +); const groupedData = useMemo( () => new Map(data.map((plan) => [format(new Date(plan.date), "yyyy-MM-dd"), plan])), [data] );apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (2)
24-27: Simplify conditional rendering logic.The nested ternary operators make the code harder to read and maintain.
-{timesheetGroupByDays === 'Monthly' ? - <MonthlyCalendarDataView data={data} /> : - timesheetGroupByDays === 'Daily' ? - <CalendarDataView data={data} t={t} /> : null} +{(() => { + switch (timesheetGroupByDays) { + case 'Monthly': + return <MonthlyCalendarDataView data={data} />; + case 'Daily': + return <CalendarDataView data={data} t={t} />; + default: + return null; + } +})()}
225-225: Fix optional chaining as suggested by static analysis.The static analysis tool correctly identified a potential null reference.
-<span className="flex-1">{task.project && task.project.name}</span> +<span className="flex-1">{task.project?.name}</span>🧰 Tools
🪛 Biome (1.9.4)
[error] 225-225: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx(6 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx(1 hunks)apps/web/lib/features/task/task-displays.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
[error] 225-225: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx (1)
27-33: Consider memoizing calendar generation parameters.While the calendar generation logic is correct, the start and end date calculations could be memoized separately to avoid recalculation during renders.
+const useCalendarBoundaries = (currentMonth: Date) => { + return useMemo(() => { + const monthStart = startOfMonth(currentMonth); + const monthEnd = endOfMonth(currentMonth); + const startDate = addDays(monthStart, -monthStart.getDay()); + const endDate = addDays(monthEnd, 6 - monthEnd.getDay()); + return { startDate, endDate }; + }, [currentMonth]); +}; const generateFullCalendar = (currentMonth: Date) => { - const monthStart = startOfMonth(currentMonth); - const monthEnd = endOfMonth(currentMonth); - const startDate = addDays(monthStart, -monthStart.getDay()); - const endDate = addDays(monthEnd, 6 - monthEnd.getDay()); + const { startDate, endDate } = useCalendarBoundaries(currentMonth); return eachDayOfInterval({ start: startDate, end: endDate }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx(1 hunks)
🔇 Additional comments (4)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx (4)
1-23: LGTM! Well-structured type definitions and imports.
The MonthlyCalendarDataViewProps interface is well-defined with appropriate optional properties, providing good flexibility for component customization.
81-121: Previous accessibility concerns remain unaddressed.
The calendar grid still needs accessibility improvements as mentioned in the previous review.
111-113: Improve fallback content rendering.
The current fallback for when renderDayContent is not provided still uses JSON.stringify.
82-85: Verify XSS protection and date validation.
Please ensure:
- User-provided dates are properly validated
- Content rendering is protected against XSS
✅ Verification successful
Let me gather more specific information about the date handling and content rendering in this component.
Based on the results, I can now generate the final response as we have sufficient information about date handling and content rendering.
No security concerns found in date handling and content rendering
The code shows safe practices:
- Dates are handled using the
formatfunction with specific format strings ("yyyy-MM-dd", "dd MMM yyyy") - Plan data is rendered through simple property access (plan.tasks, plan.date)
- No direct HTML interpolation or dangerous content rendering patterns are present
- Data is displayed through structured components (TotalDurationByDate)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for date validation and sanitization patterns
rg -l "sanitize|validate|xss" --type ts
# Search for usage of dangerouslySetInnerHTML
ast-grep --pattern 'dangerouslySetInnerHTML = { $$ }'
Length of output: 1601
Script:
#!/bin/bash
# Check how the date is being used in the component
rg -A 5 "format\(.*date" apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx
# Check content rendering patterns in the component
ast-grep --pattern 'return (
$$$
{plan?.$$}
$$$
)'
# Check if there's any HTML rendering from plan data
rg -A 5 "plan\." apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx
Length of output: 2034
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx (4)
1-7: Consider organizing imports by category.While the imports are valid, they could be better organized for maintainability:
-import React, { useMemo, useState, useCallback } from "react"; -import { format, addMonths, eachDayOfInterval, startOfMonth, endOfMonth, addDays, Locale } from "date-fns"; -import { GroupedTimesheet } from "@/app/hooks/features/useTimesheet"; -import { enGB } from 'date-fns/locale'; -import { cn } from "@/lib/utils"; -import { TotalDurationByDate } from "@/lib/features"; -import { formatDate } from "@/app/helpers"; +// React and core dependencies +import React, { useMemo, useState, useCallback } from "react"; + +// Date utilities +import { format, addMonths, eachDayOfInterval, startOfMonth, endOfMonth, addDays, Locale } from "date-fns"; +import { enGB } from 'date-fns/locale'; + +// Application utilities and types +import { cn } from "@/lib/utils"; +import { formatDate } from "@/app/helpers"; +import { GroupedTimesheet } from "@/app/hooks/features/useTimesheet"; +import { TotalDurationByDate } from "@/lib/features";
27-33: Add JSDoc documentation for the generateFullCalendar function.The function logic is correct, but would benefit from documentation explaining its purpose and return value.
+/** + * Generates an array of dates for a full calendar month view, including padding days + * from adjacent months to complete weeks. + * @param currentMonth - The date object representing the current month + * @returns Array of dates from the last Sunday before the month start to the first Saturday after the month end + */ const generateFullCalendar = (currentMonth: Date) => {
35-37: Remove unnecessary empty lines.These consecutive empty lines should be removed to maintain consistent spacing.
27-33: Consider moving generateFullCalendar to a shared utility file.This function could be useful for other calendar components. Consider moving it to a shared utility file, such as
lib/calendar-utils.ts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx(1 hunks)
🔇 Additional comments (1)
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx (1)
49-52:
Add error handling for date parsing in groupedData.
The date parsing could throw an error if the date string is invalid.
const groupedData = useMemo(
- () => new Map(data.map((plan) => [format(new Date(plan.date), "yyyy-MM-dd"), plan])),
+ () => new Map(
+ data.map((plan) => {
+ try {
+ return [format(new Date(plan.date), "yyyy-MM-dd"), plan];
+ } catch (error) {
+ console.error(`Invalid date format for plan:`, plan);
+ return null;
+ }
+ }).filter(Boolean) as [string, GroupedTimesheet][]
+ ),
[data]
);Likely invalid or redundant comment.
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (4)
apps/web/app/[locale]/timesheet/[memberId]/page.tsx (1)
42-45: Enhance type safety for view mode state.Consider using a const assertion for view mode values to prevent typos and ensure type safety.
+const TIMESHEET_VIEW_MODES = { + LIST: 'ListView', + CALENDAR: 'CalendarView' +} as const; +type TimesheetViewMode = typeof TIMESHEET_VIEW_MODES[keyof typeof TIMESHEET_VIEW_MODES]; const [timesheetNavigator, setTimesheetNavigator] = useLocalStorageState<TimesheetViewMode>( 'timesheet-viewMode', - 'ListView' + TIMESHEET_VIEW_MODES.LIST );apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (3)
1-14: Consider organizing imports for better maintainabilityGroup imports into these categories for better organization:
- External dependencies (React, next-intl)
- Internal components (@components)
- Utilities and hooks (@/app/hooks, @/lib/utils)
- Types and interfaces
233-234: Use optional chaining for safer property accessReplace redundant null checks with optional chaining:
-{task.project && <ProjectLogo imageUrl={task.project.imageUrl as string} />} -<span className="flex-1">{task.project && task.project.name}</span> +<ProjectLogo imageUrl={task.project?.imageUrl as string} /> +<span className="flex-1">{task.project?.name}</span>Also applies to: 326-327
🧰 Tools
🪛 Biome (1.9.4)
[error] 234-234: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
16-57: Consider adding error handlingThe component handles loading and empty states but lacks error handling for failed data fetches. Consider:
- Adding an error state UI
- Implementing an error boundary
- Adding retry functionality for failed fetches
Example implementation:
export function CalendarView({ data, loading, error }: { data?: GroupedTimesheet[], loading: boolean, error?: Error }) { if (error) { return ( <div className="flex flex-col items-center justify-center h-full"> <p className="text-red-500">{error.message}</p> <button onClick={onRetry}>Retry</button> </div> ); } // ... existing code }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx(6 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/CompactTimesheetComponent.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetView.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/WeeklyTimesheetCalendar.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/page.tsx(2 hunks)apps/web/app/hooks/features/useTimesheet.ts(3 hunks)apps/web/lib/features/integrations/calendar/table-time-sheet.tsx(2 hunks)apps/web/lib/features/task/task-displays.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
[error] 348-348: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
[error] 234-234: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 327-327: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (9)
apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetView.tsx (1)
21-21: LGTM! Layout improvement for empty state
The removal of h-full while maintaining min-h-[280px] ensures a consistent minimum height for the empty state container.
apps/web/lib/features/task/task-displays.tsx (1)
108-109: Optimize performance and review date handling
- The separate filter and reduce operations can be combined for better performance
- Converting to ISO string might cause timezone issues depending on the implementation of
formatDate
Consider this optimization:
- const targetDateISO = new Date(createdAt).toISOString();
- const filteredLogs = timesheetLog.filter(
- (item) => formatDate(item.timesheet.createdAt) === formatDate(targetDateISO));
- const totalDurationInSeconds = filteredLogs.reduce(
- (total, log) => total + (log.timesheet?.duration || 0), 0);
+ const targetDate = formatDate(createdAt);
+ const totalDurationInSeconds = timesheetLog.reduce((total, log) => {
+ if (formatDate(log.timesheet.createdAt) === targetDate) {
+ return total + (log.timesheet?.duration || 0);
+ }
+ return total;
+ }, 0);Let's verify the date handling implementation:
✅ Verification successful
Let me verify the implementation of date handling in the codebase. I'll check how the formatDate is used in the task-displays.tsx file to understand the complete context.
Based on the verification results, I can now provide the final response:
Optimization suggestion is valid, but date handling is safe
The performance optimization suggestion to combine filter and reduce is valid. However, the date handling concern can be dismissed as the implementation in apps/web/app/helpers/date.ts shows that:
formatDateuses moment.js with a consistent format ('ddd DD MMM YYYY')- Both dates are processed through the same
formatDatefunction before comparison - The conversion to ISO string is indeed unnecessary since moment.js can handle both string and Date objects
The suggested refactor remains beneficial for performance:
- const targetDateISO = new Date(createdAt).toISOString();
- const filteredLogs = timesheetLog.filter(
- (item) => formatDate(item.timesheet.createdAt) === formatDate(targetDateISO));
- const totalDurationInSeconds = filteredLogs.reduce(
- (total, log) => total + (log.timesheet?.duration || 0), 0);
+ const targetDate = formatDate(createdAt);
+ const totalDurationInSeconds = timesheetLog.reduce((total, log) => {
+ if (formatDate(log.timesheet.createdAt) === targetDate) {
+ return total + (log.timesheet?.duration || 0);
+ }
+ return total;
+ }, 0);🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check formatDate implementation for potential timezone issues
rg -A 10 "export.*formatDate"
Length of output: 2507
Script:
#!/bin/bash
# Check how formatDate is used in task-displays.tsx
rg -A 5 "formatDate.*createdAt" apps/web/lib/features/task/task-displays.tsx
Length of output: 436
apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx (3)
1-33: LGTM! Well-structured type definitions and helper functions.
The type definitions are comprehensive and the helper function correctly handles calendar date generation.
117-121: Add null check for plan.tasks.
The TotalDurationByDate component might receive undefined tasks if plan exists but plan.tasks is undefined.
47-50: 🛠️ Refactor suggestion
Add error handling for date parsing.
The data grouping logic should handle invalid dates gracefully to prevent runtime errors.
const groupedData = useMemo(
- () => new Map(data.map((plan) => [format(new Date(plan.date), "yyyy-MM-dd"), plan])),
+ () => new Map(data.map((plan) => {
+ try {
+ return [format(new Date(plan.date), "yyyy-MM-dd"), plan];
+ } catch (error) {
+ console.error(`Invalid date format for plan:`, plan);
+ return null;
+ }
+ }).filter(Boolean)),
[data]
);Likely invalid or redundant comment.
apps/web/app/[locale]/timesheet/[memberId]/components/WeeklyTimesheetCalendar.tsx (1)
48-51: Add error handling for date parsing.
The data grouping logic should handle invalid dates gracefully to prevent runtime errors.
apps/web/app/[locale]/timesheet/[memberId]/page.tsx (1)
52-55: Verify useTimesheet hook implementation for view modes.
Ensure that the useTimesheet hook properly handles the timesheetViewMode parameter and adjusts data fetching accordingly.
✅ Verification successful
The useTimesheet hook properly handles the timesheetViewMode parameter
The hook implementation shows proper handling of the timesheetViewMode parameter in the timesheetElementGroup calculation (lines 266-277). It correctly:
- Checks if the view mode is 'ListView' and applies appropriate grouping based on timesheetGroupByDays setting
- Falls back to groupByDate for other view modes
- Includes timesheetViewMode in the dependency array of useMemo
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check useTimesheet hook implementation
# Search for useTimesheet hook definition and its usage of timesheetViewMode
ast-grep --pattern 'export function useTimesheet({ $$$, timesheetViewMode $$$}) {
$$$
}'
Length of output: 18188
apps/web/app/hooks/features/useTimesheet.ts (1)
14-14: LGTM! View mode handling looks solid.
The implementation cleanly handles different view modes while maintaining backward compatibility with the optional parameter. The grouping logic is well-organized with proper fallbacks.
Let's verify that all components using useTimesheet have been updated to handle the new view modes:
Also applies to: 267-277
✅ Verification successful
All components using useTimesheet have been properly updated for view mode handling
The verification shows that the timesheetViewMode parameter is correctly:
- Defined in
useTimesheet.ts - Passed from the main timesheet page (
[memberId]/page.tsx) using thetimesheetNavigatorvalue - Properly handled in the hook's implementation for both ListView and CalendarView modes
- Used by all relevant components including CalendarView and ListView components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all components using useTimesheet to verify view mode handling
# Search for useTimesheet hook usage
rg "useTimesheet\(" -A 5
# Search for timesheetViewMode references
rg "timesheetViewMode"
Length of output: 6764
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (1)
196-238: 🛠️ Refactor suggestion
Extract shared task rendering logic into a reusable component
The task rendering logic is duplicated across CalendarDataView, MonthlyCalendarDataView, and WeeklyCalendarDataView components. This violates the DRY principle and makes maintenance difficult.
Create a shared TaskCard component:
interface TaskCardProps {
task: Task;
status: string;
}
const TaskCard: React.FC<TaskCardProps> = ({ task, status }) => (
<div
style={{
backgroundColor: statusColor(status).bgOpacity,
borderLeftColor: statusColor(status).border
}}
className={cn(
'border-l-4 rounded-l flex flex-col p-2 gap-2 items-start space-x-4 h-[110px] w-full',
)}>
{/* ... shared task rendering logic ... */}
</div>
);Also applies to: 289-331
🧰 Tools
🪛 Biome (1.9.4)
[error] 234-234: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/[locale]/timesheet/[memberId]/components/CompactTimesheetComponent.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/timesheet/[memberId]/components/WeeklyTimesheetCalendar.tsx
Show resolved
Hide resolved
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
Outdated
Show resolved
Hide resolved
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (1)
25-31: Consider using a view map for better maintainabilityThe current conditional rendering can be made more maintainable by using a mapping object.
+const VIEW_COMPONENTS = { + Monthly: MonthlyCalendarDataView, + Weekly: WeeklyCalendarDataView, + Daily: CalendarDataView, +} as const; + export function CalendarView({ data, loading }: { data?: GroupedTimesheet[], loading: boolean }) { const t = useTranslations(); const { timesheetGroupByDays } = useTimelogFilterOptions(); + const ViewComponent = VIEW_COMPONENTS[timesheetGroupByDays as keyof typeof VIEW_COMPONENTS] || CalendarDataView; return ( <div className="grow h-full bg-[#FFFFFF] dark:bg-dark--theme"> {data ? ( data.length > 0 ? ( - <> - {timesheetGroupByDays === 'Monthly' ? ( - <MonthlyCalendarDataView data={data} /> - ) : timesheetGroupByDays === 'Weekly' ? ( - <WeeklyCalendarDataView data={data} /> - ) : ( - <CalendarDataView data={data} t={t} /> - )} - </> + <ViewComponent data={data} t={t} /> ) : ( // ... rest of the code
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx(6 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/CompactTimesheetComponent.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/WeeklyTimesheetCalendar.tsx(1 hunks)apps/web/lib/features/integrations/calendar/table-time-sheet.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/lib/features/integrations/calendar/table-time-sheet.tsx
- apps/web/app/[locale]/timesheet/[memberId]/components/WeeklyTimesheetCalendar.tsx
- apps/web/app/[locale]/timesheet/[memberId]/components/CompactTimesheetComponent.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
[error] 231-231: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 324-324: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (1)
138-141: Extract shared task rendering logic to reduce duplication
The task rendering logic is duplicated between CalendarDataView, MonthlyCalendarDataView, and WeeklyCalendarDataView components.
Additionally, optimize the project name rendering with optional chaining:
-{task.project && <ProjectLogo imageUrl={task.project.imageUrl as string} />}
-<span className="flex-1">{task.project && task.project.name}</span>
+{task.project?.imageUrl && <ProjectLogo imageUrl={task.project.imageUrl as string} />}
+<span className="flex-1">{task.project?.name}</span>🧰 Tools
🪛 Biome (1.9.4)
[error] 140-140: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (1)
234-235: Improve optional chaining consistency.Use optional chaining consistently for project-related checks:
-{task.project && <ProjectLogo imageUrl={task.project.imageUrl as string} />} -<span className="flex-1">{task.project && task.project.name}</span> +<ProjectLogo imageUrl={task.project?.imageUrl as string} /> +<span className="flex-1">{task.project?.name}</span>🧰 Tools
🪛 Biome (1.9.4)
[error] 235-235: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
[error] 235-235: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (5)
4-18: LGTM! Clean import organization and interface definition.
The imports are well-organized and the BaseCalendarDataViewProps interface is properly typed.
22-35: LGTM! Clean conditional rendering implementation.
The view mode selection using ternary operators is clear and maintainable.
Line range hint 55-159: Consider extracting shared task rendering logic.
The task rendering logic is duplicated between CalendarDataView and BaseCalendarDataView. This was previously suggested and remains a valid improvement opportunity.
🧰 Tools
🪛 Biome (1.9.4)
[error] 144-144: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
166-168: Add error boundary for date parameter.
The renderDayContent callback should validate the date parameter to handle edge cases.
255-261: LGTM! Clean implementation of calendar view wrappers.
The Monthly and Weekly view components are well-structured and follow the DRY principle by leveraging the BaseCalendarDataView.
Innocent-Akim
removed
the
Don't Merge
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (1)
169-263: Consider implementing the commented locale prop.The BaseCalendarDataView component effectively reduces code duplication, but there's a commented locale prop that should be implemented or removed.
- // locale={ } + locale={/* Add proper locale implementation */}🧰 Tools
🪛 Biome (1.9.4)
[error] 245-245: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/web/locales/nl.json (1)
244-252: Consider using standard Dutch abbreviation for Friday.The abbreviation "Vri" for Friday is uncommon in Dutch. The standard abbreviation is "Vrij".
"DAYS": { "sun": "Zon", "mon": "Maa", "tue": "Din", "wed": "Woe", "thu": "Don", - "fri": "Vri", + "fri": "Vrij", "sat": "Zat" }apps/web/locales/ru.json (1)
244-252: Consider implementing a minimum width constraint in the calendar component.To ensure consistent layout across all languages:
- Consider setting a minimum width for day label containers to accommodate the longest abbreviations
- Use CSS text-overflow properties for graceful handling of longer translations
- Consider adding a maximum character limit in the translation guidelines for day abbreviations
This will help maintain a consistent UI across all supported languages while being future-proof for additional language support.
apps/web/locales/fr.json (1)
261-269: LGTM! Consider adding full day names for accessibility.The French abbreviations for days of the week are correct and follow standard French conventions. However, for better accessibility and flexibility in the UI, consider adding full day names alongside the abbreviations.
"DAYS": { "sun": "Dim", "mon": "Lun", "tue": "Mar", "wed": "Mer", "thu": "Jeu", "fri": "Ven", "sat": "Sam", + "sunday": "Dimanche", + "monday": "Lundi", + "tuesday": "Mardi", + "wednesday": "Mercredi", + "thursday": "Jeudi", + "friday": "Vendredi", + "saturday": "Samedi" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx(6 hunks)apps/web/locales/ar.json(1 hunks)apps/web/locales/bg.json(1 hunks)apps/web/locales/de.json(1 hunks)apps/web/locales/en.json(1 hunks)apps/web/locales/es.json(1 hunks)apps/web/locales/fr.json(1 hunks)apps/web/locales/he.json(1 hunks)apps/web/locales/it.json(1 hunks)apps/web/locales/nl.json(1 hunks)apps/web/locales/pl.json(1 hunks)apps/web/locales/pt.json(1 hunks)apps/web/locales/ru.json(1 hunks)apps/web/locales/zh.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/locales/en.json
🧰 Additional context used
🪛 Biome (1.9.4)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx
[error] 245-245: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (13)
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (3)
4-18: LGTM! Well-organized imports and interface definition.
The imports are logically grouped and the BaseCalendarDataViewProps interface is well-defined with proper typing.
22-44: LGTM! Clean conditional rendering implementation.
The component effectively handles different calendar view modes using conditional rendering, and properly integrates with the localization system.
265-271: LGTM! Clean implementation of calendar view components.
The MonthlyCalendarDataView and WeeklyCalendarDataView components effectively use composition and prop spreading for a DRY implementation.
apps/web/locales/zh.json (1)
244-252: LGTM! Proper Chinese translations for days of the week.
The translations are correctly implemented using proper Chinese characters and follow the established localization structure.
apps/web/locales/he.json (1)
261-269: LGTM! Proper Hebrew translations for days of the week.
The translations are correctly implemented using proper Hebrew characters and follow the established localization structure and Hebrew naming conventions.
apps/web/locales/ar.json (1)
243-251: LGTM! Arabic translations for days are accurate and well-structured.
The added translations use proper Arabic names for days of the week and maintain consistent JSON structure.
apps/web/locales/bg.json (1)
261-269: LGTM! Bulgarian translations for days are accurate and well-structured.
The added translations use proper Bulgarian abbreviations for days of the week and maintain consistent JSON structure.
apps/web/locales/pl.json (1)
244-252: LGTM! Polish day abbreviations are correctly implemented.
The translations use proper Polish abbreviated day names in a consistent 3-letter format, which is standard in Polish localization.
apps/web/locales/it.json (1)
244-252: LGTM! Italian day abbreviations are correctly implemented.
The translations use proper Italian abbreviated day names in a consistent 3-letter format, which is standard in Italian localization.
apps/web/locales/ru.json (1)
244-252: LGTM! Russian day abbreviations are correctly implemented.
The translations use proper Russian abbreviated day names in a 2-letter format, which is standard in Russian localization.
Since Russian uses 2-letter abbreviations while other languages use 3 letters, let's verify the calendar component handles varying abbreviation lengths properly:
✅ Verification successful
Let me try a different search strategy to find calendar-related components and their styling.
Let me check the calendar components to verify how they handle day labels.
The Russian day abbreviations are safe to use with the current calendar implementations
After reviewing the calendar implementations, I can confirm that varying abbreviation lengths are properly handled:
- The base calendar component (
calendar.tsx) uses fixed-width cells withw-9class for day headers and day cells, ensuring consistent layout regardless of text length. - Both
MonthlyTimesheetCalendarandWeeklyTimesheetCalendaruse grid layouts withgrid-cols-7and equal column widths, which automatically handle varying text lengths. - The day labels are rendered in containers that maintain their width regardless of content length.
The Russian 2-letter abbreviations will work perfectly within these layouts, as the components are designed to accommodate varying text lengths across different languages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the calendar component has width handling for day labels
# Look for width-related CSS properties in calendar components
echo "Searching for calendar components and their width handling..."
rg -A 5 "MonthlyCalendarDataView|WeeklyCalendarDataView" --type tsx
echo "Checking for flex or grid layouts that might handle varying widths..."
ast-grep --pattern 'style={$_} | className={$_}' --lang tsx
Length of output: 441
Script:
#!/bin/bash
# Search for calendar-related files and components
echo "Searching for calendar-related files..."
fd -e js -e jsx -e ts -e tsx -e css -e scss calendar
# Look for day abbreviation usage in components
echo -e "\nSearching for day abbreviation usage..."
rg -i "dayjs|weekday|abbreviation|day.*short" --type js --type typescript
# Check other language files for day abbreviations
echo -e "\nChecking other language files for day abbreviations..."
fd -e json . -x grep -A 8 '"DAYS"' {}
Length of output: 2880
Script:
#!/bin/bash
# Check the calendar components implementation
echo "Checking calendar component implementations..."
cat apps/web/components/ui/calendar.tsx
# Check timesheet calendar implementations
echo -e "\nChecking timesheet calendar implementations..."
cat "apps/web/app/[locale]/timesheet/[memberId]/components/MonthlyTimesheetCalendar.tsx"
cat "apps/web/app/[locale]/timesheet/[memberId]/components/WeeklyTimesheetCalendar.tsx"
Length of output: 17315
apps/web/locales/pt.json (1)
245-253: LGTM! Portuguese day abbreviations are correct.
The translations use standard Portuguese abbreviations:
- Dom (Domingo)
- Seg (Segunda-feira)
- Ter (Terça-feira)
- Qua (Quarta-feira)
- Qui (Quinta-feira)
- Sex (Sexta-feira)
- Sáb (Sábado)
apps/web/locales/es.json (1)
261-269: LGTM! Spanish day abbreviations are correct.
The translations use standard Spanish abbreviations:
- Dom (Domingo)
- Lun (Lunes)
- Mar (Martes)
- Mié (Miércoles)
- Jue (Jueves)
- Vie (Viernes)
- Sáb (Sábado)
apps/web/locales/de.json (1)
261-269: LGTM! German day abbreviations are correct.
The translations use standard German abbreviations:
- So (Sonntag)
- Mo (Montag)
- Di (Dienstag)
- Mi (Mittwoch)
- Do (Donnerstag)
- Fr (Freitag)
- Sa (Samstag)
Innocent-Akim
changed the title
Innocent-Akim
changed the title
Description
Please include a summary of the changes and the related issue.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of previous status
Current screenshots
Please add here videos or images of previous status
Summary by CodeRabbit
New Features
MonthlyCalendarDataViewandWeeklyCalendarDataViewfor displaying timesheet data in monthly and weekly formats.ProjectLogocomponent to enhance task representation with project logos.timesheetNavigatorstate for toggling between 'ListView' and 'CalendarView'.Improvements
CalendarViewto switch between monthly, weekly, and daily views.EditTaskModalto simplify project selection by using a static project name.TotalDurationByDatecomponent for more accurate duration calculations.EmployeeAvatarto utilize a newImageWithLoadercomponent for better loading management.Styling